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

[dhruva-reddy]

Summary

Tier 2 of 2 in the engine-consolidation series. Stacks on #33 (slug-utils).

`ensureToolExists` (`push.ts:936-1029`) and `ensureStructuredOutputExists` (`push.ts:1031-1124`) were structurally identical 94-line functions. Both implemented the same dedup-then-apply flow: look up existing dashboard/state match by canonical name, adopt with the orphan-deletion guard, mark `touched` for `mergeScoped`, call the apply function.

They differed only in:

• Resource type label (`tools` / `structuredOutputs`)
• Apply function (`applyTool` / `applyStructuredOutput`)
• State section (`state.tools` / `state.structuredOutputs`)
• Remote-list cache (`existingRemoteTools` / `existingRemoteStructuredOutputs`)
• Per-type bookkeeping array (`autoAppliedTools` / `autoAppliedStructuredOutputs`)
• Ambiguous-warning singular/plural label

All five differences parameterize cleanly. `reconcileStateKeyForResource` is the generic helper; both ensure-functions become ~14-line wrappers.

Behavior preservation is the contract

This refactor is behavior-equivalent. Verified path-by-path against pre-refactor (code-reviewer ran the full diff):

Path	Verdict
Ambiguous warning string	Byte-identical (`Multiple ${plural} share the name "${name}" — adopting ${uuid}...`)
Reuse log line	Byte-identical
Auto-apply log line	Byte-identical
`autoApplied` key format (`${resourceType}:${resourceId}`)	Identical
Orphan-deletion scope (adopted UUID only; `duplicateUuids` left for `npm run cleanup`)	Preserved
`autoApplied.add` BEFORE `if (!uuid) return`	Preserved — critical for dry-run / drift-halt semantics
`applied[type]++` / `pushToAutoAppliedList` / `touched.add` AFTER null check	Preserved
try/catch boundary	Identical
Match-path `upsertState` BEFORE orphan-deletion	Preserved

`tests/push-dry-run.test.ts` passes unchanged — the integration test pins the behavior contract end-to-end.

API discipline

Both `vapiEnv` and `formatError` are required parameters (not optional with placeholder defaults). Reason: a future caller (e.g. a sims auto-apply path) must not accidentally:

• Emit `npm run cleanup -- ` verbatim to a confused operator, or
• Render a degraded error message that loses the VapiApiError 3-line breakout

Cost: production call sites pass `VAPI_ENV` and `formatApiError` explicitly. Tests pass `"test-env"` and an inline formatter.

Files changed

File	Change	LOC
`src/reconcile-state-key.ts`	NEW (generic helper)	+205
`tests/reconcile-state-key.test.ts`	NEW (16 cases — 8 scenarios × 2 resource types)	+444
`src/push.ts`	Two 94-line ensure-fns → two ~14-line wrappers	-129

Net: +674 / -154 → +520. Production code shrinks: source-only net is +76 / -154 = -78 lines while collapsing two functions into one.

What the 16 tests pin down

Each runs for BOTH `resourceType: "tools"` and `resourceType: "structuredOutputs"`:

1. State hit, same UUID, redundant alias → drops alias, marks deletion touched, calls applyFn
2. State hit, different UUID under base-slug-match key → adopts UUID, rekeys, deletes suffixed entry
3. Dashboard hit only → reconciles to dashboard UUID via name match
4. Ambiguous dashboard match → logs warning, picks lex-smallest, surfaces `duplicateUuids`
5. No match → pure create path (state populated, `applied` incremented, `autoApplied` set, callback fired)
6. Idempotency → second call short-circuits via `autoApplied`
7. `applyFn` returns null → state NOT updated, `applied` does NOT increment, `autoApplied` STILL records the key (preserves dry-run semantics)
8. Orphan-deletion guard scope → only adopted-UUID keys dropped; `duplicateUuids` keys preserved

Test plan

• `npm run build` (tsc --noEmit) — clean
• `npm test` — 244/244 pass (228 prior + 16 new)
• `npx @biomejs/biome check --write` — clean
• `tests/push-dry-run.test.ts` passes UNCHANGED (the contract pin)

Code review

In-branch review by code-reviewer subagent. No blocking findings. Behavior contract preserved path-by-path (verified by reading pre-refactor against the new helper).

Two LOW findings addressed in-branch (`vapiEnv` and `formatError` upgraded from optional-with-default to required, per L1+L2).

One non-blocking residual: `ensureAssistantDepsExist` was NOT consolidated into this helper. Reason is sound — assistant adoption has additional steps (squad-ref propagation, deps-first-pass coordination) that the generic helper doesn't model. Tier 3 candidate, not blocking this PR.

Stack

• PR 1: refactor(engine): extract shared slug + folder helpers into slug-utils #33 — `refactor(engine): extract shared slug + folder helpers into slug-utils` (base of this stack)
• PR 2: this — `refactor(push): extract reconcileStateKeyForResource` (folds the two ensure-functions)

Reviewable alone given the byte-equivalent behavior contract. Merging this without merging #33 is technically possible (no direct slug-utils import) but the recommended order is base-first.

Related

• VapiAI/gitops fix(push,pull): recanonicalize stale UUID-suffixed state keys — root-cause duplicate generation #32 (recanonicalize stale UUID-suffixed state keys — earlier in the same stack)
• VapiAI/gitops fix: dedup dependency auto-apply to prevent duplicate tool mints (Gap #10) #23 (tool name-based dedup — symptomatic fix fix(gitops): restore 7 P0 regressions surfaced in feat/optimization-gitops-flow audit; add regression tests #10 in improvements.md, now folded into the generic helper)
• improvements.md fix(gitops): restore 7 P0 regressions surfaced in feat/optimization-gitops-flow audit; add regression tests #10 (RESOLVED 2026-05-06): the symptomatic fix shape this generic helper consolidates

[dhruva-reddy]

Superseded by #36 (merged 2026-05-15). GitHub auto-closed this PR when the stack-parent base branch was deleted on #35's merge. Same diff, retargeted at main.