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, + }, + ); + }, +);