diff --git a/AGENTS.md b/AGENTS.md index 0d8dbae..0516c18 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,6 +34,7 @@ This project manages **Vapi voice agent configurations** as code. All resources | Enforcing call time limits / graceful call ending | `docs/learnings/call-duration.md` | | Voice provider field cheat-sheet (Cartesia vs 11labs vs OpenAI etc.) | `docs/learnings/voice-providers.md` | | YAML authoring conventions, .vapi-ignore lifecycle | `docs/learnings/yaml-conventions.md` | +| Centralizing repeated values with `{{variables}}` | `docs/learnings/variables.md` | | What pull/push/apply do in every drift & existence scenario | `docs/learnings/sync-behavior.md` | **Where new knowledge goes:** @@ -202,7 +203,9 @@ docs/ ├── voicemail-detection.md # Voicemail vs human classification ├── call-duration.md # Call time limits and graceful end-of-call ├── voice-providers.md # Per-provider voice block field cheat-sheet - └── yaml-conventions.md # YAML authoring conventions, .vapi-ignore lifecycle + ├── yaml-conventions.md # YAML authoring conventions, .vapi-ignore lifecycle + ├── variables.md # Managed {{variables}} in the state file + └── sync-behavior.md # pull/push/apply scenario matrix resources/ ├── / # Org-scoped resources (npm run push -- reads here) diff --git a/CLAUDE.md b/CLAUDE.md index 567d6b2..0a1139c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,6 +65,7 @@ See `docs/learnings/voice-providers.md` for related "property X should not exist - Call time limits / graceful ending → `docs/learnings/call-duration.md` - Voice provider field cheat-sheet → `docs/learnings/voice-providers.md` - YAML authoring conventions, .vapi-ignore lifecycle → `docs/learnings/yaml-conventions.md` + - Managed `{{variables}}` in the state file → `docs/learnings/variables.md` - Pull/push/apply behavior per drift & existence scenario → `docs/learnings/sync-behavior.md` This list mirrors the "Learnings & recipes" table in `AGENTS.md`. Keep both in sync — if you add a new learnings file, update both files plus `docs/learnings/README.md`. diff --git a/docs/learnings/README.md b/docs/learnings/README.md index 5b9838b..ca7098e 100644 --- a/docs/learnings/README.md +++ b/docs/learnings/README.md @@ -28,6 +28,7 @@ Each file targets a specific topic so you can load only the context you need. | Enforcing call time limits / graceful call ending | [call-duration.md](call-duration.md) | | Voice provider field cheat-sheet (Cartesia vs 11labs vs others) | [voice-providers.md](voice-providers.md) | | YAML authoring conventions, .vapi-ignore lifecycle | [yaml-conventions.md](yaml-conventions.md) | +| Centralizing repeated values with `{{variables}}` | [variables.md](variables.md) | | What will pull/push/apply do in situation X? | [sync-behavior.md](sync-behavior.md) | --- @@ -80,3 +81,4 @@ How the gitops sync engine itself behaves: |------|----------------| | [sync-behavior.md](sync-behavior.md) | The full pull/push/apply scenario matrix: state file vs hash-store baseline vs dashboard, drift directions (clean / local-ahead / dashboard-ahead / both-diverged), per-resource conflict prompt, existence cases (local-only file, dashboard-only resource, deletions either side, fresh clone, renames, legacy-state migration), `.bkp` backup copies, flag cheat sheet | | [yaml-conventions.md](yaml-conventions.md) | YAML authoring conventions, `.vapi-ignore` lifecycle | +| [variables.md](variables.md) | Managed `{{variables}}` in the state file: whole-value substitution at push, placeholder restoration at pull, type preservation, drift-stays-clean, validation of undefined names | diff --git a/docs/learnings/variables.md b/docs/learnings/variables.md new file mode 100644 index 0000000..4ca0a61 --- /dev/null +++ b/docs/learnings/variables.md @@ -0,0 +1,108 @@ +# Managed variables + +Centralize values that repeat across resources — callback URLs, a default model +name, a shared prompt fragment, a brand name — in one place, and reference them +from any resource file with a `{{name}}` placeholder. At push the placeholder is +replaced with the managed value; at pull the placeholder is preserved. + +--- + +## TL;DR + +1. Add a `variables` section to the state file `.vapi-state..json`: + ```jsonc + { + "assistants": { "...": { "uuid": "..." } }, + "variables": { + "callback_url": "https://example.com/vapi/webhook", + "default_model": "gpt-4.1", + "max_tokens": 260 + } + } + ``` +2. Reference a variable anywhere in a resource file with a **whole-value** + placeholder: + ```yaml + model: + model: "{{default_model}}" + maxTokens: "{{max_tokens}}" # becomes the NUMBER 260, not "260" + server: + url: "{{callback_url}}" + ``` +3. `npm run push -- ` substitutes the values. `npm run pull -- ` + restores the `{{...}}` placeholders. Drift detection treats a templated file + and its rendered platform resource as **in sync** — no phantom drift. + +--- + +## Rules + +- **Whole-value only.** A placeholder substitutes only when the *entire* value + is a single `{{name}}`. Embedded placeholders inside a longer string + (`"Call us at {{callback_url}}"`) are **not** substituted — they ship + verbatim. In-string interpolation is intentionally unsupported because it + cannot round-trip cleanly through pull/drift. +- **Type is preserved.** The managed value keeps its JSON type. `"{{max_tokens}}"` + with `max_tokens: 260` sends the number `260`; an object-valued variable sends + the object. (YAML needs the placeholder quoted, but the result is the native + type.) +- **Variable names** are `[A-Za-z0-9_.-]+` — letters, digits, underscore, dot, + hyphen. No spaces. Whitespace *inside* the braces is ignored: `{{ x }}` == + `{{x}}`. +- **Undefined placeholders are a validation error.** `npm run validate`/`push` + fail (or warn, without `--strict`) on any `{{name}}` with no matching entry in + `variables`, so a typo can't silently ship the literal string `"{{name}}"`. +- **Variables compose with references.** Substitution runs *before* resourceId → + UUID resolution, so a variable whose value is a resourceId works: + `toolIds: ["{{tool_ref}}"]` with `tool_ref: "my-tool"` resolves to the tool's + UUID. + +## How it round-trips (push / pull / drift) + +The state file is the single source of values. push and pull never mutate the +`variables` section — you hand-edit it; the engine preserves it verbatim. + +- **push** — `{{name}}` → value, then the usual reference/credential resolution, + then the API call. +- **drift** — the local file is hashed *with variables rendered*, and the + platform resource is already rendered, so both sides hash in one basis. A + templated resource you haven't changed reads as `clean`, never `both-diverged`. +- **pull** — the platform value is written back, but a `{{name}}` placeholder is + restored at any path where **your local file already had that placeholder and + the value still matches**. This is guided by your file, so a literal value that + merely *happens* to equal a variable's value is never rewritten into a + placeholder. If a field was changed on the dashboard so its value no longer + matches the variable, pull writes the literal value (the dashboard is now the + source of truth for that field) — re-apply the placeholder by hand if you want + it back. + +## Limitations (v1) + +- **No in-string interpolation** — whole-value only (see Rules). +- **No nested/recursive variables** — a variable whose value itself contains a + `{{...}}` placeholder is not re-resolved. Single pass. +- **No `${ENV_VAR}` expansion** — variable values are literal. For per-developer + secrets use `.env.` (loaded into `process.env`), not the committed state + file. +- **Do not store secrets in `variables`.** The state file is committed to git. + Tokens, API keys, and signing secrets belong in credentials or `.env.`. +- **`.ts` resources** keep their authored value (placeholder restoration on pull + only applies to `.yml`/`.yaml`/`.md`), consistent with how `.ts` files are + hashed. +- **Avoid `null`-valued variables.** Null leaves are dropped by the hash + canonicalization, so a `null`-valued placeholder may not round-trip cleanly + through pull/drift. Prefer a sentinel value, or omit the field. +- **Templating an entire `.md` system prompt** as a single `{{var}}` works for + the normal case (one system message). Placeholder restoration on pull matches + model messages positionally, so if the platform returns the messages in a + different order than your local file, the literal rendered prompt is written + instead of the placeholder — re-apply by hand if that happens. + +## Why the state file (and not a separate file)? + +Variables are the value analogue of the `name → { uuid }` reference maps the +resolver already consults. Keeping them in `.vapi-state..json` means one +lookup table, one commit, one merge surface. The migration guard +(`assertStateMigrated`) and `npm run migrate` explicitly exempt the `variables` +key, so its raw values are never mistaken for the legacy fat-state shape and +`migrate` never drops them. diff --git a/improvements.md b/improvements.md index b8696d5..fb2c00f 100644 --- a/improvements.md +++ b/improvements.md @@ -1298,6 +1298,53 @@ as a transactional deploy rollback until create/delete/state coverage exists. --- +## 27. Migration seam assumes every top-level state key is a `{ uuid }` section + +### Problem + +`assertStateMigrated` and `migrateOne` (`src/migrate-hash-store.ts`) treat +**every** top-level object in the state file as a `name → { uuid }` section. +Any section that legitimately holds non-uuid values trips this. + +### Current behavior + +The managed-variables feature added a `variables` section holding raw values. +Without special-casing, `assertStateMigrated` reads each variable value, finds +it isn't exactly `{ uuid }`, and throws "legacy format" — blocking every +push/pull. Worse, `migrateOne` would call `uuidOf()` on each value, find none, +and **drop the entire section** on the next `npm run migrate`. Both were fixed +by exempting the `variables` key by name (`src/migrate-hash-store.ts:99`, +`src/migrate-hash-store.ts:177`), and `loadState` loads it via +`normalizeVariables` instead of `migrateSection` (`src/state.ts`). + +### Risk + +The exemption is **by literal key name**. The next contributor who adds another +non-uuid top-level section (e.g. a future `settings` or `defaults` block) will +hit the exact same silent-drop / false-legacy trap unless they remember to add +another `=== "section-name"` guard. The failure mode for the drop is silent. + +### Current mitigation + +`variables` is exempted and covered by `tests/state-variables.test.ts` (guard +does not trip; `migrateAll` preserves the section). `docs/learnings/variables.md` +documents the constraint. + +### Possible fix + +Replace the by-name guards with a structural rule: a top-level value is a +"uuid-section" only if every entry is string-or-`{ uuid }`-shaped; otherwise +treat it as opaque and preserve verbatim. Or maintain an explicit +`NON_UUID_SECTIONS` allow-list in one place that both the guard and the +migration consult. + +### Status + +**Partially mitigated** (`variables` handled, 2026-06-24). The general +brittleness — by-name exemptions in two functions — remains open. + +--- + ## Out of scope (intentionally not improvements) - **State file is identity-only and not git-ignored.** It's intentionally diff --git a/src/audit.ts b/src/audit.ts index cbe3f98..fd4363a 100644 --- a/src/audit.ts +++ b/src/audit.ts @@ -377,7 +377,7 @@ function checkContentDrift( const entry = state[type][resourceId]; if (!entry) continue; - const localHash = hashLocalResource(type, resourceId); + const localHash = hashLocalResource(type, resourceId, state.variables); if (!localHash) continue; const remoteResource = remoteByUuid.get(entry.uuid); diff --git a/src/delete.ts b/src/delete.ts index b2282bf..eb12fb7 100644 --- a/src/delete.ts +++ b/src/delete.ts @@ -10,6 +10,7 @@ import type { ResourceState, ResourceType, StateFile, + Variables, } from "./types.ts"; // ───────────────────────────────────────────────────────────────────────────── @@ -79,11 +80,18 @@ export function findReferencingResources( targetId: string, targetType: ReferenceableType, allResources: LoadedResources, + variables: Variables = {}, ): ResourceReference[] { const referencingResources: ResourceReference[] = []; const checkResource = (resource: ResourceFile, resourceType: string) => { - const refs = extractReferencedIds(resource.data as Record); + // Pass variables so a reference via `{{placeholder}}` is resolved to the + // real resourceId — otherwise a still-referenced resource looks + // unreferenced and becomes eligible for deletion. + const refs = extractReferencedIds( + resource.data as Record, + variables, + ); if (targetType === "tools" && refs.tools.includes(targetId)) { referencingResources.push({ @@ -375,6 +383,7 @@ export async function deleteOrphanedResources( orphan.resourceId, refType, loadedResources, + state.variables, ); if (refs.length > 0) { blocked.push({ ...orphan, refs }); diff --git a/src/drift.ts b/src/drift.ts index 1b0efdf..dcbe5cd 100644 --- a/src/drift.ts +++ b/src/drift.ts @@ -154,7 +154,7 @@ export async function checkDriftForUpdate(options: { // (rare on an update path), fall back to the baseline so the direction is // dashboard-ahead rather than a phantom both-diverged. const localHash = - hashLocalResource(resourceType, resourceId) ?? baseline; + hashLocalResource(resourceType, resourceId, state.variables) ?? baseline; // Local and platform are byte-identical → there is nothing to reconcile and // the PATCH is a no-op. NEVER block here, even if the baseline disagrees diff --git a/src/migrate-hash-store.ts b/src/migrate-hash-store.ts index a8cf1bc..7801a26 100644 --- a/src/migrate-hash-store.ts +++ b/src/migrate-hash-store.ts @@ -96,6 +96,13 @@ async function migrateOne( const slim: Record> = {}; for (const [sectionKey, section] of Object.entries(raw)) { + // `variables` is a hand-authored name→value map, NOT a uuid-section. + // Preserve it verbatim — treating it as a section would try to read a + // `.uuid` off every value, find none, and DROP the whole section. + if (sectionKey === "variables") { + (slim as Record)[sectionKey] = section; + continue; + } if (!isSection(section)) { // Preserve any non-section top-level value verbatim (none expected, // but don't silently drop unknown shapes). @@ -172,7 +179,10 @@ export function assertStateMigrated(stateFilePath: string): void { return; } - for (const section of Object.values(raw)) { + for (const [sectionKey, section] of Object.entries(raw)) { + // `variables` holds raw values (not `{ uuid }`); its entries would all + // read as "legacy" and falsely trip the guard. It is never legacy. + if (sectionKey === "variables") continue; if (!isSection(section)) continue; for (const value of Object.values(section)) { if (isLegacyEntry(value)) { diff --git a/src/pull.ts b/src/pull.ts index 2661735..2d0d438 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -37,11 +37,13 @@ import { import { FOLDER_MAP, hashLocalResource, + readLocalResourceData, resolvePullScopeFromFilePaths, } from "./resources.ts"; import { extractBaseSlug, isBackupCopyFile, slugify } from "./slug-utils.ts"; import { hashPayload, loadState, saveState, upsertState } from "./state.ts"; import type { ResourceState, ResourceType, StateFile } from "./types.ts"; +import { restoreVariablePlaceholders } from "./variables.ts"; // Map resource types to their API endpoints const ENDPOINT_MAP: Record = { @@ -652,7 +654,11 @@ export async function pullResourceType( const localFile = findLocalResourcePath(folderPath, resourceId); if (localFile && baseline) { - const localHash = hashLocalResource(resourceType, resourceId); + const localHash = hashLocalResource( + resourceType, + resourceId, + state.variables, + ); if (localHash) { // Use canonicalizeForHash so platform-default mutation (_platformDefault) // and the 3-step pipeline are applied identically across pull-write, @@ -820,6 +826,18 @@ export async function pullResourceType( // replace credential UUIDs) plus the _platformDefault marker. const withCredNames = canonicalizeForHash(resource, state, credReverse); + // Guided variable restoration: re-insert `{{name}}` placeholders ONLY where + // the existing local file already had them and the platform value still + // matches the managed value. Hashing is unaffected — hashLocalResource + // renders these back to values — so this is purely about keeping the + // author's templates intact instead of clobbering them with literals. + // No-op when there are no variables or no local file. + const toWrite = restoreVariablePlaceholders( + withCredNames, + readLocalResourceData(resourceType, resourceId), + state.variables, + ) as Record; + if (bootstrap) { const icon = isPlatformDefault ? "🔒" : isNew ? "✨" : "📝"; console.log( @@ -830,7 +848,7 @@ export async function pullResourceType( const filePath = await writeResourceFile( resourceType, resourceId, - withCredNames, + toWrite, ); const icon = isPlatformDefault ? "🔒" : isNew ? "✨" : "📝"; const relPath = relative(BASE_DIR, filePath); @@ -861,7 +879,7 @@ export async function pullResourceType( // next operator to find. const diskHash = bootstrap ? null - : hashLocalResource(resourceType, resourceId); + : hashLocalResource(resourceType, resourceId, state.variables); if (!bootstrap && diskHash === null) { console.warn( ` ⚠️ ${resourceType}/${resourceId}: failed to hash post-write disk form; falling back to in-memory hash (may produce phantom drift on next pull)`, @@ -960,16 +978,28 @@ async function resolveBothDivergedResources(options: { const withCredNames = canonicalizeForHash(entry.resource, state, credReverse); + // Guided variable restoration — preserve the author's `{{name}}` templates + // for unchanged fields (mirrors the normal pull-write path). + const toWrite = restoreVariablePlaceholders( + withCredNames, + readLocalResourceData(entry.resourceType, entry.resourceId), + state.variables, + ) as Record; + await writeResourceFile( entry.resourceType, entry.resourceId, - withCredNames, + toWrite, ); console.log( ` ⬇️ ${entry.resourceId} (both diverged — resolving with --resolve=theirs, overwriting local with platform) ${formatDriftLabel("both-diverged")}`, ); // Hash the post-write disk form (same invariant as the normal pull-write path). - const diskHash = hashLocalResource(entry.resourceType, entry.resourceId); + const diskHash = hashLocalResource( + entry.resourceType, + entry.resourceId, + state.variables, + ); if (diskHash === null) { console.warn( ` ⚠️ ${entry.resourceType}/${entry.resourceId}: failed to hash post-write disk form; falling back to in-memory hash (may produce phantom drift on next pull)`, diff --git a/src/push.ts b/src/push.ts index 7cfeb0c..a509d34 100644 --- a/src/push.ts +++ b/src/push.ts @@ -38,6 +38,7 @@ import { summarizeFindings, validateNoIgnoredReferences, validateResources, + validateVariableReferences, } from "./validate.ts"; // Map a resource label to its state-file key. Used for snapshotting — @@ -1203,7 +1204,10 @@ async function ensureAssistantDepsExist( const assistant = ctx.allAssistants.find((a) => a.resourceId === assistantId); if (!assistant) return false; - const refs = extractReferencedIds(assistant.data as Record); + const refs = extractReferencedIds( + assistant.data as Record, + ctx.state.variables, + ); let depsCreated = false; for (const toolId of refs.tools) { @@ -1524,7 +1528,13 @@ async function main(): Promise { // the FORCE_DELETE-shadowed `ignorePatterns`) — even under `--force`, // a config that references an ignored resource is a contradiction the // operator should see. - ...validateNoIgnoredReferences(loadedResources, loadIgnorePatterns()), + ...validateNoIgnoredReferences( + loadedResources, + loadIgnorePatterns(), + state.variables, + ), + // Every `{{name}}` placeholder must resolve against state.variables. + ...validateVariableReferences(loadedResources, state.variables), ]; if (findings.length > 0) { console.log(summarizeFindings(findings)); @@ -1768,6 +1778,7 @@ async function main(): Promise { for (const assistant of assistants) { const refs = extractReferencedIds( assistant.data as Record, + state.variables, ); for (const toolId of refs.tools) { await ensureToolExists(toolId, depCtx); @@ -1799,6 +1810,7 @@ async function main(): Promise { for (const squad of squads) { const refs = extractReferencedIds( squad.data as Record, + state.variables, ); for (const assistantId of refs.assistants) { await ensureAssistantExists(assistantId, depCtx); diff --git a/src/resolver.ts b/src/resolver.ts index 5cde2b7..2a97ee7 100644 --- a/src/resolver.ts +++ b/src/resolver.ts @@ -1,4 +1,5 @@ -import type { ResourceState, StateFile } from "./types.ts"; +import type { ResourceState, StateFile, Variables } from "./types.ts"; +import { resolveVariables } from "./variables.ts"; // ───────────────────────────────────────────────────────────────────────────── // ID Resolution - Convert resource IDs to Vapi UUIDs @@ -179,7 +180,14 @@ export function resolveReferences( data: Record, state: StateFile, ): Record { - const resolved = JSON.parse(JSON.stringify(data)) as Record; + // Substitute managed variables FIRST, then resolve resource references. A + // variable whose value is itself a resourceId (e.g. `toolIds: ["{{t}}"]` + // where `t` = "my-tool") therefore composes: the placeholder becomes the + // resourceId, which the reference resolution below maps to a UUID. + const resolved = resolveVariables( + JSON.parse(JSON.stringify(data)), + state.variables, + ) as Record; // Resolve toolIds at root level if (Array.isArray(resolved.toolIds)) { @@ -350,7 +358,17 @@ export interface ExtractedReferences { export function extractReferencedIds( data: Record, + variables: Variables = {}, ): ExtractedReferences { + // Resolve managed variables FIRST so a reference expressed via a placeholder + // (e.g. `toolIds: ["{{tool_ref}}"]`, `tool_ref: "my-tool"`) is seen as the + // real resourceId by every reference-aware consumer: dependency + // auto-creation (push), orphan/delete protection (delete.ts), and + // ignored-reference validation. Without this the extractor returns the + // literal "{{tool_ref}}", so the dependency is never created and a + // still-referenced resource looks unreferenced. Mirrors `resolveReferences`, + // which renders variables before the same id→UUID resolution. + data = resolveVariables(data, variables) as Record; const tools: string[] = []; const structuredOutputs: string[] = []; const assistants: string[] = []; diff --git a/src/resources.ts b/src/resources.ts index ceb270f..ad3f255 100644 --- a/src/resources.ts +++ b/src/resources.ts @@ -6,7 +6,13 @@ import { BASE_DIR, matchesIgnore, RESOURCES_DIR } from "./config.ts"; import { isBackupCopyFile } from "./slug-utils.ts"; import { stateUuid } from "./state.ts"; import { hashPayload } from "./state-serialize.ts"; -import type { ResourceFile, ResourceType, StateFile } from "./types.ts"; +import type { + ResourceFile, + ResourceType, + StateFile, + Variables, +} from "./types.ts"; +import { resolveVariables } from "./variables.ts"; // Options bag for the load functions. `ignorePatterns` is the symmetric // counterpart to pull's filter: when present, ids matching any pattern are @@ -143,16 +149,42 @@ function findLocalResourceFile( export function hashLocalResource( type: ResourceType, resourceId: string, + variables: Variables = {}, ): string | null { const filePath = findLocalResourceFile(type, resourceId); if (!filePath) return null; try { - return hashPayload(parseResourceDataFromFile(filePath)); + // Render managed variables before hashing. The platform side is already + // rendered (it never saw the `{{name}}` placeholders), so rendering here + // keeps both sides in ONE basis — a variable-using file never shows + // phantom drift. No-op when `variables` is empty. + return hashPayload( + resolveVariables(parseResourceDataFromFile(filePath), variables), + ); } catch { return null; } } +/** + * Parse the on-disk form of a local resource (if any), for pull's guided + * placeholder restoration. Returns the same shape `hashLocalResource` hashes + * (`.md` body merged into `model.messages`), or undefined when no file exists + * or it fails to parse. + */ +export function readLocalResourceData( + type: ResourceType, + resourceId: string, +): Record | undefined { + const filePath = findLocalResourceFile(type, resourceId); + if (!filePath) return undefined; + try { + return parseResourceDataFromFile(filePath); + } catch { + return undefined; + } +} + /** * Recursively scan a directory for resource files (.yml, .yaml, .ts) * Warns about unsupported files found in resource directories diff --git a/src/state-merge.ts b/src/state-merge.ts index 5e4f441..5bed930 100644 --- a/src/state-merge.ts +++ b/src/state-merge.ts @@ -33,7 +33,12 @@ export interface TouchedSets { credentials: Set; } -const SECTIONS: Array = [ +// The uuid-mapped sections this merge walks. `variables` is excluded — it is +// a hand-authored value map, not a `name → { uuid }` section, and is carried +// through verbatim below (push never mutates it). +type UuidSectionKey = Exclude; + +const SECTIONS: Array = [ "tools", "structuredOutputs", "assistants", @@ -61,6 +66,8 @@ export function mergeScoped( simulations: {}, simulationSuites: {}, evals: {}, + // Carried through verbatim — a scoped push never touches variables. + variables: { ...inMemory.variables }, }; for (const section of SECTIONS) { diff --git a/src/state-serialize.ts b/src/state-serialize.ts index 9320fff..43da627 100644 --- a/src/state-serialize.ts +++ b/src/state-serialize.ts @@ -4,6 +4,7 @@ // parser in `config.ts` (which `process.exit(1)`s when no env is supplied). import { createHash } from "crypto"; +import type { StateFile } from "./types.ts"; import type { ResourceState } from "./types.ts"; // JSON.stringify replacer that emits object keys in alphabetical order at @@ -26,6 +27,44 @@ export function sortedKeysReplacer(_key: string, value: unknown): unknown { return sorted; } +// Recursively sort object keys (arrays kept in order, primitives untouched). +// The non-replacer equivalent of applying `sortedKeysReplacer` at every level — +// used by `serializeState` to pre-sort everything outside the variables subtree. +function deepSortKeys(value: unknown): unknown { + if (value === null || typeof value !== "object") return value; + if (Array.isArray(value)) return value.map(deepSortKeys); + const out: Record = {}; + for (const key of Object.keys(value as Record).sort()) { + out[key] = deepSortKeys((value as Record)[key]); + } + return out; +} + +// Serialize a StateFile for on-disk write. Byte-identical to +// `JSON.stringify(state, sortedKeysReplacer, 2)` for every uuid-section, with +// ONE deliberate exception: the `variables` subtree. Variable NAMES are sorted +// (same anti-churn rule as every other key), but each variable VALUE's internal +// object-key order is preserved verbatim — `sortedKeysReplacer` would +// alphabetize the keys of a hand-authored object value on every save, rewriting +// the operator's chosen order for no semantic gain (hashing is key-order +// insensitive). Keeping values byte-stable avoids that churn. +export function serializeState(state: StateFile): string { + const top: Record = {}; + for (const key of Object.keys(state).sort()) { + if (key === "variables") { + const vars = state.variables; + const sortedNames: Record = {}; + for (const name of Object.keys(vars).sort()) sortedNames[name] = vars[name]; + top[key] = sortedNames; + } else { + top[key] = deepSortKeys( + (state as unknown as Record)[key], + ); + } + } + return JSON.stringify(top, null, 2); +} + // Canonicalize a value: sort object keys at every level, drop null/undefined // leaves recursively, leave array order intact. Produces a stable shape // regardless of insertion order or transient nullish leaves the API may diff --git a/src/state.ts b/src/state.ts index a4ef644..712b584 100644 --- a/src/state.ts +++ b/src/state.ts @@ -4,10 +4,12 @@ import { STATE_FILE_PATH, VAPI_ENV } from "./config.ts"; import { asResourceState, hashPayload, + serializeState, sortedKeysReplacer, upsertState, } from "./state-serialize.ts"; import type { ResourceState, StateFile } from "./types.ts"; +import { normalizeVariables } from "./variables.ts"; // Re-export pure helpers so callers can import them from the same file as // loadState / saveState (less import churn) but the helpers themselves stay @@ -60,6 +62,7 @@ function createEmptyState(): StateFile { simulations: {}, simulationSuites: {}, evals: {}, + variables: {}, }; } @@ -112,6 +115,10 @@ export function loadState(): StateFile { merged.simulationSuites as Record, ), evals: migrateSection(merged.evals as Record), + // Variables are NOT uuid-shaped — load them verbatim (validated/coerced), + // never through migrateSection. push/pull leave this section untouched, so + // hand-edited values round-trip through every save. + variables: normalizeVariables(merged.variables), }; } @@ -121,7 +128,7 @@ export async function saveState(state: StateFile): Promise { // truncating it. A truncated state file would silently wipe all UUID // mappings on the next load. const tmpPath = `${STATE_FILE_PATH}.tmp`; - await writeFile(tmpPath, JSON.stringify(state, sortedKeysReplacer, 2) + "\n"); + await writeFile(tmpPath, serializeState(state) + "\n"); await rename(tmpPath, STATE_FILE_PATH); console.log(`💾 Saved state file: ${STATE_FILE_PATH}`); } diff --git a/src/types.ts b/src/types.ts index d285a43..9823d46 100644 --- a/src/types.ts +++ b/src/types.ts @@ -22,6 +22,22 @@ export interface ResourceState { uuid: string; } +// A managed variable value. Variables are authored by hand in the `variables` +// section of the state file and referenced from resource files via whole-value +// liquid placeholders (`{{name}}`). The value can be any JSON type — at push +// the placeholder is replaced by this value with its native type preserved +// (a number stays a number, an object stays an object). See `variables.ts`. +export type VariableValue = + | string + | number + | boolean + | null + | VariableValue[] + | { [key: string]: VariableValue }; + +// `name → value` map. Names are identifier-ish (`[A-Za-z0-9_.-]+`, no spaces). +export type Variables = Record; + // `StateFile` is the on-disk shape of `.vapi-state..json`. Each section // carries `Record` instead of bare strings. // `loadState()` migrates legacy data automatically. @@ -36,6 +52,11 @@ export interface StateFile { simulations: Record; simulationSuites: Record; evals: Record; + // Hand-authored centralized values, referenced from resource files via + // `{{name}}` placeholders. Unlike the other sections this is NOT a + // `name → { uuid }` map — it carries raw values. push/pull never mutate it; + // it round-trips verbatim. See `variables.ts` and `docs/learnings/variables.md`. + variables: Variables; } export interface ResourceFile> { diff --git a/src/validate-cmd.ts b/src/validate-cmd.ts index f6ab527..81db910 100644 --- a/src/validate-cmd.ts +++ b/src/validate-cmd.ts @@ -9,8 +9,13 @@ import { resolve } from "path"; import { fileURLToPath } from "url"; import { VAPI_BASE_URL, VAPI_ENV } from "./config.ts"; import { loadResources } from "./resources.ts"; +import { loadState } from "./state.ts"; import type { LoadedResources } from "./types.ts"; -import { summarizeFindings, validateResources } from "./validate.ts"; +import { + summarizeFindings, + validateResources, + validateVariableReferences, +} from "./validate.ts"; async function main(): Promise { console.log( @@ -35,7 +40,11 @@ async function main(): Promise { evals: await loadResources("evals"), }; - const findings = validateResources(resources); + const { variables } = loadState(); + const findings = [ + ...validateResources(resources), + ...validateVariableReferences(resources, variables), + ]; console.log(`\n${summarizeFindings(findings)}\n`); const errorCount = findings.filter((f) => f.severity === "error").length; diff --git a/src/validate.ts b/src/validate.ts index 8e0f68e..f9090aa 100644 --- a/src/validate.ts +++ b/src/validate.ts @@ -17,7 +17,13 @@ import { matchesIgnore } from "./config.ts"; import { extractReferencedIds } from "./resolver.ts"; import { FOLDER_MAP } from "./resources.ts"; -import type { LoadedResources, ResourceFile, ResourceType } from "./types.ts"; +import type { + LoadedResources, + ResourceFile, + ResourceType, + Variables, +} from "./types.ts"; +import { extractPlaceholders } from "./variables.ts"; export type ValidationSeverity = "warn" | "error"; @@ -457,9 +463,13 @@ function checkResourceRefs( resource: ResourceFile, type: ResourceType, ignorePatterns: string[], + variables: Variables = {}, ): ValidationFinding[] { const findings: ValidationFinding[] = []; - const refs = extractReferencedIds(resource.data as Record); + const refs = extractReferencedIds( + resource.data as Record, + variables, + ); for (const { refKey, refType } of REF_TYPE_KEYS) { const folder = FOLDER_MAP[refType]; @@ -485,13 +495,16 @@ function checkResourceRefs( export function validateNoIgnoredReferences( loaded: LoadedResources, ignorePatterns: string[], + variables: Variables = {}, ): ValidationFinding[] { if (ignorePatterns.length === 0) return []; const findings: ValidationFinding[] = []; for (const type of RESOURCE_TYPES_WITH_REFS) { for (const resource of loaded[type]) { - findings.push(...checkResourceRefs(resource, type, ignorePatterns)); + findings.push( + ...checkResourceRefs(resource, type, ignorePatterns, variables), + ); } } return findings; @@ -501,6 +514,55 @@ export function validateNoIgnoredReferences( // Public entry: run all checks // ───────────────────────────────────────────────────────────────────────────── +// ───────────────────────────────────────────────────────────────────────────── +// Check 7: Undefined variable references +// +// A resource may reference a managed variable with a whole-value `{{name}}` +// placeholder. At push the placeholder is replaced from `state.variables`; a +// name with no matching variable would be sent to the API verbatim as the +// literal string "{{name}}". Promote it to a blocking finding so the typo is +// caught before it ships a broken callback URL / model name / prompt fragment. +// ───────────────────────────────────────────────────────────────────────────── + +const ALL_RESOURCE_TYPES: ResourceType[] = [ + "tools", + "structuredOutputs", + "assistants", + "squads", + "personalities", + "scenarios", + "simulations", + "simulationSuites", + "evals", +]; + +export function validateVariableReferences( + loaded: LoadedResources, + variables: Variables, +): ValidationFinding[] { + const findings: ValidationFinding[] = []; + const defined = new Set(Object.keys(variables)); + + for (const type of ALL_RESOURCE_TYPES) { + for (const resource of loaded[type]) { + for (const name of extractPlaceholders(resource.data)) { + if (defined.has(name)) continue; + findings.push({ + severity: "error", + type, + resourceId: resource.resourceId, + rule: "undefined-variable", + message: + `references variable {{${name}}}, which is not defined in the ` + + `\`variables\` section of the state file`, + }); + } + } + } + + return findings; +} + export function validateResources( resources: LoadedResources, ): ValidationFinding[] { diff --git a/src/variables.ts b/src/variables.ts new file mode 100644 index 0000000..6cc2672 --- /dev/null +++ b/src/variables.ts @@ -0,0 +1,158 @@ +// ───────────────────────────────────────────────────────────────────────────── +// Managed variables — centralized, hand-authored values referenced from +// resource files via whole-value liquid placeholders (`{{name}}`). +// +// This is the variable analogue of canonical.ts's reference resolution: +// - resolveVariables() forward (push): {{name}} → value +// - restoreVariablePlaceholders() reverse (pull): value → {{name}} (guided) +// - extractPlaceholders() discovery (validate): list every {{name}} used +// +// WHOLE-VALUE ONLY. A placeholder substitutes only when the ENTIRE (trimmed) +// string value is a single `{{name}}`. Embedded placeholders inside a longer +// string (e.g. "Hi {{name}}") are intentionally left untouched: in-string +// interpolation is lossy and cannot round-trip cleanly through pull/drift. +// Because substitution is whole-value, the value→placeholder reverse is a +// clean, type-preserving inverse — variables slot into the same symmetric +// model the engine already uses for tool/assistant/credential references. +// +// Config-free and dependency-light (only state-serialize's pure `canonicalize`) +// so resolver.ts, resources.ts, pull.ts, and validate.ts can all import it +// without an import cycle. +// ───────────────────────────────────────────────────────────────────────────── + +import { canonicalize } from "./state-serialize.ts"; +import type { Variables } from "./types.ts"; + +// A managed variable name: identifier-ish (letters, digits, underscore, dot, +// hyphen). NO internal spaces — keeps the whole-value match unambiguous and +// mirrors the state-file key shape. Whitespace inside the braces is allowed +// and ignored, so `{{ x }}` and `{{x}}` are the same placeholder. +const PLACEHOLDER_RE = /^\{\{\s*([A-Za-z0-9_.-]+)\s*\}\}$/; + +// If `value` is a whole-value placeholder string, return the variable name; +// otherwise null. Leading/trailing whitespace around the whole scalar is +// tolerated so a YAML value that picked up stray spaces still matches. +export function parsePlaceholder(value: unknown): string | null { + if (typeof value !== "string") return null; + const match = value.trim().match(PLACEHOLDER_RE); + return match ? (match[1] as string) : null; +} + +// Order-insensitive structural equality in the SAME basis the engine hashes in +// (`canonicalize` sorts object keys and drops nullish leaves). Used to decide +// whether a platform value still equals a managed variable's value. +export function valuesEqual(a: unknown, b: unknown): boolean { + return JSON.stringify(canonicalize(a)) === JSON.stringify(canonicalize(b)); +} + +// Forward (push): deep-walk `data`, replacing every whole-value `{{name}}` +// with the managed value, native type preserved. Unknown names are left +// as-is — validate.ts surfaces them as blocking findings and push warns; +// silently dropping them here would mask the mistake. Pure: returns a new +// structure, never mutates the input. Single pass — a variable whose value +// itself contains a placeholder is NOT recursively resolved. +export function resolveVariables( + data: unknown, + variables: Variables = {}, +): unknown { + // Tolerate a nullish map: some callers read the state file by casting raw + // JSON instead of going through loadState()/normalizeVariables, so + // `state.variables` can be undefined there. Degrade to a no-op rather than + // throwing on `hasOwnProperty.call(undefined, ...)`. + if (!variables) variables = {}; + if (Array.isArray(data)) { + return data.map((item) => resolveVariables(item, variables)); + } + if (data && typeof data === "object") { + const out: Record = {}; + for (const [key, value] of Object.entries(data as Record)) { + out[key] = resolveVariables(value, variables); + } + return out; + } + const name = parsePlaceholder(data); + if (name !== null && Object.prototype.hasOwnProperty.call(variables, name)) { + // structuredClone so an object/array variable referenced from multiple + // placeholder sites can't be aliased into a shared mutable instance. + return structuredClone(variables[name]); + } + return data; +} + +// Reverse (pull), GUIDED by the previously-authored local file. Walk the +// platform `data` in parallel with the `local` structure. Wherever the local +// file had a whole-value `{{name}}` AND the platform value at that same path +// still equals the managed value, emit the ORIGINAL local placeholder string +// verbatim (preserving the author's exact text). Everywhere else emit the +// platform value unchanged. +// +// Guidance by the local file is what makes this safe: a placeholder is only +// ever re-inserted where the author already had one, so a literal value that +// merely happens to equal a variable's value is NEVER rewritten. A field +// changed on the dashboard (value no longer equals the variable) falls through +// to the literal — the caller is responsible for any "field changed" notice. +export function restoreVariablePlaceholders( + data: unknown, + local: unknown, + variables: Variables = {}, +): unknown { + if (!variables) variables = {}; + const name = parsePlaceholder(local); + if ( + name !== null && + Object.prototype.hasOwnProperty.call(variables, name) && + valuesEqual(data, variables[name]) + ) { + return local; // keep the author's exact placeholder text + } + + if (Array.isArray(data)) { + const localArr = Array.isArray(local) ? local : []; + return data.map((item, idx) => + restoreVariablePlaceholders(item, localArr[idx], variables), + ); + } + if (data && typeof data === "object") { + const localObj = + local && typeof local === "object" && !Array.isArray(local) + ? (local as Record) + : {}; + const out: Record = {}; + for (const [key, value] of Object.entries(data as Record)) { + out[key] = restoreVariablePlaceholders(value, localObj[key], variables); + } + return out; + } + return data; +} + +// Discovery (validate): every distinct whole-value placeholder name referenced +// anywhere in `data`, sorted for deterministic output. +export function extractPlaceholders(data: unknown): string[] { + const names = new Set(); + const walk = (node: unknown): void => { + if (Array.isArray(node)) { + for (const item of node) walk(item); + return; + } + if (node && typeof node === "object") { + for (const value of Object.values(node as Record)) { + walk(value); + } + return; + } + const name = parsePlaceholder(node); + if (name !== null) names.add(name); + }; + walk(data); + return [...names].sort(); +} + +// Validate + normalize a raw `variables` blob read from the state file. A +// non-object (or array) becomes `{}`; otherwise the map is copied through. +// Variable VALUES are arbitrary JSON, so there is nothing to validate at the +// leaf level (functions/undefined cannot survive JSON.parse). +export function normalizeVariables(raw: unknown): Variables { + if (!raw || typeof raw !== "object" || Array.isArray(raw)) return {}; + return { ...(raw as Variables) }; +} diff --git a/tests/resolver-variables.test.ts b/tests/resolver-variables.test.ts new file mode 100644 index 0000000..616b05e --- /dev/null +++ b/tests/resolver-variables.test.ts @@ -0,0 +1,118 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { + extractReferencedIds, + resolveReferences, +} from "../src/resolver.ts"; +import type { StateFile, Variables } from "../src/types.ts"; + +// ───────────────────────────────────────────────────────────────────────────── +// resolveReferences wiring: managed variables are substituted at push, and +// compose with the existing reference (resourceId → UUID) resolution. +// ───────────────────────────────────────────────────────────────────────────── + +const TOOL_UUID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; + +function makeState(variables: Variables): StateFile { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: { "my-tool": { uuid: TOOL_UUID } }, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + variables, + }; +} + +test("resolveReferences: substitutes a whole-value variable, type-preserving", () => { + const state = makeState({ + default_model: "gpt-4.1", + max_tokens: 260, + callback_url: "https://example.com/vapi/webhook", + }); + + const out = resolveReferences( + { + model: { model: "{{default_model}}", maxTokens: "{{max_tokens}}" }, + server: { url: "{{callback_url}}" }, + }, + state, + ) as any; + + assert.strictEqual(out.model.model, "gpt-4.1"); + assert.strictEqual(out.model.maxTokens, 260); // number preserved + assert.strictEqual(out.server.url, "https://example.com/vapi/webhook"); +}); + +test("resolveReferences: variable yielding a resourceId composes into UUID resolution", () => { + const state = makeState({ tool_ref: "my-tool" }); + + // model.toolIds references a tool via a variable. Variable substitution runs + // first (→ "my-tool"), then reference resolution maps it to the UUID. + const out = resolveReferences( + { model: { toolIds: ["{{tool_ref}}"] } }, + state, + ) as any; + + assert.deepEqual(out.model.toolIds, [TOOL_UUID]); +}); + +test("resolveReferences: literal toolId reference still resolves alongside variables", () => { + const state = makeState({ greeting: "Hello" }); + + const out = resolveReferences( + { + firstMessage: "{{greeting}}", + model: { toolIds: ["my-tool"] }, + }, + state, + ) as any; + + assert.strictEqual(out.firstMessage, "Hello"); + assert.deepEqual(out.model.toolIds, [TOOL_UUID]); +}); + +test("resolveReferences: unknown variable is left verbatim (validate/push surfaces it)", () => { + const state = makeState({}); + const out = resolveReferences({ server: { url: "{{ghost}}" } }, state) as any; + assert.strictEqual(out.server.url, "{{ghost}}"); +}); + +test("resolveReferences: no variables section behavior is unchanged", () => { + const state = makeState({}); + const out = resolveReferences( + { model: { model: "gpt-4.1", toolIds: ["my-tool"] } }, + state, + ) as any; + assert.strictEqual(out.model.model, "gpt-4.1"); + assert.deepEqual(out.model.toolIds, [TOOL_UUID]); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// extractReferencedIds must be variable-aware too, or dependency +// auto-creation / orphan-delete protection / ignored-ref validation would see +// the literal "{{...}}" instead of the real resourceId (review finding #1). +// ───────────────────────────────────────────────────────────────────────────── + +test("extractReferencedIds: resolves a placeholder reference to the real resourceId", () => { + const data = { model: { toolIds: ["{{tool_ref}}"] } }; + const refs = extractReferencedIds(data, { tool_ref: "my-tool" }); + assert.deepEqual(refs.tools, ["my-tool"]); +}); + +test("extractReferencedIds: without variables, a placeholder stays literal", () => { + const data = { model: { toolIds: ["{{tool_ref}}"] } }; + const refs = extractReferencedIds(data); + assert.deepEqual(refs.tools, ["{{tool_ref}}"]); +}); + +test("extractReferencedIds: literal references still extracted alongside placeholders", () => { + const data = { model: { toolIds: ["literal-tool", "{{tool_ref}}"] } }; + const refs = extractReferencedIds(data, { tool_ref: "my-tool" }); + assert.deepEqual(refs.tools.sort(), ["literal-tool", "my-tool"]); +}); diff --git a/tests/state-variables.test.ts b/tests/state-variables.test.ts new file mode 100644 index 0000000..30e3781 --- /dev/null +++ b/tests/state-variables.test.ts @@ -0,0 +1,165 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { + assertStateMigrated, + migrateAll, +} from "../src/migrate-hash-store.ts"; +import { + serializeState, + sortedKeysReplacer, +} from "../src/state-serialize.ts"; +import type { StateFile } from "../src/types.ts"; + +// ───────────────────────────────────────────────────────────────────────────── +// State-file integration for the `variables` section. +// +// The migration seam treats every top-level object as a `name → { uuid }` +// section. The `variables` section is the exception: it holds raw values, so +// it MUST be exempted from both the legacy-format guard (or it would block +// every push/pull) and the slimming rewrite (or `npm run migrate` would drop +// it). These specs pin that exemption. +// ───────────────────────────────────────────────────────────────────────────── + +function withTempDir(fn: (dir: string) => void): void { + const dir = mkdtempSync(join(tmpdir(), "vapi-state-vars-")); + try { + fn(dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test("assertStateMigrated: a variables section does NOT trip the legacy guard", () => { + withTempDir((dir) => { + const statePath = join(dir, ".vapi-state.acme.json"); + writeFileSync( + statePath, + JSON.stringify({ + tools: { "my-tool": { uuid: "11111111-1111-1111-1111-111111111111" } }, + variables: { + callback_url: "https://example.com/vapi/webhook", + max_tokens: 260, + server: { url: "https://example.com/srv", timeoutSeconds: 20 }, + }, + }), + ); + // Must NOT throw — variable values are raw (not `{ uuid }`), and would + // otherwise look exactly like the legacy fat-state shape. + assert.doesNotThrow(() => assertStateMigrated(statePath)); + }); +}); + +test("assertStateMigrated: still throws on a genuinely legacy uuid-section entry", () => { + withTempDir((dir) => { + const statePath = join(dir, ".vapi-state.acme.json"); + writeFileSync( + statePath, + JSON.stringify({ + tools: { + "my-tool": { + uuid: "11111111-1111-1111-1111-111111111111", + lastPulledHash: "deadbeef", // legacy fat field + }, + }, + variables: { callback_url: "https://example.com/hook" }, + }), + ); + assert.throws(() => assertStateMigrated(statePath), /legacy format/); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// serializeState — variable VALUE key order is preserved; everything else is +// byte-identical to the prior sortedKeysReplacer serialization (review #2). +// ───────────────────────────────────────────────────────────────────────────── + +function emptySections(): Omit { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +test("serializeState: preserves hand-authored object-variable key order", () => { + const state: StateFile = { + ...emptySections(), + variables: { + // intentionally NOT alphabetical — must round-trip in this order + server: { url: "https://example.com/srv", timeoutSeconds: 20, retries: 3 }, + }, + }; + const json = serializeState(state); + const keysInOrder = json.match(/"url"|"timeoutSeconds"|"retries"/g); + assert.deepEqual(keysInOrder, ['"url"', '"timeoutSeconds"', '"retries"']); +}); + +test("serializeState: sorts variable NAMES (anti-churn) but not value internals", () => { + const state: StateFile = { + ...emptySections(), + variables: { zeta: 1, alpha: 2 }, + }; + const json = serializeState(state); + assert.ok(json.indexOf('"alpha"') < json.indexOf('"zeta"')); +}); + +test("serializeState: byte-identical to sortedKeysReplacer for a variable-free state", () => { + const state: StateFile = { + ...emptySections(), + tools: { + "b-tool": { uuid: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" }, + "a-tool": { uuid: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" }, + }, + variables: {}, + }; + assert.equal( + serializeState(state), + JSON.stringify(state, sortedKeysReplacer, 2), + ); +}); + +test("migrateAll: preserves the variables section verbatim while slimming legacy entries", async () => { + const dir = mkdtempSync(join(tmpdir(), "vapi-state-vars-")); + try { + const statePath = join(dir, ".vapi-state.testorg.json"); + // Bare-string tool entry forces a slim rewrite; it carries no hash, so + // migrateAll writes nothing to the real hash store (no side effects). + writeFileSync( + statePath, + JSON.stringify({ + tools: { foo: "22222222-2222-2222-2222-222222222222" }, + variables: { + callback_url: "https://example.com/vapi/webhook", + max_tokens: 260, + server: { url: "https://example.com/srv" }, + }, + }), + ); + + await migrateAll(dir); + + const rewritten = JSON.parse(readFileSync(statePath, "utf-8")); + // Legacy entry slimmed to `{ uuid }`. + assert.deepEqual(rewritten.tools, { + foo: { uuid: "22222222-2222-2222-2222-222222222222" }, + }); + // Variables survived untouched. + assert.deepEqual(rewritten.variables, { + callback_url: "https://example.com/vapi/webhook", + max_tokens: 260, + server: { url: "https://example.com/srv" }, + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/tests/variables.test.ts b/tests/variables.test.ts new file mode 100644 index 0000000..4497ddf --- /dev/null +++ b/tests/variables.test.ts @@ -0,0 +1,226 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { + extractPlaceholders, + normalizeVariables, + parsePlaceholder, + resolveVariables, + restoreVariablePlaceholders, + valuesEqual, +} from "../src/variables.ts"; +import type { Variables } from "../src/types.ts"; + +// ───────────────────────────────────────────────────────────────────────────── +// parsePlaceholder — whole-value match only +// ───────────────────────────────────────────────────────────────────────────── + +test("parsePlaceholder: matches a bare whole-value placeholder", () => { + assert.equal(parsePlaceholder("{{callback_url}}"), "callback_url"); +}); + +test("parsePlaceholder: tolerates inner and outer whitespace", () => { + assert.equal(parsePlaceholder("{{ callback_url }}"), "callback_url"); + assert.equal(parsePlaceholder(" {{callback_url}} "), "callback_url"); +}); + +test("parsePlaceholder: allows dot/hyphen/underscore names", () => { + assert.equal(parsePlaceholder("{{default.model}}"), "default.model"); + assert.equal(parsePlaceholder("{{call-back_url}}"), "call-back_url"); +}); + +test("parsePlaceholder: rejects embedded (in-string) placeholders", () => { + // Whole-value only — an embedded placeholder is NOT a match. + assert.equal(parsePlaceholder("Hi {{name}}"), null); + assert.equal(parsePlaceholder("{{a}}{{b}}"), null); +}); + +test("parsePlaceholder: rejects non-strings and names with spaces", () => { + assert.equal(parsePlaceholder(42), null); + assert.equal(parsePlaceholder(null), null); + assert.equal(parsePlaceholder({}), null); + assert.equal(parsePlaceholder("{{two words}}"), null); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// resolveVariables — forward (push), type-preserving +// ───────────────────────────────────────────────────────────────────────────── + +const VARS: Variables = { + callback_url: "https://example.com/vapi/webhook", + default_model: "gpt-4.1", + max_tokens: 260, + flag: true, + server_obj: { url: "https://example.com/srv", timeoutSeconds: 20 }, +}; + +test("resolveVariables: replaces a whole-value placeholder with a string", () => { + const out = resolveVariables( + { server: { url: "{{callback_url}}" } }, + VARS, + ); + assert.deepEqual(out, { + server: { url: "https://example.com/vapi/webhook" }, + }); +}); + +test("resolveVariables: preserves the native type (number, boolean, object)", () => { + const out = resolveVariables( + { + model: { maxTokens: "{{max_tokens}}", stream: "{{flag}}" }, + server: "{{server_obj}}", + }, + VARS, + ) as Record; + assert.strictEqual(out.model.maxTokens, 260); // number, not "260" + assert.strictEqual(out.model.stream, true); // boolean, not "true" + assert.deepEqual(out.server, { + url: "https://example.com/srv", + timeoutSeconds: 20, + }); +}); + +test("resolveVariables: substitutes inside arrays", () => { + const out = resolveVariables( + { messages: ["{{default_model}}", "literal"] }, + VARS, + ); + assert.deepEqual(out, { messages: ["gpt-4.1", "literal"] }); +}); + +test("resolveVariables: leaves embedded placeholders untouched", () => { + const out = resolveVariables({ prompt: "Use {{default_model}} now" }, VARS); + assert.deepEqual(out, { prompt: "Use {{default_model}} now" }); +}); + +test("resolveVariables: leaves unknown placeholders untouched", () => { + const out = resolveVariables({ x: "{{nope}}" }, VARS); + assert.deepEqual(out, { x: "{{nope}}" }); +}); + +test("resolveVariables: does not mutate the input", () => { + const input = { server: { url: "{{callback_url}}" } }; + const snapshot = JSON.parse(JSON.stringify(input)); + resolveVariables(input, VARS); + assert.deepEqual(input, snapshot); +}); + +test("resolveVariables: object values are not aliased across sites", () => { + const out = resolveVariables( + { a: "{{server_obj}}", b: "{{server_obj}}" }, + VARS, + ) as Record; + out.a.url = "mutated"; + assert.strictEqual(out.b.url, "https://example.com/srv"); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// restoreVariablePlaceholders — reverse (pull), guided by the local file +// ───────────────────────────────────────────────────────────────────────────── + +test("restore: re-inserts the placeholder where local had one and value matches", () => { + const platform = { server: { url: "https://example.com/vapi/webhook" } }; + const local = { server: { url: "{{callback_url}}" } }; + const out = restoreVariablePlaceholders(platform, local, VARS); + assert.deepEqual(out, { server: { url: "{{callback_url}}" } }); +}); + +test("restore: preserves the author's exact placeholder text (spacing)", () => { + const platform = { url: "https://example.com/vapi/webhook" }; + const local = { url: "{{ callback_url }}" }; + const out = restoreVariablePlaceholders(platform, local, VARS) as any; + assert.strictEqual(out.url, "{{ callback_url }}"); +}); + +test("restore: type-preserving inverse of resolveVariables (number)", () => { + const platform = { model: { maxTokens: 260 } }; + const local = { model: { maxTokens: "{{max_tokens}}" } }; + const out = restoreVariablePlaceholders(platform, local, VARS); + assert.deepEqual(out, { model: { maxTokens: "{{max_tokens}}" } }); +}); + +test("restore: does NOT templatize a literal that merely equals a value", () => { + // local had a literal here (no placeholder) → must stay literal even though + // it equals the managed value. This is the false-positive guard. + const platform = { model: { model: "gpt-4.1" } }; + const local = { model: { model: "gpt-4.1" } }; + const out = restoreVariablePlaceholders(platform, local, VARS); + assert.deepEqual(out, { model: { model: "gpt-4.1" } }); +}); + +test("restore: drops the placeholder when the dashboard changed the value", () => { + // local had {{callback_url}} but the platform value no longer matches → + // write the literal (dashboard is now source of truth for that field). + const platform = { server: { url: "https://CHANGED.example/hook" } }; + const local = { server: { url: "{{callback_url}}" } }; + const out = restoreVariablePlaceholders(platform, local, VARS); + assert.deepEqual(out, { server: { url: "https://CHANGED.example/hook" } }); +}); + +test("restore: no local file → platform values pass through verbatim", () => { + const platform = { server: { url: "https://example.com/vapi/webhook" } }; + const out = restoreVariablePlaceholders(platform, undefined, VARS); + assert.deepEqual(out, platform); +}); + +test("restore: round-trips resolveVariables (resolve→restore is identity)", () => { + const local = { + name: "Acme", + server: { url: "{{callback_url}}" }, + model: { model: "{{default_model}}", maxTokens: "{{max_tokens}}" }, + }; + const rendered = resolveVariables(local, VARS); + const restored = restoreVariablePlaceholders(rendered, local, VARS); + assert.deepEqual(restored, local); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// extractPlaceholders + normalizeVariables +// ───────────────────────────────────────────────────────────────────────────── + +test("extractPlaceholders: collects distinct names, sorted", () => { + const names = extractPlaceholders({ + a: "{{zeta}}", + b: ["{{alpha}}", "literal", "{{zeta}}"], + c: { d: "{{beta}}", e: "Hi {{ignored}}" }, + }); + assert.deepEqual(names, ["alpha", "beta", "zeta"]); +}); + +test("normalizeVariables: passes a flat map through", () => { + assert.deepEqual(normalizeVariables({ a: 1, b: "x" }), { a: 1, b: "x" }); +}); + +test("normalizeVariables: coerces non-objects to {}", () => { + assert.deepEqual(normalizeVariables(undefined), {}); + assert.deepEqual(normalizeVariables(null), {}); + assert.deepEqual(normalizeVariables([1, 2]), {}); + assert.deepEqual(normalizeVariables("nope"), {}); +}); + +test("valuesEqual: order-insensitive structural equality", () => { + assert.ok(valuesEqual({ a: 1, b: 2 }, { b: 2, a: 1 })); + assert.ok(!valuesEqual({ a: 1 }, { a: 2 })); + assert.ok(valuesEqual("gpt-4.1", "gpt-4.1")); +}); + +// Defensive: callers that bypass loadState() (cast raw JSON) can pass an +// undefined variables map — must degrade to a no-op, not throw. +test("resolveVariables: tolerates an undefined variables map", () => { + assert.doesNotThrow(() => + resolveVariables({ x: "{{anything}}" }, undefined as unknown as Variables), + ); + assert.deepEqual( + resolveVariables({ x: "{{anything}}" }, undefined as unknown as Variables), + { x: "{{anything}}" }, + ); +}); + +test("restoreVariablePlaceholders: tolerates an undefined variables map", () => { + assert.doesNotThrow(() => + restoreVariablePlaceholders( + { x: "v" }, + { x: "{{a}}" }, + undefined as unknown as Variables, + ), + ); +});