Skip to content

refactor(push): extract reconcileStateKeyForResource — fold two ensure-fns into one generic helper #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dhruva-reddy merged 2 commits into from
May 15, 2026
Merged

refactor(push): extract reconcileStateKeyForResource — fold two ensure-fns into one generic helper #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fd2f7b
refactor(engine): extract shared slug + folder helpers into slug-util…
dhruva-reddy May 15, 2026
4d952a4
refactor(push): extract reconcileStateKeyForResource — fold two 94-li…
dhruva-reddy May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/audit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ import { join } from "path";
import { matchesIgnore, RESOURCES_DIR } from "./config.ts";
import { findOrphanResourceIds } from "./new-file-gate.ts";
import {
extractBaseSlug,
fetchAllResources,
listExistingResourceIds,
type VapiResource,
} from "./pull.ts";
import { FOLDER_MAP } from "./resources.ts";
import { extractBaseSlug, slugify } from "./slug-utils.ts";
import { loadState } from "./state.ts";
import type { ResourceType, StateFile } from "./types.ts";
import { VALID_RESOURCE_TYPES } from "./types.ts";
Expand Down Expand Up @@ -132,17 +132,9 @@ async function defaultReadAssistantTools(
}

// ─────────────────────────────────────────────────────────────────────────────
// Slug helpers (kept local; mirror src/pull.ts conventions)
// Resource name extraction
// ─────────────────────────────────────────────────────────────────────────────

function slugify(name: string): string {
return name
.toLowerCase()
.replace(/[^a-z0-9]+/g, "-")
.replace(/^-+|-+$/g, "")
.replace(/-+/g, "-");
}

function extractRemoteName(resource: VapiResource): string | undefined {
if (typeof resource.name === "string" && resource.name) return resource.name;
// Tools store their name under function.name
Expand Down
29 changes: 9 additions & 20 deletions src/dep-dedup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@
// duplicates from prior bug runs — the caller should warn and surface
// the loser UUIDs so a follow-up `npm run cleanup` can prune them).
//
// NOTE on duplication: `slugify` and `extractBaseSlug` here mirror the
// definitions in `src/pull.ts`. pull.ts imports config.ts, which calls
// `parseEnvironment()` at module load and `process.exit(1)`s without a
// CLI env arg — making it impossible to import in a unit test. This
// module imports ONLY from `./types.ts` so it stays testable in
// isolation. Five lines duplicated is the right tradeoff; do not "DRY"
// these back into pull.ts.
// `slugify` and `extractBaseSlug` previously lived inline here as a
// deliberate copy of pull.ts's definitions — pull.ts imports config.ts
// which `process.exit(1)`s under unit tests, blocking direct reuse. They
// now live in `./slug-utils.ts` (config-free, side-effect-free) and are
// re-exported below so any existing test importing them from this module
// keeps working unchanged.

import { extractBaseSlug, slugify } from "./slug-utils.ts";
import type { ResourceState } from "./types.ts";

export { extractBaseSlug, slugify } from "./slug-utils.ts";

export interface RemoteResource {
id: string;
name?: string;
Expand All @@ -43,19 +45,6 @@ export interface DedupMatch {
duplicateUuids: string[];
}

export function slugify(name: string): string {
return name
.toLowerCase()
.replace(/[^a-z0-9]+/g, "-")
.replace(/^-+|-+$/g, "")
.replace(/-+/g, "-");
}

export function extractBaseSlug(resourceId: string): string {
const match = resourceId.match(/^(.*)-([a-f0-9]{8})$/i);
return match?.[1] ?? resourceId;
}

// Minimal payload shape this module needs. Local resource files are loaded
// as `Record<string, unknown>`, so the only fields we know exist are `name`
// (top-level, used by SOs / assistants / squads) and a nested `function.name`
Expand Down
7 changes: 4 additions & 3 deletions src/new-file-gate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
// ─────────────────────────────────────────────────────────────────────────────

import { matchesIgnore, VAPI_ENV } from "./config.ts";
import { extractBaseSlug, listExistingResourceIds } from "./pull.ts";
import { listExistingResourceIds } from "./pull.ts";
import { FOLDER_MAP } from "./resources.ts";
import { extractBaseSlug } from "./slug-utils.ts";
import type { ResourceType, StateFile } from "./types.ts";
import { VALID_RESOURCE_TYPES } from "./types.ts";

Expand Down Expand Up @@ -62,8 +63,8 @@ export interface DetectOrphanYamlsOptions {
// list are reported. Mirrors `APPLY_FILTER.filePaths` semantics used by
// selective push.
filePathFilter?: string[];
// Optional override of `extractBaseSlug`. Defaults to the pull.ts helper
// — only swapped in tests to keep the unit suite filesystem-free.
// Optional override of `extractBaseSlug`. Defaults to the slug-utils
// helper — only swapped in tests to keep the unit suite filesystem-free.
extractBaseSlug?: (resourceId: string) => string;
// Optional override of `matchesIgnore`. Defaults to the config.ts helper
// which reads `.vapi-ignore` from disk. Tests pass a stub so they don't
Expand Down
30 changes: 3 additions & 27 deletions src/pull.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
formatRecanonicalizeReport,
recanonicalizeStateKeys,
} from "./recanonicalize.ts";
import { FOLDER_MAP } from "./resources.ts";
import { extractBaseSlug, slugify } from "./slug-utils.ts";
import { hashPayload, loadState, saveState, upsertState } from "./state.ts";
import type { ResourceState, ResourceType, StateFile } from "./types.ts";

Expand Down Expand Up @@ -59,19 +61,6 @@ const ENDPOINT_MAP: Record<ResourceType, string> = {
evals: "/eval",
};

// Map resource types to their folder paths (relative to resources/)
const FOLDER_MAP: Record<ResourceType, string> = {
tools: "tools",
structuredOutputs: "structuredOutputs",
assistants: "assistants",
squads: "squads",
personalities: "simulations/personalities",
scenarios: "simulations/scenarios",
simulations: "simulations/tests",
simulationSuites: "simulations/suites",
evals: "evals",
};

// ─────────────────────────────────────────────────────────────────────────────
// Git Helpers (detect locally changed files to skip during pull)
// ─────────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -251,17 +240,9 @@ async function pullCredentials(state: StateFile): Promise<void> {
}

// ─────────────────────────────────────────────────────────────────────────────
// Naming & Slug Generation
// Resource naming (slug generation lives in src/slug-utils.ts)
// ─────────────────────────────────────────────────────────────────────────────

function slugify(name: string): string {
return name
.toLowerCase()
.replace(/[^a-z0-9]+/g, "-")
.replace(/^-+|-+$/g, "")
.replace(/-+/g, "-");
}

function extractName(resource: VapiResource): string | undefined {
if (resource.name) return resource.name;
// Tools store their name under function.name
Expand All @@ -276,11 +257,6 @@ function generateResourceId(resource: VapiResource): string {
return name ? `${slugify(name)}-${shortId}` : `resource-${shortId}`;
}

export function extractBaseSlug(resourceId: string): string {
const match = resourceId.match(/^(.*)-([a-f0-9]{8})$/i);
return match?.[1] ?? resourceId;
}

export function resourceIdMatchesName(
resourceId: string,
resource: VapiResource,
Expand Down
179 changes: 25 additions & 154 deletions src/push.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
formatRecanonicalizeReport,
recanonicalizeStateKeys,
} from "./recanonicalize.ts";
import { reconcileStateKeyForResource } from "./reconcile-state-key.ts";
import { writeSnapshot } from "./snapshot.ts";
import { mergeScoped } from "./state-merge.ts";
import {
Expand Down Expand Up @@ -947,85 +948,19 @@ async function ensureToolExists(
const tool = ctx.allTools.find((t) => t.resourceId === toolId);
if (!tool) return;

// Before creating, check whether an existing state entry (under a
// different key — e.g., bootstrap-generated `end-call-<uuid8>`) or a
// live dashboard tool already represents this same logical tool. Adopt
// instead of minting a duplicate.
const remoteList = await getExistingRemoteTools(ctx);
const match = findExistingResourceByName({
localResourceId: toolId,
localPayload: tool.data,
stateSection: ctx.state.tools,
remoteList,
await reconcileStateKeyForResource({
resourceType: "tools",
resource: tool,
state: ctx.state,
touched: ctx.touched,
applied: ctx.applied,
autoApplied: ctx.autoApplied,
pushToAutoAppliedList: (r) => ctx.autoAppliedTools.push(r),
getRemoteList: () => getExistingRemoteTools(ctx),
applyFn: applyTool,
vapiEnv: VAPI_ENV,
formatError: formatApiError,
});
if (match) {
if (match.ambiguous) {
const displayName = extractResourceName(tool.data) ?? toolId;
console.warn(
` ⚠️ 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.`,
);
}
console.log(
` 🔁 Reusing existing tool: ${toolId} → ${match.uuid} (matched via ${match.source})`,
);

// Re-key state to point at the adopted UUID under the local resourceId.
// No hash yet — applyTool below will PATCH with the local payload and
// record the post-PATCH hash, exercising the standard drift-check flow.
upsertState(ctx.state.tools, tool.resourceId, { uuid: match.uuid });

// Orphan-deletion guard — drop other state keys pointing at the SAME
// uuid so a subsequent full push doesn't see them as "tracked but no
// local file" and DELETE the dashboard resource we just adopted. Mark
// them touched so the scoped state-merge on save flushes the deletion.
// Entries pointing at `match.duplicateUuids` are SEPARATE dashboard
// duplicates — leave them alone; `npm run cleanup` handles those.
for (const [staleKey, entry] of Object.entries(ctx.state.tools)) {
if (staleKey !== tool.resourceId && entry.uuid === match.uuid) {
delete ctx.state.tools[staleKey];
ctx.touched.tools.add(staleKey);
}
}

// PATCH the dashboard with the local payload. `applyTool`'s
// `upsertResourceWithStateRecovery` branch picks PATCH because
// `state.tools` now has `existingUuid` set. Drift check fires
// (no-baseline → log + proceed when `lastPulledHash` is undefined;
// full check when it isn't).
try {
const uuid = await applyTool(tool, ctx.state);
ctx.autoApplied.add(`tools:${toolId}`);
if (!uuid) return;
upsertState(ctx.state.tools, tool.resourceId, {
uuid,
lastPushedHash: hashPayload(tool.data),
});
ctx.applied.tools++;
ctx.autoAppliedTools.push(tool);
ctx.touched.tools.add(tool.resourceId);
} catch (error) {
console.error(formatApiError(toolId, error));
throw error;
}
return;
}

console.log(` 📦 Auto-applying dependency → tool: ${toolId}`);
try {
const uuid = await applyTool(tool, ctx.state);
ctx.autoApplied.add(`tools:${toolId}`);
if (!uuid) return;
upsertState(ctx.state.tools, tool.resourceId, {
uuid,
lastPushedHash: hashPayload(tool.data),
});
ctx.applied.tools++;
ctx.autoAppliedTools.push(tool);
ctx.touched.tools.add(tool.resourceId);
} catch (error) {
console.error(formatApiError(toolId, error));
throw error;
}
}

async function ensureStructuredOutputExists(
Expand All @@ -1044,83 +979,19 @@ async function ensureStructuredOutputExists(
);
if (!output) return;

// Same dedup pattern as `ensureToolExists`, against the SO state section
// and live dashboard SO list.
const remoteList = await getExistingRemoteStructuredOutputs(ctx);
const match = findExistingResourceByName({
localResourceId: outputId,
localPayload: output.data,
stateSection: ctx.state.structuredOutputs,
remoteList,
await reconcileStateKeyForResource({
resourceType: "structuredOutputs",
resource: output,
state: ctx.state,
touched: ctx.touched,
applied: ctx.applied,
autoApplied: ctx.autoApplied,
pushToAutoAppliedList: (r) => ctx.autoAppliedStructuredOutputs.push(r),
getRemoteList: () => getExistingRemoteStructuredOutputs(ctx),
applyFn: applyStructuredOutput,
vapiEnv: VAPI_ENV,
formatError: formatApiError,
});
if (match) {
if (match.ambiguous) {
const displayName = extractResourceName(output.data) ?? outputId;
console.warn(
` ⚠️ 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.`,
);
}
console.log(
` 🔁 Reusing existing structured output: ${outputId} → ${match.uuid} (matched via ${match.source})`,
);

// Re-key state to point at the adopted UUID under the local resourceId.
// No hash yet — applyStructuredOutput below will PATCH with the local
// payload and record the post-PATCH hash, exercising the standard
// drift-check flow.
upsertState(ctx.state.structuredOutputs, output.resourceId, {
uuid: match.uuid,
});

// Orphan-deletion guard — drop other state keys pointing at the SAME
// uuid so a subsequent full push doesn't see them as "tracked but no
// local file" and DELETE the dashboard resource we just adopted. Mark
// them touched so the scoped state-merge on save flushes the deletion.
for (const [staleKey, entry] of Object.entries(
ctx.state.structuredOutputs,
)) {
if (staleKey !== output.resourceId && entry.uuid === match.uuid) {
delete ctx.state.structuredOutputs[staleKey];
ctx.touched.structuredOutputs.add(staleKey);
}
}

// PATCH via the standard apply path so drift detection fires and any
// local edits land on the dashboard.
try {
const uuid = await applyStructuredOutput(output, ctx.state);
ctx.autoApplied.add(`structuredOutputs:${outputId}`);
if (!uuid) return;
upsertState(ctx.state.structuredOutputs, output.resourceId, {
uuid,
lastPushedHash: hashPayload(output.data),
});
ctx.applied.structuredOutputs++;
ctx.autoAppliedStructuredOutputs.push(output);
ctx.touched.structuredOutputs.add(output.resourceId);
} catch (error) {
console.error(formatApiError(outputId, error));
throw error;
}
return;
}

console.log(` 📦 Auto-applying dependency → structured output: ${outputId}`);
try {
const uuid = await applyStructuredOutput(output, ctx.state);
ctx.autoApplied.add(`structuredOutputs:${outputId}`);
if (!uuid) return;
upsertState(ctx.state.structuredOutputs, output.resourceId, {
uuid,
lastPushedHash: hashPayload(output.data),
});
ctx.applied.structuredOutputs++;
ctx.autoAppliedStructuredOutputs.push(output);
ctx.touched.structuredOutputs.add(output.resourceId);
} catch (error) {
console.error(formatApiError(outputId, error));
throw error;
}
}

async function ensureAssistantDepsExist(
Expand Down
Loading