Skip to content

Commit 985e4a2

Browse files
committed
fix: dedup dependency auto-apply to prevent duplicate tool mints (Gap #10)
Targeted assistant pushes minted duplicate dashboard tools when bootstrap pull stored an existing dashboard tool under a name-slugged state key (e.g. `end-call-67aea057`) instead of the user's original local key. The exact-key lookup in `ensureToolExists` / `ensureStructuredOutputExists` missed and POSTed a fresh duplicate. Each subsequent targeted push repeated the cycle, accumulating dashboard orphans. This adds a name-based dedup check between the exact-key short-circuit and the create path, in two layers: 1. State-side: scan state for any key whose `extractBaseSlug` matches the local payload's slugified name (handles bootstrap-renamed keys). 2. Dashboard-side: lazy-fetch the live `/tool` (and `/structured-output`) list once per push and check for a remote resource with the same canonical name. When >1 distinct UUID matches the same name (real on-dashboard duplicates from prior bug runs), pick the lex-smallest UUID for stable adoption, warn naming the loser UUIDs, and point at `npm run cleanup`. Never mint another duplicate. Adoption flow: - Re-key state to the adopted UUID under the local resourceId. - Drop other state keys pointing at the same UUID and mark them touched, so a subsequent full push doesn't orphan-delete the adopted dashboard resource (Stack J / mergeScoped flushes the deletion). - Route through `applyTool`/`applyStructuredOutput` so the local payload PATCHes the dashboard with the standard drift-check flow, instead of recording a fake `lastPushedHash` that would silently drop a locally-edited dependency. Tests: 12 unit tests for `findExistingResourceByName` covering state-only, dashboard-only, both-agree, ambiguous (state-vs-state, state-vs-dashboard), no-name, exact-key-excluded, no-match. All 114 suites pass. Refs: improvements.md §10
1 parent 79b5dc0 commit 985e4a2

3 files changed

Lines changed: 516 additions & 0 deletions

File tree

src/dep-dedup.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Gap #10 — dependency deduplication helpers.
2+
//
3+
// On a targeted assistant push, `ensureToolExists` / `ensureStructuredOutputExists`
4+
// previously skipped auto-create only when (a) the dep id was UUID-shaped,
5+
// (b) `state.tools[depId]` was an exact key match, or (c) we'd already
6+
// auto-applied this id in the current run. Bootstrap pull (pull.ts:269-273)
7+
// stores resources under `<slug>-<uuid8>` keys (e.g. `end-call-67aea057`),
8+
// so a local file referencing `b2b-invoice-end-call` would miss the exact-key
9+
// check and POST a duplicate dashboard tool. Repeated targeted pushes
10+
// accumulated orphans on the dashboard.
11+
//
12+
// This module's helpers detect those collisions BEFORE create:
13+
// - `findExistingResourceByName` — match local payload's canonical name
14+
// against existing state entries (renamed/state-only keys) and the live
15+
// dashboard list. Returns the UUID to adopt, plus an `ambiguous` flag
16+
// when multiple distinct UUIDs share the same name (real on-dashboard
17+
// duplicates from prior bug runs — the caller should warn and surface
18+
// the loser UUIDs so a follow-up `npm run cleanup` can prune them).
19+
//
20+
// NOTE on duplication: `slugify` and `extractBaseSlug` here mirror the
21+
// definitions in `src/pull.ts` (lines ~253-278). pull.ts imports config.ts,
22+
// which calls `parseEnvironment()` at module load and `process.exit(1)`s
23+
// without a CLI env arg — making it impossible to import in a unit test.
24+
// This module imports ONLY from `./types.ts` so it stays testable in
25+
// isolation. Five lines duplicated is the right tradeoff; do not "DRY"
26+
// these back into pull.ts.
27+
28+
import type { ResourceState } from "./types.ts";
29+
30+
export interface RemoteResource {
31+
id: string;
32+
name?: string;
33+
function?: { name?: string };
34+
}
35+
36+
export interface DedupMatch {
37+
uuid: string;
38+
source: "state" | "dashboard" | "both";
39+
ambiguous: boolean;
40+
// Other distinct UUIDs we saw under the same canonical name. Empty when
41+
// `ambiguous` is false. Caller should surface these in a warning so the
42+
// user can run `npm run cleanup` to prune the duplicates.
43+
duplicateUuids: string[];
44+
}
45+
46+
export function slugify(name: string): string {
47+
return name
48+
.toLowerCase()
49+
.replace(/[^a-z0-9]+/g, "-")
50+
.replace(/^-+|-+$/g, "")
51+
.replace(/-+/g, "-");
52+
}
53+
54+
export function extractBaseSlug(resourceId: string): string {
55+
const match = resourceId.match(/^(.*)-([a-f0-9]{8})$/i);
56+
return match?.[1] ?? resourceId;
57+
}
58+
59+
// Pulls the canonical name from a tool / SO / assistant payload.
60+
// For tools: `function.name` is the canonical name (per pull.ts:261-267).
61+
// For SOs / assistants / etc.: top-level `name`. Top-level wins when both
62+
// are present.
63+
export function extractResourceName(
64+
payload: Record<string, unknown>,
65+
): string | undefined {
66+
if (typeof payload.name === "string" && payload.name) return payload.name;
67+
const fn = payload.function as Record<string, unknown> | undefined;
68+
if (fn && typeof fn.name === "string" && fn.name) return fn.name;
69+
return undefined;
70+
}
71+
72+
// Find an existing resource (in state or dashboard) whose canonical name
73+
// matches the local payload's canonical name. Used by ensureToolExists /
74+
// ensureStructuredOutputExists to avoid creating a duplicate when bootstrap
75+
// pull has already stored the same dashboard resource under a different
76+
// state key.
77+
//
78+
// `localResourceId` is the key the engine WANTS to use in state. If a state
79+
// entry already exists under that exact key, the caller short-circuits BEFORE
80+
// calling this — so we exclude it here as a safety belt.
81+
//
82+
// Tiebreaker for >1 distinct UUID matching the same slugified name (i.e.
83+
// real on-dashboard duplicates from prior bug runs): pick lexically smallest
84+
// UUID for stable, deterministic adoption. Set `ambiguous=true` and surface
85+
// the loser UUIDs in `duplicateUuids` so the caller can warn.
86+
export function findExistingResourceByName(args: {
87+
localResourceId: string;
88+
localPayload: Record<string, unknown>;
89+
stateSection: Record<string, ResourceState>;
90+
remoteList?: RemoteResource[];
91+
}): DedupMatch | undefined {
92+
const localName = extractResourceName(args.localPayload);
93+
if (!localName) return undefined;
94+
const localSlug = slugify(localName);
95+
96+
// uuid -> set of source labels (state:<key>, dashboard:<id>)
97+
const matches = new Map<string, Set<string>>();
98+
99+
for (const [stateKey, entry] of Object.entries(args.stateSection)) {
100+
if (stateKey === args.localResourceId) continue;
101+
if (extractBaseSlug(stateKey) === localSlug) {
102+
const set = matches.get(entry.uuid) ?? new Set<string>();
103+
set.add(`state:${stateKey}`);
104+
matches.set(entry.uuid, set);
105+
}
106+
}
107+
108+
for (const remote of args.remoteList ?? []) {
109+
const remoteName =
110+
(typeof remote.name === "string" && remote.name) || remote.function?.name;
111+
if (!remoteName) continue;
112+
if (slugify(remoteName) === localSlug) {
113+
const set = matches.get(remote.id) ?? new Set<string>();
114+
set.add(`dashboard:${remote.id}`);
115+
matches.set(remote.id, set);
116+
}
117+
}
118+
119+
if (matches.size === 0) return undefined;
120+
121+
const sorted = [...matches.keys()].sort();
122+
const winner = sorted[0]!;
123+
const winnerSources = matches.get(winner)!;
124+
const hasState = [...winnerSources].some((s) => s.startsWith("state:"));
125+
const hasDashboard = [...winnerSources].some((s) =>
126+
s.startsWith("dashboard:"),
127+
);
128+
const source: DedupMatch["source"] =
129+
hasState && hasDashboard ? "both" : hasState ? "state" : "dashboard";
130+
131+
return {
132+
uuid: winner,
133+
source,
134+
ambiguous: matches.size > 1,
135+
duplicateUuids: sorted.slice(1),
136+
};
137+
}

src/push.ts

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import { summarizeFindings, validateResources } from "./validate.ts";
1616
import { checkDriftForUpdate } from "./drift.ts";
1717
import { writeSnapshot } from "./snapshot.ts";
1818
import { mergeScoped } from "./state-merge.ts";
19+
import {
20+
findExistingResourceByName,
21+
type RemoteResource,
22+
} from "./dep-dedup.ts";
1923

2024
// Map a resource label to its state-file key. Used for snapshotting (Stack H)
2125
// — snapshot directories are keyed by the same names the state file uses.
@@ -827,6 +831,66 @@ interface DependencyContext {
827831
autoApplied: Set<string>;
828832
autoAppliedTools: ResourceFile<Record<string, unknown>>[];
829833
autoAppliedStructuredOutputs: ResourceFile<Record<string, unknown>>[];
834+
// Stack — Gap #10 dedup. Lazy-fetched at most once per push when an
835+
// auto-apply path needs to verify the tool/SO doesn't already exist on
836+
// the dashboard under a different state key. `undefined` = not yet
837+
// fetched; an empty array means "we tried, dashboard returned 0".
838+
existingRemoteTools?: RemoteResource[];
839+
existingRemoteStructuredOutputs?: RemoteResource[];
840+
// Touched sets — adoption needs to mark touched so scoped state-merge
841+
// (#15 / Stack J) persists the adopted UUID under the local resourceId.
842+
touched: TouchedSets;
843+
}
844+
845+
// Gap #10 — lazy-fetch the live dashboard tool list once per push. Used by
846+
// `ensureToolExists` to detect bootstrap-renamed entries (e.g. local key
847+
// `b2b-invoice-end-call` vs state key `end-call-67aea057`) and live
848+
// dashboard duplicates from prior bug runs before minting another POST.
849+
async function getExistingRemoteTools(
850+
ctx: DependencyContext,
851+
): Promise<RemoteResource[]> {
852+
if (ctx.existingRemoteTools !== undefined) return ctx.existingRemoteTools;
853+
if (DRY_RUN) {
854+
// Honor dry-run: no API calls. Empty list = "no dashboard match
855+
// possible" → falls through to existing create-skip log.
856+
ctx.existingRemoteTools = [];
857+
return ctx.existingRemoteTools;
858+
}
859+
try {
860+
const remote = await fetchAllResources("tools");
861+
ctx.existingRemoteTools = remote as unknown as RemoteResource[];
862+
} catch (err) {
863+
console.warn(
864+
` ⚠️ Could not fetch dashboard tools for dedup check: ${
865+
err instanceof Error ? err.message : String(err)
866+
}. Falling back to state-only dedup.`,
867+
);
868+
ctx.existingRemoteTools = [];
869+
}
870+
return ctx.existingRemoteTools;
871+
}
872+
873+
async function getExistingRemoteStructuredOutputs(
874+
ctx: DependencyContext,
875+
): Promise<RemoteResource[]> {
876+
if (ctx.existingRemoteStructuredOutputs !== undefined)
877+
return ctx.existingRemoteStructuredOutputs;
878+
if (DRY_RUN) {
879+
ctx.existingRemoteStructuredOutputs = [];
880+
return ctx.existingRemoteStructuredOutputs;
881+
}
882+
try {
883+
const remote = await fetchAllResources("structuredOutputs");
884+
ctx.existingRemoteStructuredOutputs = remote as unknown as RemoteResource[];
885+
} catch (err) {
886+
console.warn(
887+
` ⚠️ Could not fetch dashboard structured outputs for dedup check: ${
888+
err instanceof Error ? err.message : String(err)
889+
}. Falling back to state-only dedup.`,
890+
);
891+
ctx.existingRemoteStructuredOutputs = [];
892+
}
893+
return ctx.existingRemoteStructuredOutputs;
830894
}
831895

832896
async function ensureToolExists(
@@ -843,6 +907,73 @@ async function ensureToolExists(
843907
const tool = ctx.allTools.find((t) => t.resourceId === toolId);
844908
if (!tool) return;
845909

910+
// Gap #10 dedup — before creating, check if a state entry under a
911+
// different key (bootstrap-generated like `end-call-<uuid8>`) or a
912+
// live dashboard tool already represents this same logical tool.
913+
// Adopt instead of mint a duplicate.
914+
const remoteList = await getExistingRemoteTools(ctx);
915+
const match = findExistingResourceByName({
916+
localResourceId: toolId,
917+
localPayload: tool.data as Record<string, unknown>,
918+
stateSection: ctx.state.tools,
919+
remoteList,
920+
});
921+
if (match) {
922+
if (match.ambiguous) {
923+
const payload = tool.data as Record<string, unknown>;
924+
const fn = payload.function as Record<string, unknown> | undefined;
925+
const displayName =
926+
(typeof fn?.name === "string" && fn.name) ||
927+
(typeof payload.name === "string" && payload.name) ||
928+
toolId;
929+
console.warn(
930+
` ⚠️ Multiple dashboard tools share the name "${displayName}" — adopting ${match.uuid} (lex-smallest). Other UUIDs: ${match.duplicateUuids.join(", ")}. Run \`npm run cleanup -- ${VAPI_ENV}\` to prune duplicates.`,
931+
);
932+
}
933+
console.log(
934+
` 🔁 Reusing existing tool: ${toolId}${match.uuid} (matched via ${match.source})`,
935+
);
936+
937+
// Re-key state to point at the adopted UUID under the local resourceId.
938+
// No hash yet — applyTool below will PATCH with the local payload and
939+
// record the post-PATCH hash, exercising the standard drift-check flow.
940+
upsertState(ctx.state.tools, tool.resourceId, { uuid: match.uuid });
941+
942+
// Orphan-deletion guard — drop other state keys pointing at the SAME uuid
943+
// so a subsequent full push doesn't see them as "tracked but no local file"
944+
// and DELETE the dashboard resource we just adopted. Mark them touched
945+
// so mergeScoped (Stack J) flushes the deletion. Do NOT touch entries
946+
// pointing at match.duplicateUuids — those are SEPARATE dashboard duplicates
947+
// and `npm run cleanup` is the right tool for those.
948+
for (const [staleKey, entry] of Object.entries(ctx.state.tools)) {
949+
if (staleKey !== tool.resourceId && entry.uuid === match.uuid) {
950+
delete ctx.state.tools[staleKey];
951+
ctx.touched.tools.add(staleKey);
952+
}
953+
}
954+
955+
// Now PATCH the dashboard with the local payload. applyTool's
956+
// upsertResourceWithStateRecovery branch picks PATCH because state.tools
957+
// now has existingUuid set. Drift check fires (no-baseline → log + proceed
958+
// when lastPulledHash is undefined; full check when it isn't).
959+
try {
960+
const uuid = await applyTool(tool, ctx.state);
961+
ctx.autoApplied.add(`tools:${toolId}`);
962+
if (!uuid) return;
963+
upsertState(ctx.state.tools, tool.resourceId, {
964+
uuid,
965+
lastPushedHash: hashPayload(tool.data),
966+
});
967+
ctx.applied.tools++;
968+
ctx.autoAppliedTools.push(tool);
969+
ctx.touched.tools.add(tool.resourceId);
970+
} catch (error) {
971+
console.error(formatApiError(toolId, error));
972+
throw error;
973+
}
974+
return;
975+
}
976+
846977
console.log(` 📦 Auto-applying dependency → tool: ${toolId}`);
847978
try {
848979
const uuid = await applyTool(tool, ctx.state);
@@ -854,6 +985,7 @@ async function ensureToolExists(
854985
});
855986
ctx.applied.tools++;
856987
ctx.autoAppliedTools.push(tool);
988+
ctx.touched.tools.add(tool.resourceId);
857989
} catch (error) {
858990
console.error(formatApiError(toolId, error));
859991
throw error;
@@ -876,6 +1008,72 @@ async function ensureStructuredOutputExists(
8761008
);
8771009
if (!output) return;
8781010

1011+
// Gap #10 dedup — same as ensureToolExists but against the SO state
1012+
// section + dashboard SO list.
1013+
const remoteList = await getExistingRemoteStructuredOutputs(ctx);
1014+
const match = findExistingResourceByName({
1015+
localResourceId: outputId,
1016+
localPayload: output.data as Record<string, unknown>,
1017+
stateSection: ctx.state.structuredOutputs,
1018+
remoteList,
1019+
});
1020+
if (match) {
1021+
if (match.ambiguous) {
1022+
const payload = output.data as Record<string, unknown>;
1023+
const displayName =
1024+
(typeof payload.name === "string" && payload.name) || outputId;
1025+
console.warn(
1026+
` ⚠️ Multiple dashboard structured outputs share the name "${displayName}" — adopting ${match.uuid} (lex-smallest). Other UUIDs: ${match.duplicateUuids.join(", ")}. Run \`npm run cleanup -- ${VAPI_ENV}\` to prune duplicates.`,
1027+
);
1028+
}
1029+
console.log(
1030+
` 🔁 Reusing existing structured output: ${outputId}${match.uuid} (matched via ${match.source})`,
1031+
);
1032+
1033+
// Re-key state to point at the adopted UUID under the local resourceId.
1034+
// No hash yet — applyStructuredOutput below will PATCH with the local
1035+
// payload and record the post-PATCH hash, exercising the standard
1036+
// drift-check flow.
1037+
upsertState(ctx.state.structuredOutputs, output.resourceId, {
1038+
uuid: match.uuid,
1039+
});
1040+
1041+
// Orphan-deletion guard — drop other state keys pointing at the SAME uuid
1042+
// so a subsequent full push doesn't see them as "tracked but no local file"
1043+
// and DELETE the dashboard resource we just adopted. Mark them touched
1044+
// so mergeScoped (Stack J) flushes the deletion. Do NOT touch entries
1045+
// pointing at match.duplicateUuids — those are SEPARATE dashboard duplicates
1046+
// and `npm run cleanup` is the right tool for those.
1047+
for (const [staleKey, entry] of Object.entries(ctx.state.structuredOutputs)) {
1048+
if (staleKey !== output.resourceId && entry.uuid === match.uuid) {
1049+
delete ctx.state.structuredOutputs[staleKey];
1050+
ctx.touched.structuredOutputs.add(staleKey);
1051+
}
1052+
}
1053+
1054+
// Now PATCH the dashboard with the local payload. applyStructuredOutput's
1055+
// upsertResourceWithStateRecovery branch picks PATCH because
1056+
// state.structuredOutputs now has existingUuid set. Drift check fires
1057+
// (no-baseline → log + proceed when lastPulledHash is undefined; full
1058+
// check when it isn't).
1059+
try {
1060+
const uuid = await applyStructuredOutput(output, ctx.state);
1061+
ctx.autoApplied.add(`structuredOutputs:${outputId}`);
1062+
if (!uuid) return;
1063+
upsertState(ctx.state.structuredOutputs, output.resourceId, {
1064+
uuid,
1065+
lastPushedHash: hashPayload(output.data),
1066+
});
1067+
ctx.applied.structuredOutputs++;
1068+
ctx.autoAppliedStructuredOutputs.push(output);
1069+
ctx.touched.structuredOutputs.add(output.resourceId);
1070+
} catch (error) {
1071+
console.error(formatApiError(outputId, error));
1072+
throw error;
1073+
}
1074+
return;
1075+
}
1076+
8791077
console.log(` 📦 Auto-applying dependency → structured output: ${outputId}`);
8801078
try {
8811079
const uuid = await applyStructuredOutput(output, ctx.state);
@@ -887,6 +1085,7 @@ async function ensureStructuredOutputExists(
8871085
});
8881086
ctx.applied.structuredOutputs++;
8891087
ctx.autoAppliedStructuredOutputs.push(output);
1088+
ctx.touched.structuredOutputs.add(output.resourceId);
8901089
} catch (error) {
8911090
console.error(formatApiError(outputId, error));
8921091
throw error;
@@ -1178,6 +1377,7 @@ async function main(): Promise<void> {
11781377
autoApplied,
11791378
autoAppliedTools,
11801379
autoAppliedStructuredOutputs,
1380+
touched,
11811381
};
11821382

11831383
// Determine which types to check for orphaned deletions

0 commit comments

Comments
 (0)