diff --git a/.gitignore b/.gitignore index 9248f16..b04f530 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ tmp/ # Local agent state .claude/ + +# Local-only audit notes (not part of the upstream repo) +requested improvements.md diff --git a/AGENTS.md b/AGENTS.md index dcf62ba..f8c6b51 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,7 @@ This project manages **Vapi voice agent configurations** as code. All resources **Template-safe first run:** In a fresh clone, prefer `npm run pull -- --bootstrap` to refresh `.vapi-state..json` and credential mappings without materializing the target org's resources into `resources//`. `npm run push -- ` will auto-run the same bootstrap sync when it detects empty or stale state for the resources being applied. -**Excluding resources from sync (`.vapi-ignore`):** To prevent specific resources from being pulled at all (e.g. assistants owned by another team or legacy resources you don't want to manage), create `resources//.vapi-ignore` with gitignore-style patterns. See `resources//.vapi-ignore.example` for syntax and examples. Ignored resources are silently skipped on every pull and never tracked in state — distinct from "locally deleted" which keeps an entry in state. +**Excluding resources from sync (`.vapi-ignore`):** To prevent specific resources from being pulled at all (e.g. assistants owned by another team or legacy resources you don't want to manage), create `resources//.vapi-ignore` with gitignore-style patterns. See `resources/.vapi-ignore.example` for syntax and examples. Ignored resources are silently skipped on every pull and never tracked in state — distinct from "locally deleted" which keeps an entry in state. **Learnings & recipes:** Before configuring resources or debugging issues, read the relevant file in **`docs/learnings/`**. Load only what you need: @@ -46,7 +46,6 @@ This project manages **Vapi voice agent configurations** as code. All resources | Add post-call analysis | Create `resources//structuredOutputs/.yml` | | Write test simulations | Create files under `resources//simulations/` | | Promote resources across orgs | Copy files between `resources//` and `resources//` | -| Test webhook event delivery locally | Run `npm run mock:webhook` and tunnel with ngrok | | Push changes to Vapi | `npm run push -- ` | | Pull latest from Vapi | `npm run pull -- `, `--force`, or `--bootstrap` | | Pull one known remote resource | `npm run pull -- --type assistants --id ` | @@ -88,9 +87,6 @@ resources/ │ └── simulations/ └── / # Another org (each is isolated) └── (same structure) - -scripts/ -└── mock-vapi-webhook-server.ts # Local webhook receiver for server message testing ``` --- @@ -755,7 +751,6 @@ npm run call -- -a # Call an assistant via WebSo npm run call -- -s # Call a squad via WebSocket npm run eval -- -s # Run evals against a squad npm run eval -- -a # Run evals against an assistant -npm run mock:webhook # Run local webhook receiver for server message testing # Maintenance npm run cleanup -- # Dry-run: show orphaned remote resources @@ -844,12 +839,3 @@ When transferring to human: 4. Create suites (batch simulations together) 5. Run via Vapi dashboard or API -### Mock Server Testing (Webhook/Message Receipt) - -If you need a local mock server to validate webhook payloads or message delivery behavior, you can add scripts under `/scripts` (for example: `scripts/mock-vapi-webhook-server.ts`) and run them locally during testing. - -- Default expectation: no provider API key is needed for local receive-only mock testing. -- If a provider-specific key is required, refer to the Vapi monorepo secrets workflow and use `dotenvx` to decrypt the needed values. -- Assume decryption only works when the corresponding private keys are already available in your zsh environment. -- For local webhook validation, prioritize core `serverMessages` event types such as `speech-update`, `status-update`, and `end-of-call-report`. -- To test callbacks from Vapi into your local machine, expose the mock server with a tunnel like `ngrok` and use that public HTTPS URL in `assistant.server.url`. diff --git a/README.md b/README.md index 16d2e6b..701a01a 100644 --- a/README.md +++ b/README.md @@ -75,10 +75,10 @@ Every command works in two modes: | `npm run push` | ✅ | `npm run push -- [flags]` | Push local resources to Vapi | | `npm run apply` | ✅ | `npm run apply -- [--force]` | Pull → Merge → Push in one shot | | `npm run call` | ✅ | `npm run call -- -a ` | Start a WebSocket call | -| `npm run cleanup` | ✅ | `npm run cleanup -- [--force]` | Delete orphaned remote resources | +| `npm run cleanup` | ✅ | `npm run cleanup -- [--force --confirm ]` | Delete orphaned remote resources (destructive run requires `--confirm `) | | `npm run eval` | — | `npm run eval -- -s ` | Run evals against an assistant/squad | -| `npm run mock:webhook` | — | — | Local webhook receiver for testing | | `npm run build` | — | — | Type-check the codebase | +| `npm test` | — | — | Run regression tests (`node:test`) | ### Interactive Mode @@ -95,15 +95,24 @@ npm run pull # → Select org # → All resources / Let me pick… # → Shows which resources are already local (✔) +# → "Overwrite locally modified files?" — defaults to NO (local-first) # → Confirm and execute + +npm run cleanup +# → Select org +# → Dry-run preview of what would be deleted +# → "Proceed with actual deletion?" — defaults to NO +# → Destructive run is gated by both your confirm AND --confirm ``` Navigation: - **Type** to search/filter resources -- **Space** to toggle selection -- **Ctrl+A** to select/deselect all visible +- **Space** to toggle the focused row (or toggle the whole group when the cursor is on a header) +- **Ctrl+A** to select/deselect all currently-visible rows +- **Ctrl+G** to toggle every item in the focused group +- **→ / ←** (right / left arrow) to expand or collapse the focused group - **Enter** to confirm -- **Esc** to go back to the previous step +- **Esc** to clear the search; press again to step back to the previous prompt ### Direct Mode @@ -221,7 +230,20 @@ npm run pull -- my-org # ✨ new-tool -> resources/my-org/tools/new-tool.yml ``` -Use `--force` to overwrite everything with the platform version. +Detection works in two layers, so it covers both day-to-day and fresh-clone +workflows: + +1. **Git-tracked changes** — files that show up in `git status` (modified, + deleted, or individually untracked) are preserved. +2. **mtime fallback** — if git can't help (no commits yet, the resource tree + isn't tracked at all, or git just had nothing to say), files that are + newer than `.vapi-state..json` are still preserved. This is the safety + net for the "fresh clone, edit a file, run pull again" case. + +Interactive `npm run pull` defaults to local-first too — it asks +`Overwrite locally modified files?` (default `No`) before forwarding the +pull. Pass `--force` directly (or answer `Yes` to that prompt) to overwrite +everything with the platform version. ### Selective Push @@ -232,13 +254,23 @@ Push only specific resources instead of everything: npm run push -- my-org assistants npm run push -- my-org tools -# By specific file +# By specific file (long form) npm run push -- my-org resources/my-org/assistants/my-assistant.md +# By specific file (short form — folder/filename) +npm run push -- my-org assistants/my-assistant.md +npm run push -- my-org simulations/personalities/skeptical-sam.yml + # Multiple files npm run push -- my-org resources/my-org/assistants/a.md resources/my-org/tools/b.yml ``` +> A bare resource id like `npm run push -- my-org my-assistant` (no folder, +> no extension) is **rejected explicitly**. The CLI prints +> `Unrecognized argument: my-assistant` and exits with a non-zero code rather +> than silently falling through to a full apply. Pass either a type +> (`assistants`) or a path (`assistants/my-assistant.md`). + ### Auto-Dependency Resolution When pushing a single squad or assistant, missing dependencies (tools, structured outputs, etc.) are automatically created first: @@ -273,18 +305,6 @@ npm run eval -- my-org -s my-squad -v eval-variables.json Evals must be pushed first (`npm run push -- my-org evals`). Eval definitions live in `resources//evals/*.yml`. -### Webhook Local Testing - -```bash -# 1) Run local receiver -npm run mock:webhook - -# 2) Expose localhost -ngrok http 8787 -``` - -Set your assistant's `server.url` to the ngrok HTTPS URL. - --- ## File Formats @@ -497,7 +517,17 @@ Tracks resource ID ↔ Vapi UUID mappings per org: vapi-gitops/ ├── docs/ │ ├── Vapi Prompt Optimization Guide.md +<<<<<<< HEAD │ └── changelog.md +======= +│ ├── changelog.md +│ └── learnings/ # Gotchas, recipes, troubleshooting per area +│ ├── assistants.md +│ ├── tools.md +│ ├── squads.md +│ ├── simulations.md +│ └── ... +>>>>>>> e280ea5 (docs: align README and AGENTS with org-slug model and P0 fixes) ├── src/ │ ├── setup.ts # Interactive setup wizard │ ├── interactive.ts # Interactive pull/push/apply/call/cleanup flows @@ -533,8 +563,12 @@ vapi-gitops/ │ ├── scenarios/ │ ├── tests/ │ └── suites/ -├── scripts/ -│ └── mock-vapi-webhook-server.ts +├── tests/ +│ ├── credentials.test.ts # Credential walker scoping (P0-1 regression suite) +│ ├── clean-resource.test.ts # null-preservation in pull (P0-3 regression suite) +│ ├── 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) ├── .env. # API token per org (gitignored) └── .vapi-state..json # State file per org ``` @@ -592,6 +626,42 @@ The credential UUID doesn't exist in the target org. Fix: Some properties can't be updated after creation. Add them to `UPDATE_EXCLUDED_KEYS` in `src/config.ts`. +### "Refusing to run destructive cleanup" errors + +`npm run cleanup` is intentionally double-gated for destructive runs: + +- `--force` alone is not enough — you also have to name the org with + `--confirm `. This catches the common mistake of copy-pasting `--force` + from another command where it had a different meaning. +- An empty state file (zero tracked resources) is refused even with both + flags. This prevents a fresh clone or a corrupted state from being misread + as "all remote resources are orphaned" and wiping the org. + +```bash +# Wrong — refused +npm run cleanup -- my-org --force + +# Right — destructive run +npm run cleanup -- my-org --force --confirm my-org + +# Bootstrapping into an empty state? Pull first. +npm run pull -- my-org --bootstrap +``` + +The interactive `npm run cleanup` flow handles both gates for you (it shows +the dry-run preview, asks you to confirm, and forwards `--force --confirm +` automatically when you say yes). + +### "Unrecognized argument" / push appears to do nothing + +If you typed `npm run push -- my-org foo` (a bare resource id with no folder +or extension), the CLI now refuses with `Unrecognized argument: foo` rather +than silently running a full apply. Pass either: + +- a resource type — `npm run push -- my-org assistants`, or +- a path — `npm run push -- my-org assistants/foo.yml` (short form) + or `npm run push -- my-org resources/my-org/assistants/foo.yml` (long form). + --- ## API Reference diff --git a/package.json b/package.json index 8d0ae39..374b9a4 100644 --- a/package.json +++ b/package.json @@ -13,8 +13,8 @@ "call": "tsx src/call-cmd.ts", "cleanup": "tsx src/cleanup-cmd.ts", "eval": "tsx src/eval.ts", - "mock:webhook": "tsx scripts/mock-vapi-webhook-server.ts", - "build": "tsc --noEmit" + "build": "tsc --noEmit", + "test": "node --import tsx --test tests/*.test.ts" }, "devDependencies": { "@types/node": "^22.0.0", diff --git a/resources/dev/.vapi-ignore.example b/resources/.vapi-ignore.example similarity index 80% rename from resources/dev/.vapi-ignore.example rename to resources/.vapi-ignore.example index 9135461..0e050a9 100644 --- a/resources/dev/.vapi-ignore.example +++ b/resources/.vapi-ignore.example @@ -1,12 +1,18 @@ # .vapi-ignore — explicit opt-out for resources this repo does NOT manage. # -# Resources matching any pattern below are skipped during `pull:` — -# never written to disk, never tracked in state. Use this for: +# Resources matching any pattern below are skipped during `npm run pull -- +# ` — never written to disk, never tracked in state. Use this for: # - Resources owned by another team or repo # - Legacy/broken resources you don't want to touch # - Resource types you don't care about (e.g. an entire `simulations/**`) # -# To activate this file, copy it to `.vapi-ignore` (no `.example` suffix). +# To activate this file, copy it into your org's resource directory and drop +# the `.example` suffix: +# +# cp resources/.vapi-ignore.example resources//.vapi-ignore +# +# Each org gets its own file — the engine reads +# `resources//.vapi-ignore` on every pull for that org. # # ───────────────────────────────────────────────────────────────────── # Pattern syntax @@ -16,6 +22,7 @@ # - tools/... # - squads/... # - structuredOutputs/... +# - evals/... # - simulations/personalities/... # - simulations/scenarios/... # - simulations/tests/... diff --git a/resources/prod/.vapi-ignore.example b/resources/prod/.vapi-ignore.example deleted file mode 100644 index 9135461..0000000 --- a/resources/prod/.vapi-ignore.example +++ /dev/null @@ -1,45 +0,0 @@ -# .vapi-ignore — explicit opt-out for resources this repo does NOT manage. -# -# Resources matching any pattern below are skipped during `pull:` — -# never written to disk, never tracked in state. Use this for: -# - Resources owned by another team or repo -# - Legacy/broken resources you don't want to touch -# - Resource types you don't care about (e.g. an entire `simulations/**`) -# -# To activate this file, copy it to `.vapi-ignore` (no `.example` suffix). -# -# ───────────────────────────────────────────────────────────────────── -# Pattern syntax -# ───────────────────────────────────────────────────────────────────── -# Patterns match against `/` (no extension): -# - assistants/... -# - tools/... -# - squads/... -# - structuredOutputs/... -# - simulations/personalities/... -# - simulations/scenarios/... -# - simulations/tests/... -# - simulations/suites/... -# -# Wildcards: -# - `*` matches any characters within a single path segment -# - `**` matches across path segments -# - `?` matches a single character -# -# Lines starting with `#` are comments. Blank lines are ignored. -# -# ───────────────────────────────────────────────────────────────────── -# Examples -# ───────────────────────────────────────────────────────────────────── - -# Ignore a specific assistant by exact resource ID -# assistants/ab-assistant-56b80091 - -# Ignore all assistants whose name starts with `legacy-` -# assistants/legacy-* - -# Ignore an entire resource type -# simulations/** - -# Ignore tools owned by another team -# tools/team-x-* diff --git a/resources/stg/.vapi-ignore.example b/resources/stg/.vapi-ignore.example deleted file mode 100644 index 9135461..0000000 --- a/resources/stg/.vapi-ignore.example +++ /dev/null @@ -1,45 +0,0 @@ -# .vapi-ignore — explicit opt-out for resources this repo does NOT manage. -# -# Resources matching any pattern below are skipped during `pull:` — -# never written to disk, never tracked in state. Use this for: -# - Resources owned by another team or repo -# - Legacy/broken resources you don't want to touch -# - Resource types you don't care about (e.g. an entire `simulations/**`) -# -# To activate this file, copy it to `.vapi-ignore` (no `.example` suffix). -# -# ───────────────────────────────────────────────────────────────────── -# Pattern syntax -# ───────────────────────────────────────────────────────────────────── -# Patterns match against `/` (no extension): -# - assistants/... -# - tools/... -# - squads/... -# - structuredOutputs/... -# - simulations/personalities/... -# - simulations/scenarios/... -# - simulations/tests/... -# - simulations/suites/... -# -# Wildcards: -# - `*` matches any characters within a single path segment -# - `**` matches across path segments -# - `?` matches a single character -# -# Lines starting with `#` are comments. Blank lines are ignored. -# -# ───────────────────────────────────────────────────────────────────── -# Examples -# ───────────────────────────────────────────────────────────────────── - -# Ignore a specific assistant by exact resource ID -# assistants/ab-assistant-56b80091 - -# Ignore all assistants whose name starts with `legacy-` -# assistants/legacy-* - -# Ignore an entire resource type -# simulations/** - -# Ignore tools owned by another team -# tools/team-x-* diff --git a/scripts/mock-vapi-webhook-server.ts b/scripts/mock-vapi-webhook-server.ts deleted file mode 100644 index 409faea..0000000 --- a/scripts/mock-vapi-webhook-server.ts +++ /dev/null @@ -1,197 +0,0 @@ -import { - createServer, - type IncomingMessage, - type ServerResponse, -} from "node:http"; - -type AnyRecord = Record; - -interface StoredEvent { - receivedAt: string; - type: string; - payload: unknown; -} - -const DEFAULT_PORT = 8787; -const MAX_STORED_EVENTS = 200; -const events: StoredEvent[] = []; - -function getPort(): number { - const raw = process.env.MOCK_VAPI_WEBHOOK_PORT; - if (!raw) return DEFAULT_PORT; - - const parsed = Number(raw); - if (!Number.isInteger(parsed) || parsed <= 0) return DEFAULT_PORT; - return parsed; -} - -function isRecord(value: unknown): value is AnyRecord { - return typeof value === "object" && value !== null; -} - -function getString(value: unknown): string | undefined { - return typeof value === "string" ? value : undefined; -} - -async function readJsonBody(req: IncomingMessage): Promise { - const chunks: Buffer[] = []; - for await (const chunk of req) { - if (typeof chunk === "string") { - chunks.push(Buffer.from(chunk)); - } else { - chunks.push(chunk); - } - } - - const raw = Buffer.concat(chunks).toString("utf8").trim(); - if (!raw) return {}; - return JSON.parse(raw); -} - -function sendJson( - res: ServerResponse, - statusCode: number, - body: unknown, -): void { - res.statusCode = statusCode; - res.setHeader("Content-Type", "application/json"); - res.end(JSON.stringify(body)); -} - -function storeEvent(type: string, payload: unknown): void { - events.unshift({ - receivedAt: new Date().toISOString(), - type, - payload, - }); - - if (events.length > MAX_STORED_EVENTS) { - events.length = MAX_STORED_EVENTS; - } -} - -function summarizeMessage(type: string, message: AnyRecord): void { - if (type === "speech-update") { - const role = getString(message.role) ?? "unknown-role"; - const status = getString(message.status) ?? "unknown-status"; - const turn = typeof message.turn === "number" ? message.turn : "n/a"; - console.log(`[speech-update] role=${role} status=${status} turn=${turn}`); - return; - } - - if (type === "status-update") { - const status = getString(message.status) ?? "unknown-status"; - console.log(`[status-update] status=${status}`); - return; - } - - if (type === "end-of-call-report") { - const endedReason = getString(message.endedReason) ?? "unknown"; - const hasArtifact = isRecord(message.artifact); - console.log( - `[end-of-call-report] endedReason=${endedReason} artifact=${hasArtifact ? "yes" : "no"}`, - ); - return; - } - - if (type === "transcript") { - const role = getString(message.role) ?? "unknown-role"; - const transcriptType = getString(message.transcriptType) ?? "unknown"; - const transcript = getString(message.transcript) ?? ""; - console.log( - `[transcript] role=${role} type=${transcriptType} chars=${transcript.length}`, - ); - return; - } - - console.log(`[${type}] received`); -} - -function buildToolResults(message: AnyRecord): AnyRecord[] { - const rawList = message.toolCallList; - if (!Array.isArray(rawList)) return []; - - const results: AnyRecord[] = []; - for (const item of rawList) { - if (!isRecord(item)) continue; - const id = getString(item.id); - const name = getString(item.name); - if (!id || !name) continue; - - results.push({ - name, - toolCallId: id, - result: JSON.stringify({ ok: true, mocked: true }), - }); - } - return results; -} - -async function handleWebhook( - req: IncomingMessage, - res: ServerResponse, -): Promise { - try { - const parsed = await readJsonBody(req); - if (!isRecord(parsed)) { - sendJson(res, 400, { error: "Expected JSON object body." }); - return; - } - - const messageValue = parsed.message; - if (!isRecord(messageValue)) { - sendJson(res, 400, { error: "Expected body.message object." }); - return; - } - - const type = getString(messageValue.type) ?? "unknown"; - storeEvent(type, parsed); - summarizeMessage(type, messageValue); - - if (type === "tool-calls") { - const results = buildToolResults(messageValue); - sendJson(res, 200, { results }); - return; - } - - sendJson(res, 200, { ok: true, receivedType: type }); - } catch (error) { - const message = error instanceof Error ? error.message : "Unknown error"; - sendJson(res, 500, { error: message }); - } -} - -function handleRequest(req: IncomingMessage, res: ServerResponse): void { - const method = req.method ?? "GET"; - const url = req.url ?? "/"; - - if (method === "GET" && url === "/health") { - sendJson(res, 200, { ok: true, eventCount: events.length }); - return; - } - - if (method === "GET" && url === "/events") { - sendJson(res, 200, { events }); - return; - } - - if (method === "POST" && (url === "/" || url === "/webhook")) { - void handleWebhook(req, res); - return; - } - - sendJson(res, 404, { - error: "Not found.", - routes: ["GET /health", "GET /events", "POST /webhook", "POST /"], - }); -} - -const port = getPort(); -const server = createServer(handleRequest); - -server.listen(port, () => { - console.log("Mock Vapi webhook server running."); - console.log(`Listening on http://localhost:${port}`); - console.log("Supported routes: GET /health, GET /events, POST /webhook"); - console.log("Set MOCK_VAPI_WEBHOOK_PORT to override the default port."); -}); diff --git a/src/call.ts b/src/call.ts index c0f9b8f..a0a5787 100644 --- a/src/call.ts +++ b/src/call.ts @@ -205,7 +205,7 @@ async function checkMicrophonePermission(): Promise { // sox not installed or permission denied console.log("⚠️ Could not verify microphone access."); console.log( - " If prompted, please grant microphone permission in System Preferences.", + " If prompted, please grant microphone permission in System Settings.", ); console.log( " System Settings > Privacy & Security > Microphone\n", diff --git a/src/cleanup.ts b/src/cleanup.ts index b4ca89c..0722714 100644 --- a/src/cleanup.ts +++ b/src/cleanup.ts @@ -71,7 +71,7 @@ interface VapiResource { } function readConfirmToken(argv: string[]): string | undefined { - // Accept either `--confirm ` or `--confirm=`. + // Accept either `--confirm ` or `--confirm=`. for (let i = 0; i < argv.length; i++) { const arg = argv[i]; if (arg === "--confirm") return argv[i + 1]; @@ -98,8 +98,8 @@ async function main(): Promise { // Destructive cleanup must be double-gated. `--force` alone is not enough // because it is easy to set habitually or copy from another command where - // it has a different meaning. Require `--confirm ` so the caller has - // to name the environment they intend to wipe. + // it has a different meaning. Require `--confirm ` so the caller has + // to name the org they intend to wipe. if (!dryRun && confirmToken !== VAPI_ENV) { console.error( `❌ Refusing to run destructive cleanup without explicit confirmation.`, @@ -125,7 +125,7 @@ async function main(): Promise { // A state file with zero tracked resources is almost always a fresh clone, // a corrupted state, or a bootstrap that has not written yet. Deleting from - // that baseline would wipe the org. Block it explicitly. + // that baseline would wipe the entire org. Block it explicitly. if (!dryRun && stateIds.size === 0) { console.error( `❌ Refusing to run destructive cleanup: state file has 0 tracked resources.`, @@ -180,6 +180,7 @@ async function main(): Promise { endpoint: "/eval/simulation/suite", deleteEndpoint: "/eval/simulation/suite", }, + { name: "evals", endpoint: "/eval", deleteEndpoint: "/eval" }, ]; for (const { name, endpoint, deleteEndpoint } of resourceTypes) { @@ -242,7 +243,9 @@ async function main(): Promise { ); console.log("🔒 DRY-RUN MODE - No resources were deleted"); console.log(" To actually delete, run:"); - console.log(` npm run cleanup -- ${VAPI_ENV} --force`); + console.log( + ` npm run cleanup -- ${VAPI_ENV} --force --confirm ${VAPI_ENV}`, + ); console.log( "═══════════════════════════════════════════════════════════════\n", ); diff --git a/src/config.ts b/src/config.ts index 24b887f..a3a51f2 100644 --- a/src/config.ts +++ b/src/config.ts @@ -107,6 +107,14 @@ function parseFlags(): { if (arg === "--force" || arg === "--bootstrap") continue; + // --confirm : consumed by cleanup.ts directly. Eat the value here so + // parseFlags' strict-arg check below doesn't trip on the slug. + if (arg === "--confirm" && args[i + 1]) { + i++; + continue; + } + if (arg.startsWith("--confirm=")) continue; + // --type / -t (repeatable): accumulate resource types if ((arg === "--type" || arg === "-t") && args[i + 1]) { const typeArg = args[i + 1]!; @@ -144,7 +152,25 @@ function parseFlags(): { // File path if (arg.includes("/") || /\.(yml|yaml|md|ts)$/.test(arg)) { filePaths.push(arg); + continue; } + + // Unknown long flags pass through (forward-compatibility for future + // flags consumed by individual commands like cleanup/eval). + if (arg.startsWith("--")) continue; + + // Bare positional that didn't match anything. Refuse explicitly so the + // user does not silently run a full apply when they meant a partial. The + // previous behavior was to silently drop the argument; for bare resource + // ids (e.g. `npm run push -- foo`) this triggered a full apply + // with full orphan-deletion check, which can wipe state with `--force`. + console.error(`❌ Unrecognized argument: ${arg}`); + console.error( + ` Expected a resource type (e.g. assistants, tools), a folder path ` + + `(e.g. assistants/foo.yml or resources//assistants/foo.yml), or ` + + `a flag (--force, --bootstrap, --type, --id).`, + ); + process.exit(1); } if (filePaths.length > 0) { @@ -163,8 +189,8 @@ function parseFlags(): { function loadEnvFile(env: string, baseDir: string): void { const envFiles = [ - join(baseDir, `.env.${env}`), // .env.dev, .env.stg, .env.prod - join(baseDir, `.env.${env}.local`), // .env.dev.local (for local overrides) + join(baseDir, `.env.${env}`), // e.g. .env.my-org + join(baseDir, `.env.${env}.local`), // e.g. .env.my-org.local (local overrides) join(baseDir, ".env.local"), // .env.local (always loaded last) ]; @@ -223,7 +249,9 @@ export const VAPI_BASE_URL = process.env.VAPI_BASE_URL || "https://api.vapi.ai"; if (!VAPI_TOKEN) { console.error("❌ VAPI_TOKEN environment variable is required"); - console.error(" Create a .env.dev file with: VAPI_TOKEN=your-token"); + console.error( + ` Create a .env.${VAPI_ENV} file with: VAPI_TOKEN=your-token`, + ); process.exit(1); } diff --git a/src/credentials.ts b/src/credentials.ts index e428066..8b86ebc 100644 --- a/src/credentials.ts +++ b/src/credentials.ts @@ -14,7 +14,6 @@ import type { StateFile } from "./types.ts"; // enum values with UUIDs and break POST validation. // ───────────────────────────────────────────────────────────────────────────── -// Build UUID → name reverse map from state.credentials export function credentialReverseMap(state: StateFile): Map { const map = new Map(); for (const [name, uuid] of Object.entries(state.credentials)) { @@ -23,7 +22,6 @@ export function credentialReverseMap(state: StateFile): Map { return map; } -// Build name → UUID forward map from state.credentials export function credentialForwardMap(state: StateFile): Map { const map = new Map(); for (const [name, uuid] of Object.entries(state.credentials)) { @@ -34,7 +32,7 @@ export function credentialForwardMap(state: StateFile): Map { // ───────────────────────────────────────────────────────────────────────────── // Scoped walker: replace values only at `credentialId` / `credentialIds` keys. -// Works at any depth in any object/array structure. +// Works at any depth in any object/array structure. Cycle-safe via WeakSet. // ───────────────────────────────────────────────────────────────────────────── export function replaceCredentialRefs( diff --git a/src/eval.ts b/src/eval.ts index 7967060..2eac403 100644 --- a/src/eval.ts +++ b/src/eval.ts @@ -48,10 +48,12 @@ function printUsage(): void { console.error(" --stored Use stored assistantId/squadId from state instead of transient"); console.error(""); console.error("Examples:"); - console.error(" tsx src/eval.ts dev -s everblue-voice-squad-20374c37"); - console.error(" tsx src/eval.ts dev -a everblue-main-agent-633ab678 --filter name-collection"); - console.error(" tsx src/eval.ts dev -a resources/assistants/qa-address-resolution-tester-e9ed5d49.md"); - console.error(" tsx src/eval.ts dev -s everblue-voice-squad-20374c37 --stored"); + console.error(" npm run eval -- -s everblue-voice-squad-20374c37"); + console.error(" npm run eval -- -a everblue-main-agent-633ab678 --filter name-collection"); + console.error( + " npm run eval -- -a resources//assistants/qa-address-resolution-tester-e9ed5d49.md", + ); + console.error(" npm run eval -- -s everblue-voice-squad-20374c37 --stored"); } function parseArgs(): EvalConfig & { useStored: boolean } { @@ -494,8 +496,8 @@ async function main(): Promise { const evals = loadEvals(state, config.evalFilter); if (evals.length === 0) { console.error("❌ No evals found in state" + (config.evalFilter ? ` matching "${config.evalFilter}"` : "")); - console.error(" Push evals first: npm run push -- evals"); - console.error(" Eval files go in: resources/evals/"); + console.error(` Push evals first: npm run push -- ${config.env} evals`); + console.error(` Eval files go in: resources/${config.env}/evals/`); process.exit(1); } diff --git a/src/interactive.ts b/src/interactive.ts index bb9520c..c9df645 100644 --- a/src/interactive.ts +++ b/src/interactive.ts @@ -555,8 +555,16 @@ export async function runInteractivePull(): Promise { } if (scope === "all") { + // Local-first by default: preserve locally edited / deleted files + // unless the user explicitly opts into overwriting. + const overwriteLocal = await confirm({ + message: "Overwrite locally modified files?", + default: false, + }); console.log(c.dim("\n Pulling all resources...\n")); - spawnScript(["src/pull.ts", slug, "--force"]); + const args = ["src/pull.ts", slug]; + if (overwriteLocal) args.push("--force"); + spawnScript(args); console.log(c.green("\n Done!\n")); return; } @@ -667,18 +675,21 @@ export async function runInteractivePull(): Promise { byType.get(typeKey)!.push(uuid); } + // Local-first by default: preserve locally edited / deleted files + // unless the user explicitly opts into overwriting. + const overwriteLocal = await confirm({ + message: "Overwrite locally modified files?", + default: false, + }); + console.log(c.dim("\n Pulling...\n")); for (const [typeKey, uuids] of byType) { const idArgs = uuids.flatMap((id) => ["--id", id]); - spawnScript([ - "src/pull.ts", - slug, - "--force", - "--type", - typeKey, - ...idArgs, - ]); + const args = ["src/pull.ts", slug]; + if (overwriteLocal) args.push("--force"); + args.push("--type", typeKey, ...idArgs); + spawnScript(args); } console.log(c.green("\n Done!\n")); @@ -1013,6 +1024,9 @@ export async function runInteractiveCleanup(): Promise { } console.log(c.dim("\n Running cleanup with --force...\n")); - spawnScript(["src/cleanup.ts", slug, "--force"]); + // The confirm() prompt above is the user's explicit consent; pass --confirm + // so the destructive subprocess satisfies the same double-gate that + // the CLI direct path enforces. + spawnScript(["src/cleanup.ts", slug, "--force", "--confirm", slug]); console.log(c.green("\n Done!\n")); } diff --git a/src/pull.ts b/src/pull.ts index f7c9d6f..033feab 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -99,13 +99,23 @@ function gitHasCommits(): boolean { } // Returns relative paths of all locally modified, deleted, or untracked files. -// Uses `-z` (null-terminated) format so filenames containing spaces, newlines, -// or quotes parse correctly without the ad-hoc quote/arrow stripping that the -// plain porcelain format requires. With `-z`, renames emit two separate -// null-terminated records: `XY new\0old\0` — we want `new`, so we consume -// the record after any `R`/`C` status. +// +// Two flags matter here: +// +// - `--untracked-files=all`: by default, `git status --porcelain` collapses +// untracked directories to a single entry like `?? resources//`. In a +// fresh-clone workflow where the resource tree is not yet tracked, that +// collapsed entry would not match the per-file lookup downstream and locally +// edited files would silently get overwritten on pull. `=all` forces git to +// list each individual file. +// +// - `-z` (null-terminated): so filenames containing spaces, newlines, or +// quotes parse correctly without the ad-hoc quote/arrow stripping that the +// plain porcelain format requires. With `-z`, renames emit two separate +// null-terminated records: `XY new\0old\0` — we want `new`, so we consume +// the record after any `R`/`C` status. function getLocallyChangedFiles(): Set { - const status = gitCmd("status --porcelain -z"); + const status = gitCmd("status --porcelain --untracked-files=all -z"); const files = new Set(); const records = status.split("\0"); for (let i = 0; i < records.length; i++) { @@ -332,7 +342,9 @@ function removeUuidMappings( // Resource Processing // ───────────────────────────────────────────────────────────────────────────── -function cleanResource(resource: VapiResource): Record { +export function cleanResource( + resource: VapiResource, +): Record { const cleaned: Record = {}; // Preserve `null` values: the API uses `null` to represent an intentionally @@ -704,7 +716,17 @@ export async function pullResourceType( folderPath, `${resourceId}.yml`, ); - if (changedFiles.has(mdPath) || changedFiles.has(ymlPath)) { + const yamlPath = join( + "resources", + VAPI_ENV, + folderPath, + `${resourceId}.yaml`, + ); + if ( + changedFiles.has(mdPath) || + changedFiles.has(ymlPath) || + changedFiles.has(yamlPath) + ) { console.log(` ✏️ ${resourceId} (locally modified, preserving)`); newStateSection[resourceId] = resource.id; skipped++; @@ -712,20 +734,33 @@ export async function pullResourceType( } } - // Skip locally edited files even without git (mtime-based detection) - // If the resource file is newer than the state file, it was locally modified - if (!bootstrap && !force && !isNew && !changedFiles) { + // Skip locally edited files even without git (mtime-based detection). + // If the resource file is newer than the state file, it was locally + // modified after the last successful pull. This is the safety net for the + // fresh-clone case: if git either isn't enabled at all OR has nothing + // useful to say about the resource tree (untracked, no changes, etc.), + // fall through here. The bug we are guarding against was treating an + // empty `changedFiles` Set as "git already gave us the answer" — it has + // not, and the mtime check must run. + if ( + !bootstrap && + !force && + !isNew && + (!changedFiles || changedFiles.size === 0) + ) { const dir = join(RESOURCES_DIR, folderPath); - const localFile = - [join(dir, `${resourceId}.md`), join(dir, `${resourceId}.yml`), join(dir, `${resourceId}.yaml`)] - .find((p) => existsSync(p)); + const localFile = [ + join(dir, `${resourceId}.md`), + join(dir, `${resourceId}.yml`), + join(dir, `${resourceId}.yaml`), + ].find((p) => existsSync(p)); if (localFile) { const stateFilePath = join(BASE_DIR, `.vapi-state.${VAPI_ENV}.json`); if (existsSync(stateFilePath)) { const localMtime = statSync(localFile).mtimeMs; const stateMtime = statSync(stateFilePath).mtimeMs; if (localMtime > stateMtime) { - console.log(` ⏭️ ${resourceId} (locally modified, skipping)`); + console.log(` ✏️ ${resourceId} (locally modified, preserving)`); newStateSection[resourceId] = resource.id; skipped++; continue; diff --git a/src/push.ts b/src/push.ts index 8fe4dd6..55563da 100644 --- a/src/push.ts +++ b/src/push.ts @@ -96,6 +96,10 @@ async function upsertResourceWithStateRecovery(options: { const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +// Must stay in sync with `VALID_RESOURCE_TYPES` in `src/types.ts`. Used by +// `hasAnyLoadedResources`, `getTargetedResourceTypes`, and the credential / +// state-sanity checks — dropping a type here silently disables those +// pre-flight checks for that type. const ALL_RESOURCE_TYPES: ResourceType[] = [ "tools", "structuredOutputs", @@ -105,6 +109,7 @@ const ALL_RESOURCE_TYPES: ResourceType[] = [ "scenarios", "simulations", "simulationSuites", + "evals", ]; function warnUnresolvedCredentials( @@ -610,12 +615,31 @@ function isPartialApply(): boolean { ); } +// Match a CLI-supplied path against a folder name. Common shapes the user +// can pass: +// - `resources//assistants/foo.yml` +// - `./resources//assistants/foo.yml` +// - `assistants/foo.yml` ← short form +// - `assistants/support/intake.yml` ← short form with subdir +// - absolute path +// The previous version only matched `//` (with a leading slash), +// so the short form silently no-op'd: the type was never loaded into the +// apply pipeline and the more permissive `filterResourcesByPaths` was +// never even consulted. +export function pathMatchesFolder(filePath: string, folder: string): boolean { + return ( + filePath === folder || + filePath.startsWith(`${folder}/`) || + filePath.startsWith(`${folder}\\`) || + filePath.includes(`/${folder}/`) || + filePath.includes(`\\${folder}\\`) + ); +} + function shouldApplyResourceType(type: ResourceType): boolean { if (APPLY_FILTER.filePaths?.length) { const folder = FOLDER_MAP[type]; - return APPLY_FILTER.filePaths.some( - (fp) => fp.includes(`/${folder}/`) || fp.includes(`\\${folder}\\`), - ); + return APPLY_FILTER.filePaths.some((fp) => pathMatchesFolder(fp, folder)); } if (APPLY_FILTER.resourceTypes?.length) { return APPLY_FILTER.resourceTypes.includes(type); @@ -1258,6 +1282,9 @@ async function main(): Promise { "\n⚠️ Failed to persist state file after apply:", saveError instanceof Error ? saveError.message : saveError, ); + console.error( + ` Local state may be out of sync with platform. Run \`npm run pull -- ${VAPI_ENV} --bootstrap\` to recover.`, + ); } } } diff --git a/src/resources.ts b/src/resources.ts index d64cecd..17963b8 100644 --- a/src/resources.ts +++ b/src/resources.ts @@ -125,8 +125,8 @@ export async function loadResources( const ext = extname(filePath); // Compute resourceId as path relative to the resource type directory, without extension - // e.g., /resources/assistants/support/intake.yml → support/intake - // e.g., /resources/assistants/inbound-support.yml → inbound-support (backwards compatible) + // e.g., /resources//assistants/support/intake.yml → support/intake + // e.g., /resources//assistants/inbound-support.yml → inbound-support const relativePath = relative(resourceDir, filePath); const resourceId = relativePath.slice(0, -ext.length); diff --git a/tests/clean-resource.test.ts b/tests/clean-resource.test.ts new file mode 100644 index 0000000..9fe727f --- /dev/null +++ b/tests/clean-resource.test.ts @@ -0,0 +1,92 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// Regression tests for P0-3. +// +// pull.ts depends on config.ts which calls process.exit(1) at module load +// time if VAPI_TOKEN is not set or if argv[2] is not a valid slug. Set both +// before dynamic-importing the module under test. +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { cleanResource } = await import("../src/pull.ts"); + +test("cleanResource strips the EXCLUDED_FIELDS (id, orgId, createdAt, etc.)", () => { + const out = cleanResource({ + id: "uuid-1234", + orgId: "org-1", + createdAt: "2026-01-01", + updatedAt: "2026-01-02", + isDeleted: false, + name: "support-bot", + }); + assert.equal(out.id, undefined); + assert.equal(out.orgId, undefined); + assert.equal(out.createdAt, undefined); + assert.equal(out.updatedAt, undefined); + assert.equal(out.isDeleted, undefined); + assert.equal(out.name, "support-bot"); +}); + +test("cleanResource strips undefined values", () => { + const out = cleanResource({ + id: "uuid-1234", + name: "support-bot", + voicemailMessage: undefined, + }); + assert.ok(!("voicemailMessage" in out)); +}); + +test( + "P0-3 regression: cleanResource MUST preserve null. The Vapi API uses null " + + "to represent intentionally cleared fields (voicemailMessage, " + + "endCallMessage, etc.). Stripping null on pull would cause the next push " + + "to drop the clear and re-apply any prior value still on the server.", + () => { + const out = cleanResource({ + id: "uuid-1234", + name: "support-bot", + voicemailMessage: null, + endCallMessage: null, + analysisPlan: { summaryPlan: { messages: null } }, + }); + assert.equal( + out.voicemailMessage, + null, + "voicemailMessage: null must be preserved", + ); + assert.equal( + out.endCallMessage, + null, + "endCallMessage: null must be preserved", + ); + // null nested in objects is preserved by JS structural copy automatically; + // we just verify the parent object is not stripped. + assert.deepEqual(out.analysisPlan, { summaryPlan: { messages: null } }); + }, +); + +test("cleanResource preserves nested structures verbatim", () => { + const out = cleanResource({ + id: "uuid-1234", + name: "support-bot", + voice: { + provider: "cartesia", + voiceId: "abc-123", + generationConfig: { speed: 1.0 }, + }, + members: [ + { assistantId: "child-1" }, + { assistantId: "child-2" }, + ], + }); + assert.deepEqual(out.voice, { + provider: "cartesia", + voiceId: "abc-123", + generationConfig: { speed: 1.0 }, + }); + assert.deepEqual(out.members, [ + { assistantId: "child-1" }, + { assistantId: "child-2" }, + ]); +}); diff --git a/tests/cleanup-safety.test.ts b/tests/cleanup-safety.test.ts new file mode 100644 index 0000000..897b6ad --- /dev/null +++ b/tests/cleanup-safety.test.ts @@ -0,0 +1,183 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { + mkdtempSync, + writeFileSync, + rmSync, + cpSync, + symlinkSync, +} from "node:fs"; +import { join, dirname } from "node:path"; +import { tmpdir } from "node:os"; +import { fileURLToPath } from "node:url"; + +// Regression tests for P0-4. +// +// The branch `feat/optimization-gitops-flow` removed both pre-existing +// safety gates from cleanup.ts: +// 1. `--confirm ` double-gate — so a stray `--force` from another +// command can't go destructive. +// 2. Empty-state refusal — so a fresh clone or corrupted state file can't +// be misread as "all remote resources are orphaned" → wipe the org. +// +// These integration tests spawn `tsx src/cleanup.ts` against a tmp working +// directory (with node_modules symlinked from the real repo) and confirm +// the safety gates short-circuit BEFORE any API call is made. + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = join(__dirname, ".."); + +interface Fixture { + dir: string; + cleanup: () => void; +} + +function setupFixture(stateContent: object | null): Fixture { + const dir = mkdtempSync(join(tmpdir(), "vapi-cleanup-test-")); + // Copy the source tree so cleanup.ts's relative imports work and BASE_DIR + // points into the tmp dir (where we control the state file). + cpSync(join(REPO_ROOT, "src"), join(dir, "src"), { recursive: true }); + cpSync(join(REPO_ROOT, "package.json"), join(dir, "package.json")); + // node_modules is too big to copy; symlink it instead. + symlinkSync(join(REPO_ROOT, "node_modules"), join(dir, "node_modules"), "dir"); + writeFileSync( + join(dir, ".env.test-cleanup-org"), + "VAPI_TOKEN=fake-token-not-used\n", + ); + if (stateContent !== null) { + writeFileSync( + join(dir, ".vapi-state.test-cleanup-org.json"), + JSON.stringify(stateContent), + ); + } + return { + dir, + cleanup: () => rmSync(dir, { recursive: true, force: true }), + }; +} + +function emptyState() { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +function nonEmptyState() { + return { + ...emptyState(), + assistants: { foo: "11111111-1111-1111-1111-111111111111" }, + }; +} + +function runCleanup( + cwd: string, + args: string[], +): { code: number | null; stdout: string; stderr: string } { + const result = spawnSync( + "node", + ["--import", "tsx", "src/cleanup.ts", "test-cleanup-org", ...args], + { + cwd, + env: { ...process.env, VAPI_TOKEN: "fake-token-not-used" }, + encoding: "utf-8", + timeout: 20_000, + }, + ); + return { + code: result.status, + stdout: result.stdout || "", + stderr: result.stderr || "", + }; +} + +test( + "P0-4 regression: cleanup --force WITHOUT --confirm refuses to run", + () => { + const fx = setupFixture(nonEmptyState()); + try { + const res = runCleanup(fx.dir, ["--force"]); + assert.notEqual(res.code, 0, `must exit non-zero, got ${res.code}`); + assert.match( + res.stderr, + /Refusing to run destructive cleanup without explicit confirmation/, + ); + assert.doesNotMatch( + res.stdout, + /Deleting\.\.\./, + "must NOT begin deletion", + ); + } finally { + fx.cleanup(); + } + }, +); + +test( + "P0-4 regression: cleanup --force --confirm refuses to run", + () => { + const fx = setupFixture(nonEmptyState()); + try { + const res = runCleanup(fx.dir, ["--force", "--confirm", "different-org"]); + assert.notEqual(res.code, 0); + assert.match( + res.stderr, + /Refusing to run destructive cleanup without explicit confirmation/, + ); + } finally { + fx.cleanup(); + } + }, +); + +test( + "P0-4 regression: cleanup --force --confirm with EMPTY state refuses", + () => { + // Fresh-clone scenario: state file exists but is empty. Every remote + // resource would be treated as orphaned. Must refuse. + const fx = setupFixture(emptyState()); + try { + const res = runCleanup(fx.dir, [ + "--force", + "--confirm", + "test-cleanup-org", + ]); + assert.notEqual(res.code, 0); + assert.match( + res.stderr, + /Refusing to run destructive cleanup: state file has 0 tracked resources/, + ); + } finally { + fx.cleanup(); + } + }, +); + +test( + "cleanup dry-run (default, no --force) is allowed without --confirm — it " + + "never deletes anything regardless of state contents", + () => { + const fx = setupFixture(emptyState()); + try { + const res = runCleanup(fx.dir, []); + // Dry-run will fail when it tries to call the (fake) API, but the + // safety refusal must NOT have triggered. + assert.doesNotMatch( + res.stderr, + /Refusing to run destructive cleanup/, + "dry-run must not be blocked by the destructive safety gates", + ); + } finally { + fx.cleanup(); + } + }, +); diff --git a/tests/cli-arg-parsing.test.ts b/tests/cli-arg-parsing.test.ts new file mode 100644 index 0000000..684d176 --- /dev/null +++ b/tests/cli-arg-parsing.test.ts @@ -0,0 +1,157 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { + mkdtempSync, + writeFileSync, + rmSync, + cpSync, + symlinkSync, +} from "node:fs"; +import { join, dirname } from "node:path"; +import { tmpdir } from "node:os"; +import { fileURLToPath } from "node:url"; + +// Regression tests for P0-7 (bare-id refusal half). +// +// The branch dropped a bare resource id like `npm run push -- foo` +// silently — the arg didn't match any type, didn't have a `/` or extension, +// and was simply discarded. With nothing to filter, the engine then ran a +// FULL apply with full orphan-deletion check against the state. With +// `--force` that could wipe every state-tracked resource not on disk. +// +// These tests pin the new strict behavior: any unrecognized positional arg +// is rejected at parse time so the user can't accidentally trigger a full +// apply when they meant a partial. + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = join(__dirname, ".."); + +interface Fixture { + dir: string; + cleanup: () => void; +} + +function setupFixture(): Fixture { + const dir = mkdtempSync(join(tmpdir(), "vapi-cli-arg-test-")); + cpSync(join(REPO_ROOT, "src"), join(dir, "src"), { recursive: true }); + cpSync(join(REPO_ROOT, "package.json"), join(dir, "package.json")); + symlinkSync(join(REPO_ROOT, "node_modules"), join(dir, "node_modules"), "dir"); + writeFileSync( + join(dir, ".env.test-cli-arg-org"), + "VAPI_TOKEN=fake-token-not-used\n", + ); + return { + dir, + cleanup: () => rmSync(dir, { recursive: true, force: true }), + }; +} + +function runPush( + cwd: string, + extraArgs: string[], +): { code: number | null; stdout: string; stderr: string } { + const result = spawnSync( + "node", + ["--import", "tsx", "src/push.ts", "test-cli-arg-org", ...extraArgs], + { + cwd, + env: { ...process.env, VAPI_TOKEN: "fake-token-not-used" }, + encoding: "utf-8", + timeout: 20_000, + }, + ); + return { + code: result.status, + stdout: result.stdout || "", + stderr: result.stderr || "", + }; +} + +test( + "P0-7 regression: bare resource id (no slash, no extension) is rejected " + + "with an explicit error — must NOT silently fall through to a full apply", + () => { + const fx = setupFixture(); + try { + const res = runPush(fx.dir, ["foo"]); + assert.notEqual(res.code, 0, `must exit non-zero, got ${res.code}`); + assert.match(res.stderr, /Unrecognized argument: foo/); + // The push pipeline must NOT have started. + assert.doesNotMatch(res.stdout, /Loading resources/); + } finally { + fx.cleanup(); + } + }, +); + +test("misspelled resource type (e.g. assistnts) is rejected", () => { + const fx = setupFixture(); + try { + const res = runPush(fx.dir, ["assistnts"]); + assert.notEqual(res.code, 0); + assert.match(res.stderr, /Unrecognized argument: assistnts/); + } finally { + fx.cleanup(); + } +}); + +test("recognized positional resource type is accepted (does not error at parse)", () => { + const fx = setupFixture(); + try { + const res = runPush(fx.dir, ["assistants", "--bootstrap"]); + // Parse-time error would print "Unrecognized argument" to stderr. The + // command may still fail later when it tries to talk to the (fake) API, + // but the parse step must succeed. + assert.doesNotMatch( + res.stderr, + /Unrecognized argument/, + "valid type arg must not be flagged", + ); + } finally { + fx.cleanup(); + } +}); + +test("file-path arg (with extension) is accepted at parse time", () => { + const fx = setupFixture(); + try { + const res = runPush(fx.dir, ["assistants/foo.yml", "--bootstrap"]); + assert.doesNotMatch(res.stderr, /Unrecognized argument/); + } finally { + fx.cleanup(); + } +}); + +test("file-path arg in long form is accepted at parse time", () => { + const fx = setupFixture(); + try { + const res = runPush(fx.dir, [ + "resources/test-cli-arg-org/assistants/foo.yml", + "--bootstrap", + ]); + assert.doesNotMatch(res.stderr, /Unrecognized argument/); + } finally { + fx.cleanup(); + } +}); + +test( + "--confirm is forwarded through parseFlags without tripping the " + + "unrecognized-arg refusal (cleanup.ts consumes --confirm directly)", + () => { + const fx = setupFixture(); + try { + const res = runPush(fx.dir, [ + "--confirm", + "test-cli-arg-org", + "--bootstrap", + ]); + // The slug after --confirm must be EATEN by parseFlags so it doesn't + // get treated as a positional arg → "Unrecognized argument: test-cli-arg-org" + assert.doesNotMatch(res.stderr, /Unrecognized argument/); + } finally { + fx.cleanup(); + } + }, +); diff --git a/tests/credentials.test.ts b/tests/credentials.test.ts new file mode 100644 index 0000000..a4c1ab9 --- /dev/null +++ b/tests/credentials.test.ts @@ -0,0 +1,219 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { replaceCredentialRefs } from "../src/credentials.ts"; +import type { StateFile } from "../src/types.ts"; + +// Regression tests for P0-1. +// +// The branch `feat/optimization-gitops-flow` shipped a `deepReplaceValues` +// helper that walked every string in a payload and swapped any value matching +// a credential slug. Combined with the auto-slugified credential names from +// `pullCredentials` (which slugifies provider names like `openai`, `11labs`, +// `langfuse`), that meant every `provider: openai`, `voice.provider: 11labs`, +// `observabilityPlan.provider: langfuse` got rewritten to a UUID, which the +// API then rejects on POST/PATCH. These tests lock in the scoped semantics +// (only swap at exactly `credentialId` / `credentialIds` keys). + +function makeState(creds: Record): StateFile { + return { + credentials: creds, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +function reverseMap(state: StateFile): Map { + const m = new Map(); + for (const [name, uuid] of Object.entries(state.credentials)) { + m.set(uuid, name); + } + return m; +} + +function forwardMap(state: StateFile): Map { + const m = new Map(); + for (const [name, uuid] of Object.entries(state.credentials)) { + m.set(name, uuid); + } + return m; +} + +test("replaceCredentialRefs swaps at credentialId keys", () => { + const state = makeState({ + "roofr-server-credential": "11111111-1111-1111-1111-111111111111", + }); + const input = { + server: { + url: "https://example.com", + credentialId: "roofr-server-credential", + }, + }; + const out = replaceCredentialRefs(input, forwardMap(state)); + assert.equal( + (out.server as { credentialId: string }).credentialId, + "11111111-1111-1111-1111-111111111111", + ); +}); + +test("replaceCredentialRefs swaps each entry of credentialIds arrays", () => { + const state = makeState({ + "cred-a": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + "cred-b": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", + }); + const input = { + model: { + credentialIds: ["cred-a", "cred-b", "unknown-cred"], + }, + }; + const out = replaceCredentialRefs(input, forwardMap(state)); + assert.deepEqual((out.model as { credentialIds: string[] }).credentialIds, [ + "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", + "unknown-cred", + ]); +}); + +test( + "P0-1 regression: replaceCredentialRefs does NOT touch model.provider, " + + "voice.provider, observabilityPlan.provider — even when the slug exactly " + + "matches a credential name like `openai`", + () => { + const state = makeState({ + openai: "00000000-0000-4000-a000-000000000001", + "11labs": "00000000-0000-4000-a000-000000000002", + langfuse: "00000000-0000-4000-a000-000000000003", + anthropic: "00000000-0000-4000-a000-000000000004", + deepgram: "00000000-0000-4000-a000-000000000005", + }); + const input = { + model: { + provider: "openai", + model: "gpt-4o", + credentialId: "openai", + }, + voice: { + provider: "11labs", + voiceId: "rachel", + credentialId: "11labs", + }, + transcriber: { + provider: "deepgram", + model: "nova-2", + credentialId: "deepgram", + }, + observabilityPlan: { + provider: "langfuse", + credentialId: "langfuse", + }, + analysisPlan: { + provider: "anthropic", + credentialId: "anthropic", + }, + }; + const out = replaceCredentialRefs(input, forwardMap(state)) as typeof input; + + // Provider enums must NOT be swapped. + assert.equal(out.model.provider, "openai"); + assert.equal(out.voice.provider, "11labs"); + assert.equal(out.transcriber.provider, "deepgram"); + assert.equal(out.observabilityPlan.provider, "langfuse"); + assert.equal(out.analysisPlan.provider, "anthropic"); + + // credentialId values, on the other hand, MUST be swapped. + assert.equal(out.model.credentialId, "00000000-0000-4000-a000-000000000001"); + assert.equal(out.voice.credentialId, "00000000-0000-4000-a000-000000000002"); + assert.equal( + out.transcriber.credentialId, + "00000000-0000-4000-a000-000000000005", + ); + assert.equal( + out.observabilityPlan.credentialId, + "00000000-0000-4000-a000-000000000003", + ); + assert.equal( + out.analysisPlan.credentialId, + "00000000-0000-4000-a000-000000000004", + ); + }, +); + +test("replaceCredentialRefs is symmetric: reverse map restores original names", () => { + const state = makeState({ + "my-langfuse": "99999999-9999-9999-9999-999999999999", + }); + const fwd = forwardMap(state); + const rev = reverseMap(state); + + const original = { + observabilityPlan: { + provider: "langfuse", + credentialId: "my-langfuse", + }, + }; + const pushed = replaceCredentialRefs(original, fwd); + const pulled = replaceCredentialRefs(pushed, rev); + assert.deepEqual(pulled, original); +}); + +test("replaceCredentialRefs walks deeply nested structures", () => { + const state = makeState({ "deep-cred": "deadbeef-dead-beef-dead-beefdeadbeef" }); + const input = { + members: [ + { + assistant: { + model: { + tools: [ + { name: "transferCall", credentialId: "deep-cred" }, + { name: "endCall" }, + ], + }, + }, + }, + ], + }; + const out = replaceCredentialRefs(input, forwardMap(state)) as typeof input; + assert.equal( + out.members[0]!.assistant.model.tools[0]!.credentialId, + "deadbeef-dead-beef-dead-beefdeadbeef", + ); +}); + +test("replaceCredentialRefs is a no-op when replacements map is empty", () => { + const input = { credentialId: "openai", provider: "openai" }; + const out = replaceCredentialRefs(input, new Map()); + assert.deepEqual(out, input); +}); + +test("replaceCredentialRefs preserves non-plain-object values (Date, Buffer)", () => { + const state = makeState({ x: "yyy" }); + const date = new Date("2026-04-20T00:00:00Z"); + const input = { + credentialId: "x", + createdAt: date, + payload: Buffer.from("hello"), + }; + const out = replaceCredentialRefs(input, forwardMap(state)) as typeof input; + assert.equal(out.credentialId, "yyy"); + assert.equal(out.createdAt, date, "Date instance must pass through unchanged"); + assert.ok( + Buffer.isBuffer(out.payload), + "Buffer instance must pass through unchanged", + ); +}); + +test("replaceCredentialRefs handles cyclic structures without infinite recursion", () => { + const state = makeState({ x: "yyy" }); + const a: Record = { credentialId: "x" }; + const b: Record = { partner: a }; + a.partner = b; + // Should not stack-overflow. + const out = replaceCredentialRefs(a, forwardMap(state)) as typeof a; + assert.equal(out.credentialId, "yyy"); +}); diff --git a/tests/path-matching.test.ts b/tests/path-matching.test.ts new file mode 100644 index 0000000..630987f --- /dev/null +++ b/tests/path-matching.test.ts @@ -0,0 +1,138 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// Regression tests for P0-7. +// +// push.ts depends on config.ts which calls process.exit(1) at module load +// time if VAPI_TOKEN is not set or if argv[2] is not a valid slug. Set both +// before dynamic-importing the module under test. +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { pathMatchesFolder } = await import("../src/push.ts"); + +// The bug: +// shouldApplyResourceType used `fp.includes("/" + folder + "/")`, which +// requires a leading slash. So `assistants/foo.yml` (the natural CLI short +// form documented in AGENTS.md) silently no-op'd: +// - shouldApplyResourceType returned false → the type was never loaded +// - filterResourcesByPaths was never even consulted +// - exit code was 0, "Applied 0 resource(s)" +// These tests pin the behavior across the path shapes a user can pass. + +test("matches a long-form path (resources///file.yml)", () => { + assert.equal( + pathMatchesFolder( + "resources/my-org/assistants/support-bot.yml", + "assistants", + ), + true, + ); +}); + +test("matches a long-form path with leading ./", () => { + assert.equal( + pathMatchesFolder( + "./resources/my-org/assistants/support-bot.yml", + "assistants", + ), + true, + ); +}); + +test("matches an absolute path", () => { + assert.equal( + pathMatchesFolder( + "/Users/dev/work/gitops/resources/my-org/assistants/support-bot.yml", + "assistants", + ), + true, + ); +}); + +test( + "P0-7 regression: matches a SHORT-form path (folder/file.yml) — this " + + "previously silently no-op'd because the matcher required a leading slash", + () => { + assert.equal( + pathMatchesFolder("assistants/support-bot.yml", "assistants"), + true, + ); + }, +); + +test("P0-7 regression: matches a short-form path with subdirectory", () => { + assert.equal( + pathMatchesFolder("assistants/support/intake.yml", "assistants"), + true, + ); +}); + +test("matches a short-form path for nested folders (simulations/personalities)", () => { + assert.equal( + pathMatchesFolder( + "simulations/personalities/rude-customer.yml", + "simulations/personalities", + ), + true, + ); + assert.equal( + pathMatchesFolder( + "resources/my-org/simulations/personalities/rude-customer.yml", + "simulations/personalities", + ), + true, + ); +}); + +test("matches Windows-style short-form paths (assistants\\foo.yml)", () => { + assert.equal( + pathMatchesFolder("assistants\\support-bot.yml", "assistants"), + true, + ); +}); + +test("matches Windows-style long-form paths", () => { + assert.equal( + pathMatchesFolder( + "resources\\my-org\\assistants\\support-bot.yml", + "assistants", + ), + true, + ); +}); + +test("rejects an unrelated folder", () => { + assert.equal( + pathMatchesFolder("tools/transferCall.yml", "assistants"), + false, + ); + assert.equal( + pathMatchesFolder("resources/my-org/tools/transferCall.yml", "assistants"), + false, + ); +}); + +test("rejects a path that contains the folder name as a substring of another segment", () => { + // `assistants_legacy` should NOT match `assistants` because the segment + // boundary is enforced. + assert.equal( + pathMatchesFolder("assistants_legacy/foo.yml", "assistants"), + false, + ); + assert.equal( + pathMatchesFolder( + "resources/my-org/assistants_legacy/foo.yml", + "assistants", + ), + false, + ); +}); + +test("matches the bare folder name itself", () => { + // `npm run push -- assistants` (positional resource type) is parsed + // by config.ts as a `resourceTypes` filter, not a `filePaths` filter, so in + // practice this path won't be hit. But document the behavior anyway: bare + // folder name matches. + assert.equal(pathMatchesFolder("assistants", "assistants"), true); +});