fix(pull): state-aware findExistingResourceId — prevent same-name clobber (#29)

* fix(pull): make findExistingResourceId state-aware to prevent clobber

When two dashboard resources share a name (e.g. via Duplicate Assistant
or any platform auto-seed) and one is already tracked in state, the
prior pull behavior was to silently reuse the existing file's slug for
the new UUID — clobbering the original's content and orphaning its
state mapping.

Fix: before adopting a matching file, check whether it's already
claimed in state by a different UUID. If so, refuse adoption and let
the caller fall through to `generateResourceId(resource)`, which
produces a deterministic `<name>-<uuid8>.md` slug per UUID. Cross-env
adoption (file exists, state empty for it) and re-pull (file exists,
state claims it for THIS UUID) paths are unchanged.

Promotes `findExistingResourceId` from `function` to `export function`
so tests can pin behavior directly.

Closes customer issue surfaced in gitops-mudflap working session
2026-05-13 (PRISM-737-adjacent).

* fix(pull): merge state[resourceType] into adoption guard (code-review H1)

Addresses the BLOCKING finding from the code review of d7c718f. The
first fix passed only `newStateSection` (in-flight pull state) to
`findExistingResourceId`. That made the fix iteration-order-dependent:
if the dashboard's /assistant list returned the new same-name twin
BEFORE the tracked one, the new twin saw `newStateSection` empty (the
tracked one hadn't been processed yet), and clobbered the existing
file anyway.

Fix: pass a merged view `{...state[resourceType], ...newStateSection}`
to the adoption guard. The prior-pull state carries claims loaded from
disk; the in-flight section carries claims made earlier in this same
loop. Spreading `newStateSection` last preserves intra-pull precedence
when both have the same slug (e.g. an earlier iteration re-keyed it).

Adds a B-first integration test that fails under d7c718f and passes
under this fix. The original A-first test is preserved by extracting
both into a shared `runClobberScenario(testName, order)` helper.

Also tightens the same-UUID branch comment in `findExistingResourceId`
to clarify that it's defensive — the caller's `reverseMap.get` short-
circuits this case in production. Comment-only change.

Suite: 167/167 pass.

Author: dhruva-vapi

src/pull.ts

@@ -316,9 +316,19 @@ export function listExistingResourceIds(resourceType: ResourceType): string[] {
 // When pulling a new environment, a resource may already exist on disk under a
 // different UUID suffix (e.g., `end-call-tool-8102e715` from dev). Match by
 // name-slug so we reuse the existing file instead of creating a duplicate.
-function findExistingResourceId(
+//
+// State-awareness guard: if a name-matching file is already claimed in state
+// by a *different* UUID, refuse adoption. Without this guard, two dashboard
+// resources sharing a name (e.g. a Duplicate-Assistant click, or any
+// platform auto-seed of a same-named twin) collapse onto the same file —
+// the second pull silently overwrites the first's content and reassigns
+// the slug's state mapping to the new UUID, orphaning the original.
+// Falling through to `generateResourceId` (caller) produces a deterministic
+// `<name>-<uuid8>` slug per UUID, so the second resource gets its own file.
+export function findExistingResourceId(
   existingResourceIds: string[],
   resource: VapiResource,
+  stateSection: Record<string, ResourceState>,
 ): string | undefined {
   const name = extractName(resource);
   if (!name) return undefined;
@@ -327,8 +337,23 @@ function findExistingResourceId(
   const matches = existingResourceIds.filter(
     (id) => extractBaseSlug(id) === nameSlug,
   );
+  if (matches.length === 0) return undefined;
+
+  // A file is adoptable when it is either unclaimed in state (cross-env
+  // pull: file shipped from dev, this env has no prior state for it) or
+  // already claimed by THIS resource's UUID. The same-UUID branch is
+  // defensive — in the production code path the caller's
+  // `reverseMap.get(resource.id)` short-circuits this case, so we only
+  // reach this function when the UUID is unknown to state. Keep the
+  // branch anyway so the helper is correct in isolation.
+  // A file claimed by a *different* UUID is not adoptable — adopting it
+  // would clobber the existing resource's content and state mapping.
+  const adoptable = matches.filter((id) => {
+    const claim = stateSection[id]?.uuid;
+    return claim === undefined || claim === resource.id;
+  });
 
-  return matches.length === 1 ? matches[0] : undefined;
+  return adoptable.length === 1 ? adoptable[0] : undefined;
 }
 
 function removeUuidMappings(
@@ -676,10 +701,23 @@ export async function pullResourceType(
     }
 
     if (!resourceId) {
-      // Reuse an existing file's resourceId if the name matches (cross-env pull)
+      // Reuse an existing file's resourceId if the name matches (cross-env
+      // pull). The adoption guard needs both prior-pull claims AND
+      // intra-pull claims so the fix is iteration-order-independent:
+      //   - `state[resourceType]` carries prior-pull claims loaded from
+      //     disk. Without this, if the dashboard returns the new same-name
+      //     twin BEFORE the tracked one, the new twin sees `newStateSection`
+      //     empty and clobbers the tracked file. The customer's mudflap-prod
+      //     5-Rileys investigation surfaced this ordering dependency.
+      //   - `newStateSection` carries intra-pull claims from earlier
+      //     iterations. Handles the converse (tracked-then-twin order).
+      // Spread `newStateSection` last so it wins when both have the same
+      // slug — the in-flight value is the more authoritative claim during
+      // this pull (e.g. an earlier iteration rekeyed the slug to a new UUID).
+      const claimView = { ...state[resourceType], ...newStateSection };
       resourceId = bootstrap
         ? generateResourceId(resource)
-        : (findExistingResourceId(existingResourceIds, resource) ??
+        : (findExistingResourceId(existingResourceIds, resource, claimView) ??
           generateResourceId(resource));
     }
     const isNew =

tests/find-existing-resource-id.test.ts

@@ -0,0 +1,172 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import type { ResourceState } from "../src/types.ts";
+
+// 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. Same pattern as
+// `clean-resource.test.ts`.
+process.argv = ["node", "test", "test-fixture-org"];
+process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used";
+
+const { findExistingResourceId } = await import("../src/pull.ts");
+
+// Helper: build a minimal resource shape with the id+name fields the function
+// reads. Other fields are irrelevant for adoption logic.
+function resource(id: string, name: string) {
+  return { id, name };
+}
+
+function stateEntry(uuid: string): ResourceState {
+  return { uuid };
+}
+
+// ─────────────────────────────────────────────────────────────────────────────
+// Adoption: positive cases (the function reuses an on-disk file's slug)
+// ─────────────────────────────────────────────────────────────────────────────
+
+test("adoption: state empty, one file matches name → adopt (cross-env pull)", () => {
+  // Classic cross-env pull: dev shipped a file `riley-8102e715.md`, prod is
+  // a fresh clone with no state entry for it. Pull should reuse the file
+  // even though the UUID-suffix on disk differs from prod's UUID.
+  const onDisk = ["riley-8102e715"];
+  const newState: Record<string, ResourceState> = {};
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-prod-aaaa", "Riley"),
+    newState,
+  );
+  assert.equal(result, "riley-8102e715");
+});
+
+test("adoption: file claimed in state by the SAME UUID → adopt (re-pull is idempotent)", () => {
+  // Second pull of an already-tracked resource. State maps the slug to
+  // THIS resource's UUID, so reusing the slug is correct.
+  const onDisk = ["riley"];
+  const newState: Record<string, ResourceState> = {
+    riley: stateEntry("uuid-aaaa"),
+  };
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-aaaa", "Riley"),
+    newState,
+  );
+  assert.equal(result, "riley");
+});
+
+test("adoption: two matches but only one is adoptable (the unclaimed one) → adopt it", () => {
+  // The dashboard has 2 same-named resources. The first was already
+  // processed earlier in this same pull loop and adopted `riley.md`.
+  // Now a different file `riley-deadbeef.md` (e.g. from cross-env pull
+  // history) is on disk, unclaimed in state, and the current resource
+  // can adopt it without conflict.
+  const onDisk = ["riley", "riley-deadbeef"];
+  const newState: Record<string, ResourceState> = {
+    riley: stateEntry("uuid-aaaa"),
+  };
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-bbbb", "Riley"),
+    newState,
+  );
+  assert.equal(result, "riley-deadbeef");
+});
+
+// ─────────────────────────────────────────────────────────────────────────────
+// No adoption: the fix's core case + the existing 0-or-2+ behavior
+// ─────────────────────────────────────────────────────────────────────────────
+
+test("no adoption: file claimed by DIFFERENT UUID → undefined (the fix's main case)", () => {
+  // This is the clobber scenario the fix prevents: state already maps
+  // `riley` to UUID A; a new resource with the same name but a NEW UUID
+  // (B) must NOT adopt `riley.md` — doing so would overwrite A's content.
+  const onDisk = ["riley"];
+  const newState: Record<string, ResourceState> = {
+    riley: stateEntry("uuid-aaaa"),
+  };
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-bbbb", "Riley"),
+    newState,
+  );
+  assert.equal(result, undefined);
+});
+
+test("no adoption: N+ matches with mixed claims → undefined", () => {
+  // Two on-disk files both name-match. One is unclaimed (adoptable),
+  // one is claimed by a different UUID (NOT adoptable). But ALSO a
+  // third match exists, claimed by yet another different UUID. With
+  // multiple adoptable candidates the 1:1 ambiguity guard still kicks
+  // in. Here we test a related shape: 2 adoptable matches → ambiguous.
+  const onDisk = ["riley", "riley-aaaa1111", "riley-bbbb2222"];
+  const newState: Record<string, ResourceState> = {
+    riley: stateEntry("uuid-other"),
+    // riley-aaaa1111 and riley-bbbb2222 are both unclaimed → 2 adoptable
+  };
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-cccc", "Riley"),
+    newState,
+  );
+  assert.equal(result, undefined);
+});
+
+test("no adoption: N+ matches but all claimed by other UUIDs → undefined", () => {
+  // Every name-matching file is claimed by some other UUID. No file is
+  // adoptable for the current resource; fall through to
+  // generateResourceId in the caller.
+  const onDisk = ["riley", "riley-aaaa1111"];
+  const newState: Record<string, ResourceState> = {
+    riley: stateEntry("uuid-aaaa"),
+    "riley-aaaa1111": stateEntry("uuid-bbbb"),
+  };
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-cccc", "Riley"),
+    newState,
+  );
+  assert.equal(result, undefined);
+});
+
+test("no adoption: no name-matching files on disk → undefined", () => {
+  const onDisk = ["alex", "morgan-1234abcd"];
+  const newState: Record<string, ResourceState> = {};
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-aaaa", "Riley"),
+    newState,
+  );
+  assert.equal(result, undefined);
+});
+
+// ─────────────────────────────────────────────────────────────────────────────
+// Edge cases (regression guards, unchanged behavior)
+// ─────────────────────────────────────────────────────────────────────────────
+
+test("regression: resource without a name → undefined (unchanged)", () => {
+  // Tools store their name under function.name (see extractName). A
+  // resource with neither a top-level name nor a function.name is
+  // un-adoptable by design — no slug to compute.
+  const onDisk = ["riley"];
+  const newState: Record<string, ResourceState> = {};
+  const result = findExistingResourceId(
+    onDisk,
+    { id: "uuid-aaaa" }, // no name
+    newState,
+  );
+  assert.equal(result, undefined);
+});
+
+test("regression: two same-name files, state empty → undefined (unchanged ambiguity)", () => {
+  // Pre-fix behavior: 2+ matches without a state discriminator → ambiguous,
+  // refuse adoption. Fix should preserve this — both files are adoptable
+  // (unclaimed), so `adoptable.length === 2` and the 1:1 guard fires.
+  const onDisk = ["riley", "riley-deadbeef"];
+  const newState: Record<string, ResourceState> = {};
+  const result = findExistingResourceId(
+    onDisk,
+    resource("uuid-aaaa", "Riley"),
+    newState,
+  );
+  assert.equal(result, undefined);
+});