Skip to content

Commit 64e75ec

Browse files
committed
refactor(engine): extract shared slug + folder helpers into slug-utils
Consolidates 4+ duplicated helpers that had accumulated across the gitops engine as symptom-fixes piled up. Pure factoring, zero behavior change at every reachable call site. Duplications collapsed: - `slugify` — 4 byte-identical copies (pull.ts, dep-dedup.ts, audit.ts, setup.ts) → 1 in src/slug-utils.ts - `extractBaseSlug` — 2 byte-identical copies → 1 - `FOLDER_MAP` — 2 byte-identical copies (pull.ts, resources.ts) → 1 - `UUID_SUFFIX_RE` — open-coded in 3 places → 1 constant - recanonicalize's inlined precondition-2 check (UUID prefix match) → extracted as `isEngineSuffixedSlug` src/slug-utils.ts is config-free by design (no `./config.ts` import, no side effects at load) so it's safely importable from any test without priming process.argv / VAPI_TOKEN. This is the testability property the prior dep-dedup.ts comment claimed for its local duplicates but didn't actually enforce. Regex tightening: shared `UUID_SUFFIX_RE` uses `^(.+)-([0-9a-f]{8})$` (non-empty base) where the prior pull/dep-dedup copies used `^(.*)-...` (allowed empty base). Strict improvement — engine- generated keys always have a non-empty base, and the only input class affected is the synthetic `-<8hex>` shape which is never produced by `generateResourceId`. Pinned by a regression test in tests/slug-utils.test.ts. Back-compat: dep-dedup.ts re-exports slugify/extractBaseSlug so existing tests importing them via that path keep working. Tests: 228/228 pass (208 prior + 20 new slug-utils cases covering slugify behavior, UUID_SUFFIX_RE boundaries, extractBaseSlug loose form, isEngineSuffixedSlug strict form).
1 parent 70162a2 commit 64e75ec

8 files changed

Lines changed: 275 additions & 85 deletions

File tree

src/audit.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ import { join } from "path";
2323
import { matchesIgnore, RESOURCES_DIR } from "./config.ts";
2424
import { findOrphanResourceIds } from "./new-file-gate.ts";
2525
import {
26-
extractBaseSlug,
2726
fetchAllResources,
2827
listExistingResourceIds,
2928
type VapiResource,
3029
} from "./pull.ts";
3130
import { FOLDER_MAP } from "./resources.ts";
31+
import { extractBaseSlug, slugify } from "./slug-utils.ts";
3232
import { loadState } from "./state.ts";
3333
import type { ResourceType, StateFile } from "./types.ts";
3434
import { VALID_RESOURCE_TYPES } from "./types.ts";
@@ -132,17 +132,9 @@ async function defaultReadAssistantTools(
132132
}
133133

134134
// ─────────────────────────────────────────────────────────────────────────────
135-
// Slug helpers (kept local; mirror src/pull.ts conventions)
135+
// Resource name extraction
136136
// ─────────────────────────────────────────────────────────────────────────────
137137

138-
function slugify(name: string): string {
139-
return name
140-
.toLowerCase()
141-
.replace(/[^a-z0-9]+/g, "-")
142-
.replace(/^-+|-+$/g, "")
143-
.replace(/-+/g, "-");
144-
}
145-
146138
function extractRemoteName(resource: VapiResource): string | undefined {
147139
if (typeof resource.name === "string" && resource.name) return resource.name;
148140
// Tools store their name under function.name

src/dep-dedup.ts

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,18 @@
1717
// duplicates from prior bug runs — the caller should warn and surface
1818
// the loser UUIDs so a follow-up `npm run cleanup` can prune them).
1919
//
20-
// NOTE on duplication: `slugify` and `extractBaseSlug` here mirror the
21-
// definitions in `src/pull.ts`. pull.ts imports config.ts, which calls
22-
// `parseEnvironment()` at module load and `process.exit(1)`s without a
23-
// CLI env arg — making it impossible to import in a unit test. This
24-
// 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.
20+
// `slugify` and `extractBaseSlug` previously lived inline here as a
21+
// deliberate copy of pull.ts's definitions — pull.ts imports config.ts
22+
// which `process.exit(1)`s under unit tests, blocking direct reuse. They
23+
// now live in `./slug-utils.ts` (config-free, side-effect-free) and are
24+
// re-exported below so any existing test importing them from this module
25+
// keeps working unchanged.
2726

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

30+
export { extractBaseSlug, slugify } from "./slug-utils.ts";
31+
3032
export interface RemoteResource {
3133
id: string;
3234
name?: string;
@@ -43,19 +45,6 @@ export interface DedupMatch {
4345
duplicateUuids: string[];
4446
}
4547

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-
5948
// Minimal payload shape this module needs. Local resource files are loaded
6049
// as `Record<string, unknown>`, so the only fields we know exist are `name`
6150
// (top-level, used by SOs / assistants / squads) and a nested `function.name`

src/new-file-gate.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@
2626
// ─────────────────────────────────────────────────────────────────────────────
2727

2828
import { matchesIgnore, VAPI_ENV } from "./config.ts";
29-
import { extractBaseSlug, listExistingResourceIds } from "./pull.ts";
29+
import { listExistingResourceIds } from "./pull.ts";
3030
import { FOLDER_MAP } from "./resources.ts";
31+
import { extractBaseSlug } from "./slug-utils.ts";
3132
import type { ResourceType, StateFile } from "./types.ts";
3233
import { VALID_RESOURCE_TYPES } from "./types.ts";
3334

@@ -62,8 +63,8 @@ export interface DetectOrphanYamlsOptions {
6263
// list are reported. Mirrors `APPLY_FILTER.filePaths` semantics used by
6364
// selective push.
6465
filePathFilter?: string[];
65-
// Optional override of `extractBaseSlug`. Defaults to the pull.ts helper
66-
// — only swapped in tests to keep the unit suite filesystem-free.
66+
// Optional override of `extractBaseSlug`. Defaults to the slug-utils
67+
// helper — only swapped in tests to keep the unit suite filesystem-free.
6768
extractBaseSlug?: (resourceId: string) => string;
6869
// Optional override of `matchesIgnore`. Defaults to the config.ts helper
6970
// which reads `.vapi-ignore` from disk. Tests pass a stub so they don't

src/pull.ts

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import {
2020
formatRecanonicalizeReport,
2121
recanonicalizeStateKeys,
2222
} from "./recanonicalize.ts";
23+
import { FOLDER_MAP } from "./resources.ts";
24+
import { extractBaseSlug, slugify } from "./slug-utils.ts";
2325
import { hashPayload, loadState, saveState, upsertState } from "./state.ts";
2426
import type { ResourceState, ResourceType, StateFile } from "./types.ts";
2527

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

62-
// Map resource types to their folder paths (relative to resources/)
63-
const FOLDER_MAP: Record<ResourceType, string> = {
64-
tools: "tools",
65-
structuredOutputs: "structuredOutputs",
66-
assistants: "assistants",
67-
squads: "squads",
68-
personalities: "simulations/personalities",
69-
scenarios: "simulations/scenarios",
70-
simulations: "simulations/tests",
71-
simulationSuites: "simulations/suites",
72-
evals: "evals",
73-
};
74-
7564
// ─────────────────────────────────────────────────────────────────────────────
7665
// Git Helpers (detect locally changed files to skip during pull)
7766
// ─────────────────────────────────────────────────────────────────────────────
@@ -251,17 +240,9 @@ async function pullCredentials(state: StateFile): Promise<void> {
251240
}
252241

253242
// ─────────────────────────────────────────────────────────────────────────────
254-
// Naming & Slug Generation
243+
// Resource naming (slug generation lives in src/slug-utils.ts)
255244
// ─────────────────────────────────────────────────────────────────────────────
256245

257-
function slugify(name: string): string {
258-
return name
259-
.toLowerCase()
260-
.replace(/[^a-z0-9]+/g, "-")
261-
.replace(/^-+|-+$/g, "")
262-
.replace(/-+/g, "-");
263-
}
264-
265246
function extractName(resource: VapiResource): string | undefined {
266247
if (resource.name) return resource.name;
267248
// Tools store their name under function.name
@@ -276,11 +257,6 @@ function generateResourceId(resource: VapiResource): string {
276257
return name ? `${slugify(name)}-${shortId}` : `resource-${shortId}`;
277258
}
278259

279-
export function extractBaseSlug(resourceId: string): string {
280-
const match = resourceId.match(/^(.*)-([a-f0-9]{8})$/i);
281-
return match?.[1] ?? resourceId;
282-
}
283-
284260
export function resourceIdMatchesName(
285261
resourceId: string,
286262
resource: VapiResource,

src/recanonicalize.ts

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,11 @@ import { existsSync } from "fs";
4444
import { join } from "path";
4545
import { RESOURCES_DIR } from "./config.ts";
4646
import { FOLDER_MAP, VALID_EXTENSIONS } from "./resources.ts";
47+
import { isEngineSuffixedSlug } from "./slug-utils.ts";
4748
import type { TouchedSets } from "./state-merge.ts";
4849
import type { ResourceType, StateFile } from "./types.ts";
4950
import { VALID_RESOURCE_TYPES } from "./types.ts";
5051

51-
const UUID_SUFFIX_RE = /^(.+)-([0-9a-f]{8})$/i;
52-
5352
export interface RecanonicalizeRekey {
5453
type: ResourceType;
5554
fromKey: string;
@@ -132,21 +131,14 @@ export function recanonicalizeStateKeys(
132131
const entry = section[stateKey];
133132
if (!entry) continue;
134133

135-
const match = stateKey.match(UUID_SUFFIX_RE);
136-
if (!match) continue;
137-
138-
const [, canonicalSlug, capturedUuid8] = match;
139-
if (!canonicalSlug || !capturedUuid8) continue;
140-
141-
// Precondition 2 — only recanonicalize engine-generated suffixes. If
142-
// the captured 8 hex chars don't match the entry's UUID prefix, this
143-
// is a user-named resource that coincidentally looks suffixed.
144-
//
145-
// Mirrors generateResourceId() in src/pull.ts:265-273 — UUID dashes
146-
// start at index 8, so `slice(0, 8)` on the raw UUID is equivalent
147-
// to stripping dashes first; the dash-strip is defensive.
148-
const entryUuid8 = entry.uuid.replace(/-/g, "").slice(0, 8).toLowerCase();
149-
if (capturedUuid8.toLowerCase() !== entryUuid8) continue;
134+
// Preconditions 1 + 2 — the key must match the engine-generated
135+
// shape `<base>-<uuid8>` AND the captured 8-hex must match the
136+
// entry's UUID prefix. `isEngineSuffixedSlug` returns `null` on
137+
// either failure, ruling out user-named resources that
138+
// coincidentally end in `-abcd1234`.
139+
const parsed = isEngineSuffixedSlug(stateKey, entry.uuid);
140+
if (!parsed) continue;
141+
const canonicalSlug = parsed.base;
150142

151143
// Precondition 3 — canonical slot must be unclaimed in state.
152144
//

src/setup.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { mkdir, rm, writeFile } from "fs/promises";
55
import { dirname, join } from "path";
66
import { fileURLToPath } from "url";
77
import searchableCheckbox, { BACK_SENTINEL } from "./searchableCheckbox.js";
8+
import { slugify } from "./slug-utils.ts";
89

910
// ─────────────────────────────────────────────────────────────────────────────
1011
// Constants
@@ -152,14 +153,6 @@ async function fetchAllResourceSnapshots(
152153
// Slug helpers
153154
// ─────────────────────────────────────────────────────────────────────────────
154155

155-
function slugify(name: string): string {
156-
return name
157-
.toLowerCase()
158-
.replace(/[^a-z0-9]+/g, "-")
159-
.replace(/^-+|-+$/g, "")
160-
.replace(/-+/g, "-");
161-
}
162-
163156
// ─────────────────────────────────────────────────────────────────────────────
164157
// Dependency detection — scan selected resources for UUID references
165158
// to resources that aren't yet selected

src/slug-utils.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// ─────────────────────────────────────────────────────────────────────────────
2+
// Shared slug helpers — config-free, no side-effect imports.
3+
//
4+
// This module exists to break two duplications that previously lived across
5+
// `pull.ts`, `dep-dedup.ts`, `audit.ts`, and `setup.ts`:
6+
// - `slugify(name)` — 4 byte-identical copies
7+
// - `extractBaseSlug(resourceId)` — 2 byte-identical copies
8+
//
9+
// It also exposes the strict `isEngineSuffixedSlug` form used by
10+
// `recanonicalize.ts` to prove a state key was engine-generated (i.e. the
11+
// captured 8-hex matches the entry's UUID prefix), and the canonical
12+
// `UUID_SUFFIX_RE` constant.
13+
//
14+
// Config-free is load-bearing: `config.ts` asserts `argv[2]` / `VAPI_TOKEN`
15+
// at module load. Any test that imports a slug helper without going through
16+
// this module would otherwise have to prime `process.argv` and
17+
// `process.env.VAPI_TOKEN` (see `tests/recanonicalize.test.ts:7-8`). This
18+
// module has zero such side effects so it's safely importable from any test.
19+
// ─────────────────────────────────────────────────────────────────────────────
20+
21+
// `^(.+)-([0-9a-f]{8})$` deliberately requires a non-empty base. An engine-
22+
// generated state key always carries a real slug before the 8-hex suffix —
23+
// the synthetic `-deadbeef` shape (empty base) is never produced.
24+
export const UUID_SUFFIX_RE = /^(.+)-([0-9a-f]{8})$/i;
25+
26+
// Lowercase, replace non-alphanumeric runs with `-`, trim leading/trailing
27+
// `-`, and collapse repeated `-`. Mirrors the slug shape produced by
28+
// `generateResourceId` in `src/pull.ts` and downstream `<base>-<uuid8>`
29+
// patterns.
30+
export function slugify(name: string): string {
31+
return name
32+
.toLowerCase()
33+
.replace(/[^a-z0-9]+/g, "-")
34+
.replace(/^-+|-+$/g, "")
35+
.replace(/-+/g, "-");
36+
}
37+
38+
// Loose form: strip a trailing 8-hex segment if the resourceId matches the
39+
// engine-generated `<base>-<uuid8>` shape; otherwise return the input
40+
// unchanged. Used by callers that don't have a UUID handy (audit's
41+
// sibling-base-slug check, the orphan-gate's pairing pass, pull's
42+
// `findExistingResourceId`, dep-dedup's `extractBaseSlug` consumers).
43+
//
44+
// This intentionally does NOT verify that the captured suffix matches any
45+
// specific UUID — that proof requires `isEngineSuffixedSlug`. Loose callers
46+
// only need a best-effort canonical form.
47+
export function extractBaseSlug(resourceId: string): string {
48+
const match = resourceId.match(UUID_SUFFIX_RE);
49+
return match?.[1] ?? resourceId;
50+
}
51+
52+
// Strict form: return the parsed `{ base, suffix }` ONLY when the captured
53+
// 8 hex chars match the leading 8 hex chars of `uuid` (case-insensitive,
54+
// dashes stripped defensively). Returns `null` otherwise — including when
55+
// the resourceId doesn't match the engine shape at all.
56+
//
57+
// Use this when you have BOTH a state key AND its entry's UUID and need to
58+
// prove the key was engine-generated (the precondition-2 check in
59+
// `recanonicalize.ts`). A user-given name that coincidentally ends in
60+
// `-abcd1234` will NOT match because its UUID prefix is different.
61+
//
62+
// Mirrors `generateResourceId` in `src/pull.ts:265-273` — UUIDs have the
63+
// form `xxxxxxxx-xxxx-...` so the first 8 hex chars are dash-free, but the
64+
// dash-strip is kept as defense against malformed input.
65+
export function isEngineSuffixedSlug(
66+
stateKey: string,
67+
uuid: string,
68+
): { base: string; suffix: string } | null {
69+
const match = stateKey.match(UUID_SUFFIX_RE);
70+
if (!match) return null;
71+
const base = match[1];
72+
const capturedSuffix = match[2];
73+
if (!base || !capturedSuffix) return null;
74+
const uuidPrefix = uuid.replace(/-/g, "").slice(0, 8).toLowerCase();
75+
if (capturedSuffix.toLowerCase() !== uuidPrefix) return null;
76+
return { base, suffix: capturedSuffix.toLowerCase() };
77+
}

0 commit comments

Comments
 (0)