Skip to content

fix(.vapi-ignore): honor patterns on push and apply, not just pull#26

Merged
dhruva-reddy merged 1 commit into
mainfrom
fix/vapi-ignore-symmetric-push
May 11, 2026
Merged

fix(.vapi-ignore): honor patterns on push and apply, not just pull#26
dhruva-reddy merged 1 commit into
mainfrom
fix/vapi-ignore-symmetric-push

Conversation

@dhruva-reddy
Copy link
Copy Markdown
Contributor

Describe your changes

Makes .vapi-ignore symmetric across pull, push, and apply. Previously the file was pull-only — push.ts and apply.ts ignored it entirely, which meant a single broken YAML asset (referenced credentials, missing tools, dead refs) could halt every customer's npm run push until the file was physically deleted. With this PR, four things change:

  • Load-time filter (src/resources.ts): loadResources() and loadSingleResource() accept { ignorePatterns } and skip matching files with a 🚫 log line, mirroring pull.ts:700.
  • Push wiring (src/push.ts): reads FORCE_DELETE ? [] : loadIgnorePatterns() and threads it into all 9 loadResources call sites. --force bypasses the filter, matching pull's force-flag semantics at pull.ts:907.
  • Orphan-delete protection (src/delete.ts): findOrphanedResources accepts ignoredIds; deleteOrphanedResources calls loadIgnorePatterns() directly (not the FORCE_DELETE-shadowed shorthand) so protection stays on even under --force. This is the critical data-safety piece — without it, the first --force push after someone .vapi-ignores a previously-tracked resource would DELETE it from the dashboard.
  • Reference validation (src/validate.ts): new validateNoIgnoredReferences promotes the silent-drop in resolver.ts:103-110 (squad referencing a now-ignored assistant) to a validation error. In default mode it surfaces as a warning; --strict aborts the push.

Plus a new test file tests/vapi-ignore-push.test.ts covering T1–T5 spawn-fixture integration tests + 9 unit tests (128/128 total tests pass). Most of the non-test file changes are biome import-reorder noise from biome check --write — no behavior changes outside the four files listed above.

Relevant Context (linear ticket, slack link, etc)

Surfaced live during a gitops-amazon3p full-fleet sync. Two pre-existing stale assets (my-voice-f1751ee7.md referencing a nonexistent PlayHT voice, recording-consent-7230a9c8.md referencing three tools that were never imported during the prod → youssaf-org rename) halted npm run push -- youssaf-org mid-stream. Adding both paths to .vapi-ignore had no effect because push didn't read the file. The customer fork resolved by deleting the YAML directly, but the underlying engine gap remained — this PR closes it.

No Linear ticket; tracked in improvements.md entry #21.

API Changes

  • Is this changing the public API?

    • Yes
    • No
  • If yes, is it backward‐compatible?

    • Yes
    • No

Internal API surface change only: loadResources and loadSingleResource now accept an optional { ignorePatterns?: string[] } second argument. Default behavior (no arg / empty array) is identical to prior behavior — backward-compatible for any internal caller. No public REST API, SDK, or CLI flag contract changes.

How did you test this?

  • New file tests/vapi-ignore-push.test.ts — 5 spawn-fixture integration tests (T1: dry-run honors ignore; T2: --force bypasses; T3: orphan-delete protection under --force; T4: squad-references-ignored is hard error under --strict; T5: explicit-file push honors ignore) + 9 in-process unit tests covering loadResources, findOrphanedResources, and validateNoIgnoredReferences.
  • npm test → 128/128 pass, no regressions in existing suites (push-dry-run, cleanup-safety, path-matching, validate, drift, dep-dedup, etc.).
  • npx @biomejs/biome check src/ tests/ → clean.
  • npm run build (tsc --noEmit) → clean.
  • Manual: re-running npm run push -- youssaf-org in gitops-amazon3p is the validation case — the engine would now silently skip ignored assets without halting the fleet sync. (Manual verification deferred to first downstream consumer; the integration tests pin the same contract end-to-end via spawn-fixture.)

Known non-blocking follow-ups (from code-reviewer)

  • validateNoIgnoredReferences false-positives under --force (no abort in default mode, but --strict --force is noisy). One-line fix.
  • extractReferencedIds doesn't walk squads or evals refs — forward-looking gap; no existing config uses them today.
  • Bootstrap pull writes .vapi-ignore-matched YAMLs to disk (no apply-time damage; load-filter drops them on next push).
  • config.ts module-load assertions require dynamic-import shim in tests — architecture nit, follow-up to defer asserts to a getConfig() accessor.
  • delete.ts:228-242 O(n³) hot path — hoist loadedIds to a Set (perf nit).
  • tests/vapi-ignore-push.test.ts:179-186 safety timeout not cleared on graceful exit — .unref() follow-up.

`.vapi-ignore` is now bidirectional. Four wired pieces:

1. **Load-filter** — `src/resources.ts:loadResources` accepts
   `{ ignorePatterns }`; matched ids emit `🚫 <id> (matched .vapi-ignore: <pattern>)`
   and are filtered out before duplicate detection, validation, or any API call.

2. **Push wire** — `src/push.ts` reads
   `const ignorePatterns = FORCE_DELETE ? [] : loadIgnorePatterns()` once and
   passes it into every `loadResources` call. `--force` bypasses the
   load-filter for deliberate overrides.

3. **Orphan-protect (CRITICAL DATA SAFETY)** — `src/delete.ts`
   `findOrphanedResources` accepts `ignoredIds: Set<string>`; ids matched by
   `.vapi-ignore` are excluded from the orphan list, and
   `deleteOrphanedResources` emits a
   `🚫 <type>/<id> retained (matched .vapi-ignore — orphan-protected)` line
   for each retained id. **Orphan-protect ALWAYS honors the ignore list,
   even under `--force`** — this prevents a `--force` push from silently
   DELETE'ing a dashboard resource the repo has explicitly opted out of
   managing.

4. **Ref-validate** — `src/validate.ts:validateNoIgnoredReferences` walks
   each loaded resource's referenced ids and emits an `error`-severity
   finding for any ref pointing at an ignored id; `--strict` push aborts
   before any API call.

Docs updated (`AGENTS.md`, `docs/learnings/yaml-conventions.md`,
`docs/learnings/simulations.md`, `resources/.vapi-ignore.example`) to
describe the bidirectional semantics, the `--force` bypass and the
ref-validate rule. `improvements.md` gets a `[RESOLVED 2026-05-11]` entry
under #21.

Tests: `tests/vapi-ignore-push.test.ts` pins T1–T5 spawn-fixture
integration tests + in-process unit tests for each helper. All 128 tests
pass; biome + tsc clean.
@dhruva-reddy dhruva-reddy merged commit 7cbcb50 into main May 11, 2026
@dhruva-reddy dhruva-reddy deleted the fix/vapi-ignore-symmetric-push branch May 11, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant