From 4d952a41616d731b1e3c96f32e98aa1b036a35c0 Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Thu, 14 May 2026 19:41:02 -0700 Subject: [PATCH] =?UTF-8?q?refactor(push):=20extract=20reconcileStateKeyFo?= =?UTF-8?q?rResource=20=E2=80=94=20fold=20two=2094-line=20ensure-fns=20int?= =?UTF-8?q?o=20one=20generic=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ensureToolExists` and `ensureStructuredOutputExists` were structurally identical 94-line functions differing only in: resource type label, apply function, state section, remote-list cache, and the per-type bookkeeping array. Both implemented the same dedup-then-apply flow: look up existing dashboard/state match by canonical name, adopt with orphan-deletion guard, mark `touched` for `mergeScoped`, call apply. Behavioral contract preserved exactly: - Log strings byte-identical (verified path-by-path against pre-refactor) - `autoApplied.add` BEFORE the `if (!uuid) return` early-exit - `applied[type]++`, `pushToAutoAppliedList`, `touched.add` AFTER the null check — preserves dry-run / drift-halt semantics - Orphan-deletion guard scope unchanged: deletes state keys pointing at the adopted UUID, leaves `duplicateUuids` alone for `npm run cleanup` to handle - try/catch boundary identical - All 228 prior tests pass unchanged, including the integration test in tests/push-dry-run.test.ts push.ts shrunk -129 net LOC (two 94-line functions collapsed to ~14-line wrappers). Helper is `tools | structuredOutputs` narrow today; adding a future type requires a deliberate union widening + LABELS map entry, not a config flag. `vapiEnv` and `formatError` parameters are required (not optional with placeholder defaults) so a future caller can't accidentally emit a degraded warning or error message. Tests: 244/244 pass (228 prior + 16 new — 8 scenarios × 2 resource types covering happy path, ambiguous match, null applyFn ordering contract, orphan-deletion guard scope, run-scoped idempotency, state-hit/dashboard-hit/no-match branches). Closes the symptom-fix pattern documented in improvements.md #10 (now handled by the generic helper instead of the two hardcoded functions). --- src/push.ts | 179 ++---------- src/reconcile-state-key.ts | 205 ++++++++++++++ tests/reconcile-state-key.test.ts | 444 ++++++++++++++++++++++++++++++ 3 files changed, 674 insertions(+), 154 deletions(-) create mode 100644 src/reconcile-state-key.ts create mode 100644 tests/reconcile-state-key.test.ts diff --git a/src/push.ts b/src/push.ts index b40183f..d1d3757 100644 --- a/src/push.ts +++ b/src/push.ts @@ -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 { @@ -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-`) 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( @@ -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( diff --git a/src/reconcile-state-key.ts b/src/reconcile-state-key.ts new file mode 100644 index 0000000..9291747 --- /dev/null +++ b/src/reconcile-state-key.ts @@ -0,0 +1,205 @@ +// Generic state-key reconciliation for auto-applied dependencies. +// +// Both `ensureToolExists` and `ensureStructuredOutputExists` in `push.ts` +// share a 94-line body that: +// +// 1. Probes for an existing dashboard / state resource whose canonical +// name matches the local payload (`findExistingResourceByName`), so a +// bootstrap-renamed state entry (`-`) or a live dashboard +// twin isn't shadowed by a brand-new POST. +// 2. On match: rekeys state to the adopted UUID, runs the orphan-deletion +// guard (drop OTHER state keys pointing at the same UUID so a +// subsequent push doesn't DELETE the freshly-adopted dashboard +// resource), and PATCHes via the standard apply path so drift +// detection fires. +// 3. On no match: logs the auto-apply line and runs the standard apply +// path (which creates). +// 4. In BOTH branches: records `autoApplied` even when the apply returns +// `null` (the apply-path short-circuit that signals "skipped"), then +// conditionally updates state with the post-PATCH hash, increments the +// applied counter, pushes onto the per-type bookkeeping array, and +// marks the local resourceId touched. +// +// The two functions differ only in: resource type label (singular + plural +// for log lines), state section, remote list fetcher, the per-type counter +// in `applied`, the per-type bookkeeping array, and the apply function. +// This module extracts the shared body — narrowed to `tools` / +// `structuredOutputs` because those are the only two auto-apply paths in +// the engine today. + +import type { RemoteResource } from "./dep-dedup.ts"; +import { + extractResourceName, + findExistingResourceByName, +} from "./dep-dedup.ts"; +import { hashPayload, upsertState } from "./state.ts"; +import type { TouchedSets } from "./state-merge.ts"; +import type { ResourceFile, ResourceType, StateFile } from "./types.ts"; + +// The two resource types that flow through the auto-apply path. Adding a +// new caller (e.g. simulations) is a one-line union widening. +export type ReconcileResourceType = "tools" | "structuredOutputs"; + +// Per-type display labels used in the three log lines this helper emits. +// `singular` populates the "Reusing existing X" / "→ X" lines; `plural` +// populates the ambiguous-match warning ("Multiple dashboard Xs share the +// name"). +const LABELS: Record< + ReconcileResourceType, + { singular: string; plural: string } +> = { + tools: { singular: "tool", plural: "dashboard tools" }, + structuredOutputs: { + singular: "structured output", + plural: "dashboard structured outputs", + }, +}; + +export interface ReconcileStateKeyOptions { + resourceType: ReconcileResourceType; + resource: ResourceFile>; + state: StateFile; + touched: TouchedSets; + // Per-type counter map (incremented on a non-null apply result). The + // shape mirrors `DependencyContext.applied` so callers can pass it + // directly without remapping. + applied: Record; + // Cross-type Set of `${resourceType}:${resourceId}` keys for run-scoped + // idempotency. The apply path adds to this set BEFORE checking the apply + // result, matching the legacy behavior — see "applyFn null semantics" in + // the PR description. + autoApplied: Set; + // Callback for the per-type bookkeeping array + // (`ctx.autoAppliedTools` / `ctx.autoAppliedStructuredOutputs`). Wrapped + // as a callback so this module stays decoupled from `DependencyContext`. + pushToAutoAppliedList: ( + resource: ResourceFile>, + ) => void; + // Lazy fetcher for the live dashboard inventory used in the dedup check. + // Callers cache the result inside `DependencyContext` so it fires at most + // once per push. + getRemoteList: () => Promise; + // The standard apply pipeline for this resource type. Returns the + // post-PATCH UUID, or `null` when the apply was skipped (e.g. dry-run, + // dependency unresolved, drift-check halt) — in which case `autoApplied` + // is still recorded but the state-update / counter / bookkeeping steps + // are SKIPPED. Matches the legacy `ensureToolExists` / + // `ensureStructuredOutputExists` semantics exactly. + applyFn: (resource: ResourceFile, state: StateFile) => Promise; + // Surfaced in the ambiguous-match warning to direct the operator at the + // right cleanup command. Required so a future caller (e.g. a sims + // auto-apply path) can't omit it and emit `npm run cleanup -- ` + // verbatim to a confused operator. Tests pass `"test-env"`. + vapiEnv: string; + // Format errors uniformly with the rest of the push pipeline. Required + // for the same future-caller-safety reason as `vapiEnv` — production + // callers MUST inject `formatApiError` from push.ts so VapiApiError + // breakouts render with status code + endpoint, not a degraded + // `Error.message`-only fallback. + formatError: (resourceId: string, error: unknown) => string; +} + +export async function reconcileStateKeyForResource( + opts: ReconcileStateKeyOptions, +): Promise { + const { + resourceType, + resource, + state, + touched, + applied, + autoApplied, + pushToAutoAppliedList, + getRemoteList, + applyFn, + vapiEnv, + formatError, + } = opts; + + const { singular, plural } = LABELS[resourceType]; + const stateSection = state[resourceType]; + const touchedSection = touched[resourceType]; + const resourceId = resource.resourceId; + const autoAppliedKey = `${resourceType}:${resourceId}`; + + // Before creating, check whether an existing state entry (under a + // different key — e.g., bootstrap-generated `-`) or a live + // dashboard resource already represents this same logical resource. + // Adopt instead of minting a duplicate. + const remoteList = await getRemoteList(); + const match = findExistingResourceByName({ + localResourceId: resourceId, + localPayload: resource.data, + stateSection, + remoteList, + }); + + if (match) { + if (match.ambiguous) { + const displayName = extractResourceName(resource.data) ?? resourceId; + console.warn( + ` ⚠️ Multiple ${plural} share the name "${displayName}" — adopting ${match.uuid} (lex-smallest). Other UUIDs: ${match.duplicateUuids.join(", ")}. Run \`npm run cleanup -- ${vapiEnv}\` to prune duplicates.`, + ); + } + console.log( + ` 🔁 Reusing existing ${singular}: ${resourceId} → ${match.uuid} (matched via ${match.source})`, + ); + + // Re-key state to point at the adopted UUID under the local resourceId. + // No hash yet — `applyFn` below will PATCH with the local payload and + // record the post-PATCH hash, exercising the standard drift-check flow. + upsertState(stateSection, 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(stateSection)) { + if (staleKey !== resourceId && entry.uuid === match.uuid) { + delete stateSection[staleKey]; + touchedSection.add(staleKey); + } + } + + // PATCH the dashboard with the local payload. The apply function's + // `upsertResourceWithStateRecovery` branch picks PATCH because the + // state section 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 applyFn(resource, state); + autoApplied.add(autoAppliedKey); + if (!uuid) return; + upsertState(stateSection, resourceId, { + uuid, + lastPushedHash: hashPayload(resource.data), + }); + applied[resourceType]++; + pushToAutoAppliedList(resource); + touchedSection.add(resourceId); + } catch (error) { + console.error(formatError(resourceId, error)); + throw error; + } + return; + } + + console.log(` 📦 Auto-applying dependency → ${singular}: ${resourceId}`); + try { + const uuid = await applyFn(resource, state); + autoApplied.add(autoAppliedKey); + if (!uuid) return; + upsertState(stateSection, resourceId, { + uuid, + lastPushedHash: hashPayload(resource.data), + }); + applied[resourceType]++; + pushToAutoAppliedList(resource); + touchedSection.add(resourceId); + } catch (error) { + console.error(formatError(resourceId, error)); + throw error; + } +} diff --git a/tests/reconcile-state-key.test.ts b/tests/reconcile-state-key.test.ts new file mode 100644 index 0000000..a4b4b0e --- /dev/null +++ b/tests/reconcile-state-key.test.ts @@ -0,0 +1,444 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// reconcile-state-key.ts → state.ts → config.ts: config.ts asserts argv[2] +// and VAPI_TOKEN at module load. Prime both before dynamic import. +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { reconcileStateKeyForResource } = await import( + "../src/reconcile-state-key.ts" +); + +import type { RemoteResource } from "../src/dep-dedup.ts"; +import type { TouchedSets } from "../src/state-merge.ts"; +import type { + ResourceFile, + ResourceState, + ResourceType, + StateFile, +} from "../src/types.ts"; + +// ───────────────────────────────────────────────────────────────────────────── +// Test fixtures — every scenario is run twice (tools + structuredOutputs) +// to prove the helper is genuinely uniform. Each scenario builds the inputs +// with the helpers below, runs reconcileStateKeyForResource, then asserts on +// state mutations, touched sets, applied counters, and apply-fn invocations. +// ───────────────────────────────────────────────────────────────────────────── + +type RType = "tools" | "structuredOutputs"; + +function emptyState(): StateFile { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +function emptyTouched(): TouchedSets { + return { + tools: new Set(), + structuredOutputs: new Set(), + assistants: new Set(), + squads: new Set(), + personalities: new Set(), + scenarios: new Set(), + simulations: new Set(), + simulationSuites: new Set(), + evals: new Set(), + credentials: new Set(), + }; +} + +function emptyApplied(): Record { + return { + tools: 0, + structuredOutputs: 0, + assistants: 0, + squads: 0, + personalities: 0, + scenarios: 0, + simulations: 0, + simulationSuites: 0, + evals: 0, + }; +} + +// Construct a payload whose canonical name slugifies to the same value for +// both types. Tools use `function.name`, SOs use top-level `name`. +function makePayload(rtype: RType, name: string): Record { + if (rtype === "tools") return { function: { name } }; + return { name }; +} + +function makeResource( + rtype: RType, + resourceId: string, + name: string, +): ResourceFile> { + return { + resourceId, + filePath: `/fake/${rtype}/${resourceId}.yml`, + data: makePayload(rtype, name), + }; +} + +interface Harness { + state: StateFile; + touched: TouchedSets; + applied: Record; + autoApplied: Set; + autoAppliedList: ResourceFile>[]; + applyCalls: { resourceId: string }[]; +} + +function makeHarness(): Harness { + return { + state: emptyState(), + touched: emptyTouched(), + applied: emptyApplied(), + autoApplied: new Set(), + autoAppliedList: [], + applyCalls: [], + }; +} + +interface RunOpts { + rtype: RType; + resource: ResourceFile>; + remoteList?: RemoteResource[]; + applyResult?: string | null; + applyThrows?: Error; + harness: Harness; +} + +async function runReconcile(opts: RunOpts): Promise { + const { rtype, resource, remoteList = [], harness } = opts; + await reconcileStateKeyForResource({ + resourceType: rtype, + resource, + state: harness.state, + touched: harness.touched, + applied: harness.applied, + autoApplied: harness.autoApplied, + pushToAutoAppliedList: (r) => harness.autoAppliedList.push(r), + getRemoteList: async () => remoteList, + applyFn: async (r, _state) => { + harness.applyCalls.push({ resourceId: r.resourceId }); + if (opts.applyThrows) throw opts.applyThrows; + // Default: return a UUID matching whatever state has under the + // resourceId, falling back to a synthetic uuid. Mirrors what the real + // applyTool would return after a successful PATCH/POST. + if (opts.applyResult !== undefined) return opts.applyResult; + const existing = harness.state[rtype][r.resourceId]?.uuid; + return existing ?? `uuid-${r.resourceId}-created`; + }, + vapiEnv: "test-env", + }); +} + +const RTYPES: RType[] = ["tools", "structuredOutputs"]; + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 1 — State hit, SAME uuid, redundant alias. +// State already has `-` pointing at the same UUID we'd be +// adopting from the dashboard. The alias should be dropped and marked +// touched; the canonical key keeps the adopted UUID. applyFn is called. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 1: state hit, same uuid, redundant alias dropped`, async () => { + const h = makeHarness(); + const sharedUuid = "00000000-0000-0000-0000-000000000aaa"; + // Pre-existing alias under a bootstrap-renamed key, same UUID. + h.state[rtype]["my-resource-12345678"] = { uuid: sharedUuid }; + const resource = makeResource(rtype, "my-resource", "my-resource"); + + await runReconcile({ + rtype, + resource, + remoteList: [{ id: sharedUuid, name: "my-resource" }], + harness: h, + }); + + // Canonical key now points at the adopted UUID (with lastPushedHash + // populated after applyFn success). + assert.equal(h.state[rtype]["my-resource"]?.uuid, sharedUuid); + assert.ok(h.state[rtype]["my-resource"]?.lastPushedHash); + // Alias was deleted. + assert.equal(h.state[rtype]["my-resource-12345678"], undefined); + // Both keys marked touched: the deletion AND the new canonical entry. + assert.ok(h.touched[rtype].has("my-resource-12345678")); + assert.ok(h.touched[rtype].has("my-resource")); + // applyFn was called once. + assert.equal(h.applyCalls.length, 1); + // applied counter incremented, bookkeeping populated. + assert.equal(h.applied[rtype], 1); + assert.equal(h.autoAppliedList.length, 1); + assert.ok(h.autoApplied.has(`${rtype}:my-resource`)); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 2 — State hit, DIFFERENT uuid, base-slug match. +// State has `-` pointing at uuid A; local file wants the +// canonical slot. We adopt uuid A, rekey state to the canonical key, and +// delete the suffixed entry. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 2: state hit, different uuid, adopts and rekeys`, async () => { + const h = makeHarness(); + const adoptedUuid = "11111111-1111-1111-1111-111111111aaa"; + h.state[rtype]["end-call-67aea057"] = { uuid: adoptedUuid }; + const resource = makeResource(rtype, "end-call", "end-call"); + + await runReconcile({ + rtype, + resource, + remoteList: [{ id: adoptedUuid, name: "end-call" }], + harness: h, + }); + + assert.equal(h.state[rtype]["end-call"]?.uuid, adoptedUuid); + assert.equal(h.state[rtype]["end-call-67aea057"], undefined); + assert.ok(h.touched[rtype].has("end-call-67aea057")); + assert.ok(h.touched[rtype].has("end-call")); + assert.equal(h.applyCalls.length, 1); + assert.equal(h.applied[rtype], 1); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 3 — Dashboard hit only (state empty, dashboard has same-name). +// Pure dashboard adoption: no prior state entry, but a live dashboard +// resource shares the slugified name. We adopt and PATCH. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 3: dashboard hit only, adopts remote UUID`, async () => { + const h = makeHarness(); + const dashboardUuid = "22222222-2222-2222-2222-222222222aaa"; + const resource = makeResource(rtype, "my-resource", "My Resource"); + + await runReconcile({ + rtype, + resource, + remoteList: [{ id: dashboardUuid, name: "My Resource" }], + harness: h, + }); + + assert.equal(h.state[rtype]["my-resource"]?.uuid, dashboardUuid); + assert.equal(h.applyCalls.length, 1); + assert.equal(h.applied[rtype], 1); + assert.ok(h.autoApplied.has(`${rtype}:my-resource`)); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 4 — Ambiguous dashboard match. +// Two dashboard resources share the slugified name. We pick the +// lex-smallest, surface ambiguous=true via the warning log, and DO NOT +// touch the other UUIDs' state entries. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 4: ambiguous dashboard match, picks lex-smallest`, async () => { + const h = makeHarness(); + const uuidA = "11111111-1111-1111-1111-111111111aaa"; + const uuidB = "22222222-2222-2222-2222-222222222aaa"; + const resource = makeResource(rtype, "dup", "dup"); + + // Capture warning output to confirm the ambiguous path fired. + const warnings: string[] = []; + const origWarn = console.warn; + console.warn = (msg: string) => { + warnings.push(msg); + }; + try { + await runReconcile({ + rtype, + resource, + remoteList: [ + { id: uuidB, name: "dup" }, + { id: uuidA, name: "dup" }, + ], + harness: h, + }); + } finally { + console.warn = origWarn; + } + + // Lex-smallest wins → uuidA. + assert.equal(h.state[rtype]["dup"]?.uuid, uuidA); + assert.ok(warnings.some((w) => w.includes("Multiple"))); + assert.ok(warnings.some((w) => w.includes(uuidB))); + assert.equal(h.applied[rtype], 1); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 5 — No match, pure create path. +// State empty, dashboard empty. We fall through to the create branch: +// applyFn called, state populated with the fresh UUID, applied++, +// autoApplied set populated, bookkeeping array pushed. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 5: no match, pure create path`, async () => { + const h = makeHarness(); + const resource = makeResource(rtype, "brand-new", "brand-new"); + + await runReconcile({ + rtype, + resource, + applyResult: "33333333-3333-3333-3333-333333333aaa", + harness: h, + }); + + assert.equal( + h.state[rtype]["brand-new"]?.uuid, + "33333333-3333-3333-3333-333333333aaa", + ); + assert.ok(h.state[rtype]["brand-new"]?.lastPushedHash); + assert.equal(h.applied[rtype], 1); + assert.equal(h.autoAppliedList.length, 1); + assert.ok(h.autoApplied.has(`${rtype}:brand-new`)); + assert.ok(h.touched[rtype].has("brand-new")); + assert.equal(h.applyCalls.length, 1); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 6 — Idempotency. +// The caller (push.ts ensureToolExists / ensureStructuredOutputExists) is +// responsible for the autoApplied short-circuit; this helper itself runs +// unconditionally. We verify the caller's guard works by simulating two +// invocations from a wrapper that mimics the production short-circuit. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 6: caller-side autoApplied short-circuit prevents 2nd reconcile`, async () => { + const h = makeHarness(); + const resource = makeResource(rtype, "once", "once"); + const autoAppliedKey = `${rtype}:once`; + + async function callOnce(): Promise { + if (h.autoApplied.has(autoAppliedKey)) return; + await runReconcile({ + rtype, + resource, + applyResult: "44444444-4444-4444-4444-444444444aaa", + harness: h, + }); + } + + await callOnce(); + await callOnce(); + + // Second call must short-circuit before reconcile runs. + assert.equal(h.applyCalls.length, 1); + assert.equal(h.applied[rtype], 1); + assert.equal(h.autoAppliedList.length, 1); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 7 — applyFn returns null. +// Matches push.ts:998 / push.ts:1093 semantics exactly: autoApplied IS +// recorded, but state hash, applied counter, bookkeeping array, and +// touched set are NOT updated for the resourceId. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 7: applyFn returns null — autoApplied recorded, counter not incremented`, async () => { + const h = makeHarness(); + const resource = makeResource(rtype, "skipme", "skipme"); + + await runReconcile({ + rtype, + resource, + applyResult: null, + harness: h, + }); + + // autoApplied IS recorded (even on null result) — this is the + // "we tried; don't try again this run" signal. + assert.ok(h.autoApplied.has(`${rtype}:skipme`)); + // State NOT updated with hash, applied NOT incremented, bookkeeping + // NOT populated, touched NOT marked for the resourceId. + assert.equal(h.state[rtype]["skipme"], undefined); + assert.equal(h.applied[rtype], 0); + assert.equal(h.autoAppliedList.length, 0); + assert.equal(h.touched[rtype].has("skipme"), false); + assert.equal(h.applyCalls.length, 1); + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Scenario 8 — Orphan-deletion guard scope. +// Only state keys pointing at the ADOPTED UUID are dropped. Keys pointing +// at OTHER UUIDs (including those flagged as match.duplicateUuids — real +// on-dashboard duplicates left for `npm run cleanup`) MUST be left alone. +// ───────────────────────────────────────────────────────────────────────────── + +for (const rtype of RTYPES) { + test(`[${rtype}] scenario 8: orphan-deletion only drops keys pointing at the adopted uuid`, async () => { + const h = makeHarness(); + const adoptedUuid = "11111111-1111-1111-1111-111111111aaa"; + const duplicateUuid = "22222222-2222-2222-2222-222222222aaa"; + + // Three state entries: + // - alias A pointing at the adopted uuid → MUST be deleted + // - alias B pointing at the duplicate uuid → MUST be preserved + // - alias C pointing at an unrelated uuid → MUST be preserved + h.state[rtype]["dup-aaaaaaaa"] = { uuid: adoptedUuid }; + h.state[rtype]["dup-bbbbbbbb"] = { uuid: duplicateUuid }; + h.state[rtype]["unrelated-cccccccc"] = { + uuid: "99999999-9999-9999-9999-999999999aaa", + }; + + const resource = makeResource(rtype, "dup", "dup"); + + // Capture warning so the ambiguous test path doesn't pollute output. + const origWarn = console.warn; + console.warn = (): void => {}; + try { + await runReconcile({ + rtype, + resource, + remoteList: [ + { id: adoptedUuid, name: "dup" }, + { id: duplicateUuid, name: "dup" }, + ], + harness: h, + }); + } finally { + console.warn = origWarn; + } + + // Canonical key adopts the lex-smallest UUID. + assert.equal(h.state[rtype]["dup"]?.uuid, adoptedUuid); + // Alias pointing at the adopted UUID: deleted. + assert.equal(h.state[rtype]["dup-aaaaaaaa"], undefined); + assert.ok(h.touched[rtype].has("dup-aaaaaaaa")); + // Alias pointing at the duplicate UUID: preserved. + const dupEntry: ResourceState | undefined = h.state[rtype]["dup-bbbbbbbb"]; + assert.equal(dupEntry?.uuid, duplicateUuid); + assert.equal(h.touched[rtype].has("dup-bbbbbbbb"), false); + // Unrelated entry: preserved. + assert.equal( + h.state[rtype]["unrelated-cccccccc"]?.uuid, + "99999999-9999-9999-9999-999999999aaa", + ); + assert.equal(h.touched[rtype].has("unrelated-cccccccc"), false); + }); +}