From a32bc5c0b4239ec0b78e3f0b6f429b7b50ef657e Mon Sep 17 00:00:00 2001 From: Scott Lowe Date: Sat, 3 Oct 2026 00:13:17 -0700 Subject: [PATCH] fix(cleanup): never delete resources matched by .vapi-ignore .vapi-ignore marks platform resources a repository must not manage, and pull never writes them to state. cleanup treats every platform resource missing from state as an orphan and didn't read .vapi-ignore, so a destructive cleanup deleted exactly the resources a team had excluded, such as another team's assistants in a shared org. Push already orphan-protects them. cleanup now loads the org's ignore patterns and keeps every orphan that matches, checking the IDs pull would give the resource (its name slug, with and without the UUID suffix). Kept resources are listed as retained in both dry and destructive runs. tests/cleanup-ignore.test.ts runs the real cleanup command against a stub API: before this change a destructive run deleted an ignored assistant and tool along with the true orphan; now only the orphan. Docs: AGENTS.md, the workflows guide and .vapi-ignore.example describe the fixed behaviour; improvements.md #36 is resolved. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 10 +-- docs/guides/workflows.md | 9 +- improvements.md | 21 +++-- resources/.vapi-ignore.example | 2 + src/cleanup.ts | 93 ++++++++++++++++++-- tests/cleanup-ignore.test.ts | 150 +++++++++++++++++++++++++++++++++ 6 files changed, 257 insertions(+), 28 deletions(-) create mode 100644 tests/cleanup-ignore.test.ts diff --git a/AGENTS.md b/AGENTS.md index 40a790b..aa5e47f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -229,13 +229,9 @@ The engine resolves IDs and credential names to each org's UUIDs on push. `resources//.vapi-ignore` lists platform resources this repo must not manage, as gitignore-style patterns (see `resources/.vapi-ignore.example`). -Matched resources are skipped on pull and push, and push never deletes them. -A resource that references an ignored one is a validation error. - -**`npm run cleanup` does not read `.vapi-ignore`.** Ignored resources are -never in the state file, so cleanup lists them as orphans and a destructive -run would delete them. Check its dry-run list against `.vapi-ignore` with the -human before any `--force` run. +Matched resources are skipped on pull and push, and neither push nor +`npm run cleanup` deletes them. A resource that references an ignored one is a +validation error. --- diff --git a/docs/guides/workflows.md b/docs/guides/workflows.md index 77fff90..14660d2 100644 --- a/docs/guides/workflows.md +++ b/docs/guides/workflows.md @@ -186,11 +186,10 @@ npm run cleanup -- npm run cleanup -- --force --confirm ``` -**Check the dry-run list before a destructive run.** Cleanup treats every -platform resource that isn't in `.vapi-state..json` as an orphan, and it -does not read `.vapi-ignore`: resources you've excluded with `.vapi-ignore` -are never in the state file, so they appear in the list and `--force` would -delete them. +Cleanup treats every platform resource that isn't in +`.vapi-state..json` as an orphan, except those matched by +`resources//.vapi-ignore`: it lists those as retained and never deletes +them. Still read the dry-run list before a destructive run. **When the list includes Vapi's built-in fixtures** (for example the stock simulation personalities, which can't be deleted — see diff --git a/improvements.md b/improvements.md index 36ebe72..7bfad7d 100644 --- a/improvements.md +++ b/improvements.md @@ -87,7 +87,7 @@ you which stack PR closes the row.** | 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 | | 35 | A failed promotion pushed nothing, not even state | git lost track of resources already on the platform | None | RESOLVED 2026-10-01 | -| 36 | `cleanup` deletes resources excluded by `.vapi-ignore` | A destructive cleanup can delete resources another team owns | None | Open | +| 36 | `cleanup` deletes resources excluded by `.vapi-ignore` | A destructive cleanup can delete resources another team owns | None | RESOLVED 2026-10-03 | **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. @@ -1888,6 +1888,8 @@ None needed once the fix below lands. ## 36. `cleanup` deletes resources excluded by `.vapi-ignore` +**[RESOLVED 2026-10-03]** + **Discovered:** 2026-10-03, while reviewing the agent instructions for the public release. ### Problem @@ -1914,19 +1916,20 @@ another team deletes that team's assistants, tools or squads. ### Current mitigation -The default run is a dry run, and the destructive run needs `--confirm `. -The docs and `AGENTS.md` now tell people and agents to check the dry-run list -against `.vapi-ignore` first. +None needed once the fix below lands. -### Possible fix +### Possible fix (landed) -Load the org's ignore patterns in `cleanup.ts` and exclude matches from the -deletion list (printing them as retained, as push does), with a test that a -destructive cleanup keeps an ignored resource. +`src/cleanup.ts` loads the org's ignore patterns and keeps every orphan that +matches one, checking the IDs pull would give the resource (its name slug, +with and without the UUID suffix). Kept resources are listed as retained. +`tests/cleanup-ignore.test.ts` runs a destructive cleanup against a stub API: +before the fix it deleted an ignored assistant and tool along with the true +orphan; after it, only the orphan. ### Status -Open. +**RESOLVED 2026-10-03.** --- diff --git a/resources/.vapi-ignore.example b/resources/.vapi-ignore.example index 3f63ccd..dbaca6e 100644 --- a/resources/.vapi-ignore.example +++ b/resources/.vapi-ignore.example @@ -10,6 +10,8 @@ # id matches `.vapi-ignore`. # - `--force` on push bypasses the load-filter (so a deliberate override # can flow through) BUT the orphan-protect still applies. +# - `npm run cleanup -- ` never deletes a matched resource, even with +# `--force --confirm `; it lists it as retained. # - A resource that references an ignored resource (e.g. a squad # pointing at `assistants/foo` while `assistants/foo` is ignored) is # a validation ERROR — `--strict` push aborts before any API call. diff --git a/src/cleanup.ts b/src/cleanup.ts index 2492ca1..13723c7 100644 --- a/src/cleanup.ts +++ b/src/cleanup.ts @@ -1,7 +1,16 @@ import { resolve } from "path"; import { fileURLToPath } from "url"; -import { VAPI_BASE_URL, VAPI_ENV, VAPI_TOKEN } from "./config.ts"; +import { + loadIgnorePatterns, + matchesIgnore, + VAPI_BASE_URL, + VAPI_ENV, + VAPI_TOKEN, +} from "./config.ts"; +import { FOLDER_MAP } from "./resource-parse.ts"; +import { slugify } from "./slug-utils.ts"; import { loadState } from "./state.ts"; +import type { ResourceType } from "./types.ts"; // ───────────────────────────────────────────────────────────────────────────── // Dangerous Sync - Delete everything NOT in state file @@ -68,6 +77,29 @@ async function vapiDelete(endpoint: string): Promise { interface VapiResource { id: string; name?: string; + function?: { name?: string }; +} + +// The .vapi-ignore pattern a platform resource matches, or null. Checks the +// ids pull would give it — its name slug with the UUID suffix (what pull +// writes for an untracked resource) and without it — so a pattern written +// against either form protects the resource. +function ignoredBy( + folder: string, + resource: VapiResource, + patterns: string[], +): string | null { + if (patterns.length === 0) return null; + const name = resource.name ?? resource.function?.name; + const shortId = resource.id.slice(0, 8); + const ids = name + ? [`${slugify(name)}-${shortId}`, slugify(name)] + : [`resource-${shortId}`]; + for (const id of ids) { + const matched = matchesIgnore(folder, id, patterns); + if (matched) return matched; + } + return null; } function readConfirmToken(argv: string[]): string | undefined { @@ -149,43 +181,75 @@ async function main(): Promise { }[] = []; // Fetch and compare each resource type - const resourceTypes = [ + const resourceTypes: Array<{ + type: ResourceType; + name: string; + endpoint: string; + deleteEndpoint: string; + }> = [ { + type: "assistants", name: "assistants", endpoint: "/assistant", deleteEndpoint: "/assistant", }, - { name: "tools", endpoint: "/tool", deleteEndpoint: "/tool" }, { + type: "tools", + name: "tools", + endpoint: "/tool", + deleteEndpoint: "/tool", + }, + { + type: "structuredOutputs", name: "structured outputs", endpoint: "/structured-output", deleteEndpoint: "/structured-output", }, - { name: "squads", endpoint: "/squad", deleteEndpoint: "/squad" }, { + type: "squads", + name: "squads", + endpoint: "/squad", + deleteEndpoint: "/squad", + }, + { + type: "personalities", name: "personalities", endpoint: "/eval/simulation/personality", deleteEndpoint: "/eval/simulation/personality", }, { + type: "scenarios", name: "scenarios", endpoint: "/eval/simulation/scenario", deleteEndpoint: "/eval/simulation/scenario", }, { + type: "simulations", name: "simulations", endpoint: "/eval/simulation", deleteEndpoint: "/eval/simulation", }, { + type: "simulationSuites", name: "simulation suites", endpoint: "/eval/simulation/suite", deleteEndpoint: "/eval/simulation/suite", }, - { name: "evals", endpoint: "/eval", deleteEndpoint: "/eval" }, + { + type: "evals", + name: "evals", + endpoint: "/eval", + deleteEndpoint: "/eval", + }, ]; - for (const { name, endpoint, deleteEndpoint } of resourceTypes) { + // Resources this repo must not manage (.vapi-ignore) are never written to + // state, so every one of them would look like an orphan here. Keep them, + // as push's orphan-protection does. + const ignorePatterns = loadIgnorePatterns(); + let retained = 0; + + for (const { type, name, endpoint, deleteEndpoint } of resourceTypes) { console.log(`📥 Fetching ${name}...`); try { // Enable debug for structured outputs to see response format @@ -202,7 +266,16 @@ async function main(): Promise { continue; } - const orphans = resources.filter((r) => !stateIds.has(r.id)); + const orphans: VapiResource[] = []; + for (const r of resources.filter((r) => !stateIds.has(r.id))) { + const matched = ignoredBy(FOLDER_MAP[type], r, ignorePatterns); + if (matched) { + console.log( + ` 🚫 ${r.name || r.function?.name || r.id} retained (matched .vapi-ignore: ${matched})`, + ); + retained++; + } else orphans.push(r); + } if (orphans.length > 0) { console.log( @@ -228,6 +301,12 @@ async function main(): Promise { "\n═══════════════════════════════════════════════════════════════", ); + if (retained > 0) { + console.log( + `\n🚫 ${retained} resource(s) not in state were kept because they match .vapi-ignore`, + ); + } + if (toDelete.length === 0) { console.log("✅ Nothing to delete - all resources match state file\n"); return; diff --git a/tests/cleanup-ignore.test.ts b/tests/cleanup-ignore.test.ts new file mode 100644 index 0000000..ae2ad31 --- /dev/null +++ b/tests/cleanup-ignore.test.ts @@ -0,0 +1,150 @@ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { + cpSync, + mkdirSync, + mkdtempSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +// `npm run cleanup` deletes platform resources that aren't in the state +// file. Resources excluded by .vapi-ignore are never written to state, so +// cleanup must keep them, as push's orphan-protection does. + +const REPO = fileURLToPath(new URL("..", import.meta.url)); +const ORG = "test-cleanup-org"; +const TRACKED = "11111111-1111-4111-8111-111111111111"; +const IGNORED_ASSISTANT = "22222222-2222-4222-8222-222222222222"; +const IGNORED_TOOL = "33333333-3333-4333-8333-333333333333"; +const ORPHAN = "44444444-4444-4444-8444-444444444444"; + +const PLATFORM: Record = { + "/assistant": [ + { id: TRACKED, name: "Front Desk" }, + { id: IGNORED_ASSISTANT, name: "Legacy Bot" }, + { id: ORPHAN, name: "Old Experiment" }, + ], + "/tool": [ + { id: IGNORED_TOOL, type: "function", function: { name: "legacy_lookup" } }, + ], +}; + +async function cleanupRun( + args: string[], +): Promise<{ code: number | null; output: string; deletes: string[] }> { + const deletes: string[] = []; + const server = createServer((req, res) => { + res.setHeader("content-type", "application/json"); + if (req.method === "DELETE") { + deletes.push(req.url ?? ""); + res.end("{}"); + return; + } + const path = (req.url ?? "").split("?")[0]!; + res.end(JSON.stringify(PLATFORM[path] ?? [])); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const baseUrl = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; + const dir = mkdtempSync(join(tmpdir(), "vapi-cleanup-ignore-")); + try { + cpSync(join(REPO, "src"), join(dir, "src"), { recursive: true }); + cpSync(join(REPO, "package.json"), join(dir, "package.json")); + symlinkSync(join(REPO, "node_modules"), join(dir, "node_modules"), "dir"); + writeFileSync(join(dir, `.env.${ORG}`), "VAPI_TOKEN=fake-token-not-used\n"); + writeFileSync( + join(dir, `.vapi-state.${ORG}.json`), + JSON.stringify({ assistants: { "front-desk": { uuid: TRACKED } } }), + ); + mkdirSync(join(dir, "resources", ORG), { recursive: true }); + writeFileSync( + join(dir, "resources", ORG, ".vapi-ignore"), + "# owned by another team\nassistants/legacy-*\ntools/legacy-*\n", + ); + const child = spawn( + process.execPath, + ["--import", "tsx", "src/cleanup.ts", ORG, ...args], + { + cwd: dir, + env: { + ...process.env, + VAPI_BASE_URL: baseUrl, + VAPI_TOKEN: "fake-token-not-used", + }, + }, + ); + let output = ""; + child.stdout.on("data", (chunk) => (output += chunk)); + child.stderr.on("data", (chunk) => (output += chunk)); + const code = await new Promise((resolve) => + child.on("close", resolve), + ); + return { code, output, deletes }; + } finally { + rmSync(dir, { recursive: true, force: true }); + await new Promise((resolve) => server.close(() => resolve())); + } +} + +test( + "a destructive cleanup deletes only true orphans and keeps .vapi-ignore matches", + { timeout: 60_000 }, + async () => { + const result = await cleanupRun(["--force", "--confirm", ORG]); + assert.deepEqual( + { + code: result.code, + deletes: result.deletes, + retainedAssistant: result.output.includes( + "Legacy Bot retained (matched .vapi-ignore: assistants/legacy-*)", + ), + retainedTool: result.output.includes( + "legacy_lookup retained (matched .vapi-ignore: tools/legacy-*)", + ), + }, + { + code: 0, + deletes: [`/assistant/${ORPHAN}`], + retainedAssistant: true, + retainedTool: true, + }, + ); + }, +); + +test( + "a dry run lists ignored resources as kept, not as orphans", + { timeout: 60_000 }, + async () => { + const result = await cleanupRun([]); + assert.deepEqual( + { + code: result.code, + deletes: result.deletes, + orphanListed: result.output.includes( + `assistants: Old Experiment (${ORPHAN})`, + ), + ignoredListed: + result.output.includes(IGNORED_ASSISTANT) && + result.output.includes("🗑️ assistants: Legacy Bot"), + keptSummary: result.output.includes( + "2 resource(s) not in state were kept because they match .vapi-ignore", + ), + }, + { + code: 0, + deletes: [], + orphanListed: true, + ignoredListed: false, + keptSummary: true, + }, + ); + }, +);