From d7c718f5677029756c383fee66509c6854d0ca92 Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Wed, 13 May 2026 13:48:24 -0700 Subject: [PATCH 1/2] fix(pull): make findExistingResourceId state-aware to prevent clobber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `-.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). --- src/pull.ts | 38 +++- tests/find-existing-resource-id.test.ts | 172 +++++++++++++++ tests/pull-same-name-clobber.test.ts | 276 ++++++++++++++++++++++++ 3 files changed, 481 insertions(+), 5 deletions(-) create mode 100644 tests/find-existing-resource-id.test.ts create mode 100644 tests/pull-same-name-clobber.test.ts diff --git a/src/pull.ts b/src/pull.ts index 48dfb04..9585506 100644 --- a/src/pull.ts +++ b/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 +// `-` slug per UUID, so the second resource gets its own file. +export function findExistingResourceId( existingResourceIds: string[], resource: VapiResource, + stateSection: Record, ): string | undefined { const name = extractName(resource); if (!name) return undefined; @@ -327,8 +337,19 @@ 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 (re-pull: idempotent reuse). + // 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,11 +697,18 @@ 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). Pass `newStateSection` (the in-flight state map this pull is + // building) so that adoptions made earlier in this same loop block + // subsequent same-name resources from clobbering — `state[resourceType]` + // is the on-disk snapshot and would not reflect intra-pull claims. resourceId = bootstrap ? generateResourceId(resource) - : (findExistingResourceId(existingResourceIds, resource) ?? - generateResourceId(resource)); + : (findExistingResourceId( + existingResourceIds, + resource, + newStateSection, + ) ?? generateResourceId(resource)); } const isNew = !reverseMap.get(resource.id) || diff --git a/tests/find-existing-resource-id.test.ts b/tests/find-existing-resource-id.test.ts new file mode 100644 index 0000000..2fd4cba --- /dev/null +++ b/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 = {}; + 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 = { + 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 = { + 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 = { + 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 = { + 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 = { + 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 = {}; + 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 = {}; + 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 = {}; + const result = findExistingResourceId( + onDisk, + resource("uuid-aaaa", "Riley"), + newState, + ); + assert.equal(result, undefined); +}); diff --git a/tests/pull-same-name-clobber.test.ts b/tests/pull-same-name-clobber.test.ts new file mode 100644 index 0000000..479ea09 --- /dev/null +++ b/tests/pull-same-name-clobber.test.ts @@ -0,0 +1,276 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { + cpSync, + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { Worker } from "node:worker_threads"; + +// ───────────────────────────────────────────────────────────────────────────── +// Integration test for the state-aware adoption fix in pull.ts. +// +// Scenario: dashboard has 2 assistants both named "Riley" (UUID A and B). +// State already maps the slug `riley` → A. On disk, `riley.md` holds A's +// content. A bug-free pull must: +// 1. Preserve `riley.md` unchanged (still A's content) — NOT clobber it. +// 2. Create a fresh `riley-.md` for the new resource B. +// 3. Persist both mappings in state: `riley → A` AND `riley- → B`. +// +// Without the fix, B silently overwrites `riley.md` and the state mapping +// for `riley` flips to B — orphaning A's UUID with no on-disk artifact. +// +// Reproduces the mudflap "5 Rileys" customer scenario that will keep getting +// triggered as Vapi auto-seeds same-named twins for new orgs. +// ───────────────────────────────────────────────────────────────────────────── + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = join(__dirname, ".."); + +interface StubRoute { + method: string; + pathStartsWith: string; + body: unknown; +} + +// Mirrors the spawn-fixture / Worker-stub pattern in +// `tests/vapi-ignore-push.test.ts`. The HTTP stub MUST live on a separate +// thread so `fetchAllResources` can be served while `spawnSync` parks the +// main thread's event loop. +function startStub( + routes: StubRoute[], +): Promise<{ worker: Worker; port: number }> { + return new Promise((resolveStart, rejectStart) => { + const stubSource = ` + const http = require('node:http'); + const { parentPort, workerData } = require('node:worker_threads'); + const routes = workerData.routes; + const server = http.createServer((req, res) => { + const url = req.url || ''; + const method = (req.method || 'GET').toUpperCase(); + const match = routes.find( + (r) => r.method === method && url.startsWith(r.pathStartsWith), + ); + res.statusCode = 200; + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify(match ? match.body : [])); + }); + server.listen(0, '127.0.0.1', () => { + const addr = server.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + parentPort.postMessage({ type: 'listening', port }); + }); + parentPort.on('message', (msg) => { + if (msg && msg.type === 'shutdown') { + server.close(() => process.exit(0)); + } + }); + `; + const worker = new Worker(stubSource, { + eval: true, + workerData: { routes }, + }); + worker.once("error", rejectStart); + worker.on("message", (msg: { type: string; port?: number }) => { + if (msg.type === "listening" && typeof msg.port === "number") { + resolveStart({ worker, port: msg.port }); + } + }); + }); +} + +const ENV = "test-clobber"; +const UUID_A = "aaaaaaaa-1111-1111-1111-111111111111"; +const UUID_B = "bbbbbbbb-2222-2222-2222-222222222222"; + +// Minimal assistant body the API would return. Includes a distinctive +// marker so we can assert which body landed in each file. +function rileyDashboardBody(uuid: string, marker: string) { + return { + id: uuid, + orgId: "org-test", + name: "Riley", + model: { + provider: "openai", + model: "gpt-4o", + messages: [{ role: "system", content: `marker:${marker}` }], + }, + voice: { provider: "11labs", voiceId: "burt" }, + }; +} + +// Pre-pull on-disk content for `riley.md`. Uses A's marker so we can tell +// whether B clobbered it. +const PREEXISTING_RILEY_MD = `--- +model: + provider: openai + model: gpt-4o +name: Riley +voice: + provider: 11labs + voiceId: burt +--- + +marker:A-original +`; + +test("pull: 2 dashboard resources with same name (A in state, B new) — fix prevents clobber", async () => { + const dir = mkdtempSync(join(tmpdir(), "vapi-pull-clobber-")); + + // Copy source tree + package.json, symlink node_modules. Mirrors + // vapi-ignore-push.test.ts's spawn-fixture setup. + 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", + ); + + // Seed the resource tree: existing `riley.md` holding A's content. + const assistantsDir = join(dir, "resources", ENV, "assistants"); + mkdirSync(assistantsDir, { recursive: true }); + writeFileSync(join(assistantsDir, "riley.md"), PREEXISTING_RILEY_MD); + + // Seed state: slug `riley` already maps to UUID A. + writeFileSync( + join(dir, `.vapi-state.${ENV}.json`), + JSON.stringify( + { + credentials: {}, + assistants: { + riley: { uuid: UUID_A, lastPulledHash: "stale-hash-A" }, + }, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }, + null, + 2, + ), + ); + + // HTTP stub returns BOTH Rileys (A and B) for the /assistant list call. + // Other endpoints return [] (no credentials, no other resource types). + const { worker, port } = await startStub([ + { + method: "GET", + pathStartsWith: "/assistant", + body: [ + rileyDashboardBody(UUID_A, "A-fresh-from-platform"), + rileyDashboardBody(UUID_B, "B-new-twin"), + ], + }, + ]); + + writeFileSync( + join(dir, `.env.${ENV}`), + [ + "VAPI_TOKEN=fake-token-not-used", + `VAPI_BASE_URL=http://127.0.0.1:${port}`, + "", + ].join("\n"), + ); + + try { + // Run pull via the CLI entrypoint (same path real customers exercise). + // --force so the mtime-based "locally modified" guard does not kick in + // and short-circuit the platform overwrite of riley.md (we want pull + // to actually try to write riley.md — the question is whether B's + // content lands there or A's content stays). + const res = spawnSync( + "node", + ["--import", "tsx", "src/pull.ts", ENV, "--force"], + { + cwd: dir, + env: { + ...process.env, + VAPI_TOKEN: "fake-token-not-used", + VAPI_BASE_URL: `http://127.0.0.1:${port}`, + }, + encoding: "utf-8", + timeout: 30_000, + }, + ); + + assert.equal( + res.status, + 0, + `pull exit code ${res.status}\nstdout=${res.stdout}\nstderr=${res.stderr}`, + ); + + // ── Filesystem assertions ──────────────────────────────────────────── + // riley.md must still exist AND hold A's content (the platform's + // A-fresh-from-platform marker, since --force overwrites with platform + // state — but NOT B's content). + const rileyPath = join(assistantsDir, "riley.md"); + assert.ok(existsSync(rileyPath), "riley.md must still exist"); + const rileyContent = readFileSync(rileyPath, "utf-8"); + assert.match( + rileyContent, + /marker:A-fresh-from-platform/, + `riley.md must hold A's content (the file mapped to A in state); got:\n${rileyContent}`, + ); + assert.doesNotMatch( + rileyContent, + /marker:B-new-twin/, + `riley.md must NOT have been clobbered by B; got:\n${rileyContent}`, + ); + + // B must have landed in its own file `riley-.md`. + const expectedBSlug = `riley-${UUID_B.slice(0, 8)}`; + const bPath = join(assistantsDir, `${expectedBSlug}.md`); + assert.ok( + existsSync(bPath), + `expected B's file at ${bPath}; assistants dir contents: ${readdirSync(assistantsDir).join(", ")}`, + ); + const bContent = readFileSync(bPath, "utf-8"); + assert.match( + bContent, + /marker:B-new-twin/, + `${expectedBSlug}.md must hold B's content; got:\n${bContent}`, + ); + + // ── State assertions ───────────────────────────────────────────────── + const finalState = JSON.parse( + readFileSync(join(dir, `.vapi-state.${ENV}.json`), "utf-8"), + ); + assert.equal( + finalState.assistants.riley?.uuid, + UUID_A, + `state[riley] must still map to A (${UUID_A}); got ${JSON.stringify(finalState.assistants.riley)}`, + ); + assert.equal( + finalState.assistants[expectedBSlug]?.uuid, + UUID_B, + `state[${expectedBSlug}] must map to B (${UUID_B}); got ${JSON.stringify(finalState.assistants[expectedBSlug])}`, + ); + } finally { + worker.postMessage({ type: "shutdown" }); + await new Promise((resolveShutdown) => { + worker.once("exit", () => resolveShutdown()); + setTimeout(() => { + worker + .terminate() + .then(() => resolveShutdown()) + .catch(() => resolveShutdown()); + }, 1000); + }); + rmSync(dir, { recursive: true, force: true }); + } +}); From 697a8dc55ed3395a220a2704fddb6e0ce6d543ac Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Wed, 13 May 2026 13:53:30 -0700 Subject: [PATCH 2/2] fix(pull): merge state[resourceType] into adoption guard (code-review H1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/pull.ts | 30 +++++++++----- tests/pull-same-name-clobber.test.ts | 59 +++++++++++++++++++++------- 2 files changed, 64 insertions(+), 25 deletions(-) diff --git a/src/pull.ts b/src/pull.ts index 9585506..7dee8c3 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -341,7 +341,11 @@ export function findExistingResourceId( // 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 (re-pull: idempotent reuse). + // 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) => { @@ -698,17 +702,23 @@ export async function pullResourceType( if (!resourceId) { // Reuse an existing file's resourceId if the name matches (cross-env - // pull). Pass `newStateSection` (the in-flight state map this pull is - // building) so that adoptions made earlier in this same loop block - // subsequent same-name resources from clobbering — `state[resourceType]` - // is the on-disk snapshot and would not reflect intra-pull claims. + // 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, - newStateSection, - ) ?? generateResourceId(resource)); + : (findExistingResourceId(existingResourceIds, resource, claimView) ?? + generateResourceId(resource)); } const isNew = !reverseMap.get(resource.id) || diff --git a/tests/pull-same-name-clobber.test.ts b/tests/pull-same-name-clobber.test.ts index 479ea09..2be3e17 100644 --- a/tests/pull-same-name-clobber.test.ts +++ b/tests/pull-same-name-clobber.test.ts @@ -124,7 +124,16 @@ voice: marker:A-original `; -test("pull: 2 dashboard resources with same name (A in state, B new) — fix prevents clobber", async () => { +// Run the full clobber scenario with a configurable dashboard-response +// ordering. Asserts the fix works regardless of which Riley the API +// returns first — this is the H1 case from code review: without merging +// the prior-pull state into the adoption guard, B-first ordering would +// clobber A's file because `newStateSection` is empty when B is +// processed. +async function runClobberScenario( + testName: string, + dashboardOrder: "A-first" | "B-first", +): Promise { const dir = mkdtempSync(join(tmpdir(), "vapi-pull-clobber-")); // Copy source tree + package.json, symlink node_modules. Mirrors @@ -165,16 +174,24 @@ test("pull: 2 dashboard resources with same name (A in state, B new) — fix pre ), ); - // HTTP stub returns BOTH Rileys (A and B) for the /assistant list call. - // Other endpoints return [] (no credentials, no other resource types). + // HTTP stub returns BOTH Rileys (A and B) for the /assistant list call, + // in the configured order. The fix must produce the correct outcome + // regardless of which one the dashboard returns first. + const orderedBodies = + dashboardOrder === "A-first" + ? [ + rileyDashboardBody(UUID_A, "A-fresh-from-platform"), + rileyDashboardBody(UUID_B, "B-new-twin"), + ] + : [ + rileyDashboardBody(UUID_B, "B-new-twin"), + rileyDashboardBody(UUID_A, "A-fresh-from-platform"), + ]; const { worker, port } = await startStub([ { method: "GET", pathStartsWith: "/assistant", - body: [ - rileyDashboardBody(UUID_A, "A-fresh-from-platform"), - rileyDashboardBody(UUID_B, "B-new-twin"), - ], + body: orderedBodies, }, ]); @@ -211,7 +228,7 @@ test("pull: 2 dashboard resources with same name (A in state, B new) — fix pre assert.equal( res.status, 0, - `pull exit code ${res.status}\nstdout=${res.stdout}\nstderr=${res.stderr}`, + `[${testName}] pull exit code ${res.status}\nstdout=${res.stdout}\nstderr=${res.stderr}`, ); // ── Filesystem assertions ──────────────────────────────────────────── @@ -219,17 +236,17 @@ test("pull: 2 dashboard resources with same name (A in state, B new) — fix pre // A-fresh-from-platform marker, since --force overwrites with platform // state — but NOT B's content). const rileyPath = join(assistantsDir, "riley.md"); - assert.ok(existsSync(rileyPath), "riley.md must still exist"); + assert.ok(existsSync(rileyPath), `[${testName}] riley.md must still exist`); const rileyContent = readFileSync(rileyPath, "utf-8"); assert.match( rileyContent, /marker:A-fresh-from-platform/, - `riley.md must hold A's content (the file mapped to A in state); got:\n${rileyContent}`, + `[${testName}] riley.md must hold A's content (the file mapped to A in state); got:\n${rileyContent}`, ); assert.doesNotMatch( rileyContent, /marker:B-new-twin/, - `riley.md must NOT have been clobbered by B; got:\n${rileyContent}`, + `[${testName}] riley.md must NOT have been clobbered by B; got:\n${rileyContent}`, ); // B must have landed in its own file `riley-.md`. @@ -237,13 +254,13 @@ test("pull: 2 dashboard resources with same name (A in state, B new) — fix pre const bPath = join(assistantsDir, `${expectedBSlug}.md`); assert.ok( existsSync(bPath), - `expected B's file at ${bPath}; assistants dir contents: ${readdirSync(assistantsDir).join(", ")}`, + `[${testName}] expected B's file at ${bPath}; assistants dir contents: ${readdirSync(assistantsDir).join(", ")}`, ); const bContent = readFileSync(bPath, "utf-8"); assert.match( bContent, /marker:B-new-twin/, - `${expectedBSlug}.md must hold B's content; got:\n${bContent}`, + `[${testName}] ${expectedBSlug}.md must hold B's content; got:\n${bContent}`, ); // ── State assertions ───────────────────────────────────────────────── @@ -253,12 +270,12 @@ test("pull: 2 dashboard resources with same name (A in state, B new) — fix pre assert.equal( finalState.assistants.riley?.uuid, UUID_A, - `state[riley] must still map to A (${UUID_A}); got ${JSON.stringify(finalState.assistants.riley)}`, + `[${testName}] state[riley] must still map to A (${UUID_A}); got ${JSON.stringify(finalState.assistants.riley)}`, ); assert.equal( finalState.assistants[expectedBSlug]?.uuid, UUID_B, - `state[${expectedBSlug}] must map to B (${UUID_B}); got ${JSON.stringify(finalState.assistants[expectedBSlug])}`, + `[${testName}] state[${expectedBSlug}] must map to B (${UUID_B}); got ${JSON.stringify(finalState.assistants[expectedBSlug])}`, ); } finally { worker.postMessage({ type: "shutdown" }); @@ -273,4 +290,16 @@ test("pull: 2 dashboard resources with same name (A in state, B new) — fix pre }); rmSync(dir, { recursive: true, force: true }); } +} + +test("pull: 2 same-name resources (A-first ordering) — fix prevents clobber", async () => { + await runClobberScenario("A-first", "A-first"); +}); + +test("pull: 2 same-name resources (B-first ordering) — fix prevents clobber regardless of dashboard list order", async () => { + // Regression guard for the H1 finding from code review: without merging + // `state[resourceType]` into the adoption guard, B-first ordering would + // clobber A's file (B is processed while `newStateSection` is still empty, + // sees `riley.md` as "unclaimed in flight" — but prior state has it). + await runClobberScenario("B-first", "B-first"); });