Skip to content

Commit fd8c23e

Browse files
dhruva-vapidhruva-vapi
authored
refactor(push): extract reconcileStateKeyForResource — fold two 94-line ensure-fns into one generic helper (#36)
`ensureToolExists` and `ensureStructuredOutputExists` were structurally identical 94-line functions differing only in: resource type label, apply function, state section, remote-list cache, and the per-type bookkeeping array. Both implemented the same dedup-then-apply flow: look up existing dashboard/state match by canonical name, adopt with orphan-deletion guard, mark `touched` for `mergeScoped`, call apply. Behavioral contract preserved exactly: - Log strings byte-identical (verified path-by-path against pre-refactor) - `autoApplied.add` BEFORE the `if (!uuid) return` early-exit - `applied[type]++`, `pushToAutoAppliedList`, `touched.add` AFTER the null check — preserves dry-run / drift-halt semantics - Orphan-deletion guard scope unchanged: deletes state keys pointing at the adopted UUID, leaves `duplicateUuids` alone for `npm run cleanup` to handle - try/catch boundary identical - All 228 prior tests pass unchanged, including the integration test in tests/push-dry-run.test.ts push.ts shrunk -129 net LOC (two 94-line functions collapsed to ~14-line wrappers). Helper is `tools | structuredOutputs` narrow today; adding a future type requires a deliberate union widening + LABELS map entry, not a config flag. `vapiEnv` and `formatError` parameters are required (not optional with placeholder defaults) so a future caller can't accidentally emit a degraded warning or error message. Tests: 244/244 pass (228 prior + 16 new — 8 scenarios × 2 resource types covering happy path, ambiguous match, null applyFn ordering contract, orphan-deletion guard scope, run-scoped idempotency, state-hit/dashboard-hit/no-match branches). Closes the symptom-fix pattern documented in improvements.md #10 (now handled by the generic helper instead of the two hardcoded functions).
1 parent 3fd2f7b commit fd8c23e

3 files changed

Lines changed: 674 additions & 154 deletions

File tree

‎src/push.ts‎

Lines changed: 25 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
formatRecanonicalizeReport,
2727
recanonicalizeStateKeys,
2828
} from "./recanonicalize.ts";
29+
import { reconcileStateKeyForResource } from "./reconcile-state-key.ts";
2930
import { writeSnapshot } from "./snapshot.ts";
3031
import { mergeScoped } from "./state-merge.ts";
3132
import {
@@ -947,85 +948,19 @@ async function ensureToolExists(
947948
const tool = ctx.allTools.find((t) => t.resourceId === toolId);
948949
if (!tool) return;
949950

950-
// Before creating, check whether an existing state entry (under a
951-
// different key — e.g., bootstrap-generated `end-call-<uuid8>`) or a
952-
// live dashboard tool already represents this same logical tool. Adopt
953-
// instead of minting a duplicate.
954-
const remoteList = await getExistingRemoteTools(ctx);
955-
const match = findExistingResourceByName({
956-
localResourceId: toolId,
957-
localPayload: tool.data,
958-
stateSection: ctx.state.tools,
959-
remoteList,
951+
await reconcileStateKeyForResource({
952+
resourceType: "tools",
953+
resource: tool,
954+
state: ctx.state,
955+
touched: ctx.touched,
956+
applied: ctx.applied,
957+
autoApplied: ctx.autoApplied,
958+
pushToAutoAppliedList: (r) => ctx.autoAppliedTools.push(r),
959+
getRemoteList: () => getExistingRemoteTools(ctx),
960+
applyFn: applyTool,
961+
vapiEnv: VAPI_ENV,
962+
formatError: formatApiError,
960963
});
961-
if (match) {
962-
if (match.ambiguous) {
963-
const displayName = extractResourceName(tool.data) ?? toolId;
964-
console.warn(
965-
` ⚠️ 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.`,
966-
);
967-
}
968-
console.log(
969-
` 🔁 Reusing existing tool: ${toolId} → ${match.uuid} (matched via ${match.source})`,
970-
);
971-
972-
// Re-key state to point at the adopted UUID under the local resourceId.
973-
// No hash yet — applyTool below will PATCH with the local payload and
974-
// record the post-PATCH hash, exercising the standard drift-check flow.
975-
upsertState(ctx.state.tools, tool.resourceId, { uuid: match.uuid });
976-
977-
// Orphan-deletion guard — drop other state keys pointing at the SAME
978-
// uuid so a subsequent full push doesn't see them as "tracked but no
979-
// local file" and DELETE the dashboard resource we just adopted. Mark
980-
// them touched so the scoped state-merge on save flushes the deletion.
981-
// Entries pointing at `match.duplicateUuids` are SEPARATE dashboard
982-
// duplicates — leave them alone; `npm run cleanup` handles those.
983-
for (const [staleKey, entry] of Object.entries(ctx.state.tools)) {
984-
if (staleKey !== tool.resourceId && entry.uuid === match.uuid) {
985-
delete ctx.state.tools[staleKey];
986-
ctx.touched.tools.add(staleKey);
987-
}
988-
}
989-
990-
// PATCH the dashboard with the local payload. `applyTool`'s
991-
// `upsertResourceWithStateRecovery` branch picks PATCH because
992-
// `state.tools` now has `existingUuid` set. Drift check fires
993-
// (no-baseline → log + proceed when `lastPulledHash` is undefined;
994-
// full check when it isn't).
995-
try {
996-
const uuid = await applyTool(tool, ctx.state);
997-
ctx.autoApplied.add(`tools:${toolId}`);
998-
if (!uuid) return;
999-
upsertState(ctx.state.tools, tool.resourceId, {
1000-
uuid,
1001-
lastPushedHash: hashPayload(tool.data),
1002-
});
1003-
ctx.applied.tools++;
1004-
ctx.autoAppliedTools.push(tool);
1005-
ctx.touched.tools.add(tool.resourceId);
1006-
} catch (error) {
1007-
console.error(formatApiError(toolId, error));
1008-
throw error;
1009-
}
1010-
return;
1011-
}
1012-
1013-
console.log(` 📦 Auto-applying dependency → tool: ${toolId}`);
1014-
try {
1015-
const uuid = await applyTool(tool, ctx.state);
1016-
ctx.autoApplied.add(`tools:${toolId}`);
1017-
if (!uuid) return;
1018-
upsertState(ctx.state.tools, tool.resourceId, {
1019-
uuid,
1020-
lastPushedHash: hashPayload(tool.data),
1021-
});
1022-
ctx.applied.tools++;
1023-
ctx.autoAppliedTools.push(tool);
1024-
ctx.touched.tools.add(tool.resourceId);
1025-
} catch (error) {
1026-
console.error(formatApiError(toolId, error));
1027-
throw error;
1028-
}
1029964
}
1030965

1031966
async function ensureStructuredOutputExists(
@@ -1044,83 +979,19 @@ async function ensureStructuredOutputExists(
1044979
);
1045980
if (!output) return;
1046981

1047-
// Same dedup pattern as `ensureToolExists`, against the SO state section
1048-
// and live dashboard SO list.
1049-
const remoteList = await getExistingRemoteStructuredOutputs(ctx);
1050-
const match = findExistingResourceByName({
1051-
localResourceId: outputId,
1052-
localPayload: output.data,
1053-
stateSection: ctx.state.structuredOutputs,
1054-
remoteList,
982+
await reconcileStateKeyForResource({
983+
resourceType: "structuredOutputs",
984+
resource: output,
985+
state: ctx.state,
986+
touched: ctx.touched,
987+
applied: ctx.applied,
988+
autoApplied: ctx.autoApplied,
989+
pushToAutoAppliedList: (r) => ctx.autoAppliedStructuredOutputs.push(r),
990+
getRemoteList: () => getExistingRemoteStructuredOutputs(ctx),
991+
applyFn: applyStructuredOutput,
992+
vapiEnv: VAPI_ENV,
993+
formatError: formatApiError,
1055994
});
1056-
if (match) {
1057-
if (match.ambiguous) {
1058-
const displayName = extractResourceName(output.data) ?? outputId;
1059-
console.warn(
1060-
` ⚠️ 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.`,
1061-
);
1062-
}
1063-
console.log(
1064-
` 🔁 Reusing existing structured output: ${outputId} → ${match.uuid} (matched via ${match.source})`,
1065-
);
1066-
1067-
// Re-key state to point at the adopted UUID under the local resourceId.
1068-
// No hash yet — applyStructuredOutput below will PATCH with the local
1069-
// payload and record the post-PATCH hash, exercising the standard
1070-
// drift-check flow.
1071-
upsertState(ctx.state.structuredOutputs, output.resourceId, {
1072-
uuid: match.uuid,
1073-
});
1074-
1075-
// Orphan-deletion guard — drop other state keys pointing at the SAME
1076-
// uuid so a subsequent full push doesn't see them as "tracked but no
1077-
// local file" and DELETE the dashboard resource we just adopted. Mark
1078-
// them touched so the scoped state-merge on save flushes the deletion.
1079-
for (const [staleKey, entry] of Object.entries(
1080-
ctx.state.structuredOutputs,
1081-
)) {
1082-
if (staleKey !== output.resourceId && entry.uuid === match.uuid) {
1083-
delete ctx.state.structuredOutputs[staleKey];
1084-
ctx.touched.structuredOutputs.add(staleKey);
1085-
}
1086-
}
1087-
1088-
// PATCH via the standard apply path so drift detection fires and any
1089-
// local edits land on the dashboard.
1090-
try {
1091-
const uuid = await applyStructuredOutput(output, ctx.state);
1092-
ctx.autoApplied.add(`structuredOutputs:${outputId}`);
1093-
if (!uuid) return;
1094-
upsertState(ctx.state.structuredOutputs, output.resourceId, {
1095-
uuid,
1096-
lastPushedHash: hashPayload(output.data),
1097-
});
1098-
ctx.applied.structuredOutputs++;
1099-
ctx.autoAppliedStructuredOutputs.push(output);
1100-
ctx.touched.structuredOutputs.add(output.resourceId);
1101-
} catch (error) {
1102-
console.error(formatApiError(outputId, error));
1103-
throw error;
1104-
}
1105-
return;
1106-
}
1107-
1108-
console.log(` 📦 Auto-applying dependency → structured output: ${outputId}`);
1109-
try {
1110-
const uuid = await applyStructuredOutput(output, ctx.state);
1111-
ctx.autoApplied.add(`structuredOutputs:${outputId}`);
1112-
if (!uuid) return;
1113-
upsertState(ctx.state.structuredOutputs, output.resourceId, {
1114-
uuid,
1115-
lastPushedHash: hashPayload(output.data),
1116-
});
1117-
ctx.applied.structuredOutputs++;
1118-
ctx.autoAppliedStructuredOutputs.push(output);
1119-
ctx.touched.structuredOutputs.add(output.resourceId);
1120-
} catch (error) {
1121-
console.error(formatApiError(outputId, error));
1122-
throw error;
1123-
}
1124995
}
1125996

1126997
async function ensureAssistantDepsExist(

0 commit comments

Comments
 (0)