From 70162a21ea9c9c361583a4deb398d6f905a8abf1 Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Thu, 14 May 2026 19:20:13 -0700 Subject: [PATCH] fix(push,pull): recanonicalize stale UUID-suffixed state keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generic, resource-type-agnostic pass that collapses engine-generated `-` state keys back to canonical `` when the underlying name-collision has resolved. Root-cause fix for the recurring duplicate-generation pattern that 10+ prior symptom-fixes had addressed. Pull rewrites state keys to UUID-suffixed form when name-collision adoption is refused (src/pull.ts:findExistingResourceId), but never undoes that rekey when the conflict resolves. Next push sees the canonical-slug local file as orphan → bypasses the orphan-YAML gate with --allow-new-files → silently creates a third dashboard duplicate. Safety preconditions (every rekey must satisfy ALL 5): 1. Key matches `^(.+)-([0-9a-f]{8})$` (engine-generated shape) 2. Captured suffix matches the entry's UUID prefix 3. Canonical slug is unclaimed in state 4. Local file exists at canonical slug (any VALID_EXTENSIONS shape) 5. NO local file at UUID-suffixed slug (avoid silent data loss) Same-UUID self-aliases auto-resolve. Different-UUID twins are refused with a clear conflict reason. `touched` is plumbed through so scoped pushes flush the rename via `mergeScoped`. Runs at end of pull (gated on `!bootstrap && !resourceIds`, honors typeFilter) and start of push after maybeBootstrapState, before the orphan-YAML gate. VALID_EXTENSIONS is imported from src/resources.ts so precondition 5 stays in lockstep with the loader (.yml/.yaml/.ts/.md). Tests: 208/208 pass (17 new cases covering happy path, every-type uniformity, all 5 preconditions, multi-segment slugs, .ts extension support, H1 mergeScoped regression, H2 same-UUID auto-resolve, credentials safety, typeFilter wiring, formatter). --- src/pull.ts | 24 ++ src/push.ts | 33 +++ src/recanonicalize.ts | 273 ++++++++++++++++++ src/resources.ts | 13 +- tests/recanonicalize.test.ts | 522 +++++++++++++++++++++++++++++++++++ 5 files changed, 864 insertions(+), 1 deletion(-) create mode 100644 src/recanonicalize.ts create mode 100644 tests/recanonicalize.test.ts diff --git a/src/pull.ts b/src/pull.ts index 7dee8c3..d4cb024 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -16,6 +16,10 @@ import { VAPI_TOKEN, } from "./config.ts"; import { credentialReverseMap, replaceCredentialRefs } from "./credentials.ts"; +import { + formatRecanonicalizeReport, + recanonicalizeStateKeys, +} from "./recanonicalize.ts"; import { hashPayload, loadState, saveState, upsertState } from "./state.ts"; import type { ResourceState, ResourceType, StateFile } from "./types.ts"; @@ -1046,6 +1050,26 @@ export async function runPull(options: PullOptions = {}): Promise { resourceIds, }); + // Collapse UUID-suffixed state keys back to canonical when the underlying + // name-collision has resolved (the conflicting twin was deleted, etc.). + // Skipped during bootstrap because bootstrap is supposed to populate state + // from scratch — there is no prior rekey to undo. Also skipped on targeted + // ID pulls so we don't sweep types we haven't fully refreshed. When the + // pull is scoped by typeFilter, the pass is restricted to those types so + // we don't touch sections this pull didn't refresh — preserves the stated + // safety boundary even though the preconditions themselves are safe. + // Pull's saveState writes wholesale, so `touched` isn't needed here. + if (!bootstrap && !resourceIds?.length) { + const report = recanonicalizeStateKeys({ + state, + types: typeFilter?.length ? (typeFilter as ResourceType[]) : undefined, + }); + const summary = formatRecanonicalizeReport(report); + if (summary) { + console.log(`\n${summary}`); + } + } + await saveState(state); // Summary diff --git a/src/push.ts b/src/push.ts index c068429..b40183f 100644 --- a/src/push.ts +++ b/src/push.ts @@ -22,6 +22,10 @@ import { } from "./dep-dedup.ts"; import { checkDriftForUpdate } from "./drift.ts"; import { detectOrphanYamls, formatGateMessage } from "./new-file-gate.ts"; +import { + formatRecanonicalizeReport, + recanonicalizeStateKeys, +} from "./recanonicalize.ts"; import { writeSnapshot } from "./snapshot.ts"; import { mergeScoped } from "./state-merge.ts"; import { @@ -1367,6 +1371,35 @@ async function main(): Promise { state = await maybeBootstrapState(loadedResources, state); + // Recanonicalize stale UUID-suffixed state keys back to canonical slugs + // before the orphan-YAML gate runs. This is the safe collapse of the + // pull-side rekey-on-name-collision behavior (src/pull.ts) once the + // underlying collision has resolved (e.g. the conflicting twin was + // deleted on the dashboard). Without this pass, the orphan-YAML gate + // sees the canonical-slug local file as "new" — the recurring + // duplicate-creation root cause documented in improvements.md. + // + // Gating: `BOOTSTRAP_SYNC` (the user's explicit `--bootstrap` flag) + // skips this pass because state is being rebuilt from scratch. Note + // that the *internal* bootstrap-recovery pull triggered by + // `maybeBootstrapState` above does NOT set `BOOTSTRAP_SYNC` — so + // recanonicalize still runs after that recovery, which is intentional: + // bootstrap freshly generates UUID-suffixed keys for unresolved + // collisions, and recanonicalize collapses any that no longer have a + // live conflict. + // + // `touched` is plumbed through so scoped pushes flush the rename via + // `mergeScoped` at save time. Without this, the on-disk stale key + // would silently re-overwrite the in-memory canonical rename — H1 in + // the code review. + if (!BOOTSTRAP_SYNC) { + const recanonReport = recanonicalizeStateKeys({ state, touched }); + const recanonSummary = formatRecanonicalizeReport(recanonReport); + if (recanonSummary) { + console.log(`\n${recanonSummary}`); + } + } + // Orphan-YAML pre-flight gate. Runs ONCE for ALL resource types after // bootstrap (so state-recovery has a chance to rekey first) and BEFORE // any apply phase. Halts push when local files exist with no state entry diff --git a/src/recanonicalize.ts b/src/recanonicalize.ts new file mode 100644 index 0000000..3fc6c13 --- /dev/null +++ b/src/recanonicalize.ts @@ -0,0 +1,273 @@ +// ───────────────────────────────────────────────────────────────────────────── +// State key recanonicalization — generic across all resource types. +// +// The pull engine generates UUID-suffixed state keys (`-`) when +// name-collision adoption is refused (src/pull.ts:findExistingResourceId / +// generateResourceId). That rekey is fire-and-forget: once the underlying +// collision resolves (e.g. the conflicting twin is deleted on the dashboard), +// nothing ever collapses the UUID-suffixed entry back to its canonical slug. +// +// Without this collapse, subsequent pushes treat the canonical-slug local +// file as a brand-new resource (orphan-YAML), and the operator either hits +// the orphan-YAML gate (#30) and bypasses with `--allow-new-files`, or — in +// older engine versions — silently creates a third dashboard duplicate. +// +// This module runs a safe collapse pass at end-of-pull and start-of-push. +// The pass is resource-type-agnostic: it walks every section of StateFile +// uniformly, so adding a new ResourceType doesn't need a code change here. +// +// SAFETY MODEL — every rekey must satisfy all five preconditions: +// 1. Key matches `^(.+)-([0-9a-f]{8})$` — the engine's generated shape. +// 2. The captured `` matches the entry's UUID prefix. This rules +// out resources whose user-given names legitimately end in +// `-abcd1234`. We only touch keys we can prove the engine wrote. +// 3. Canonical slug `` is unclaimed in the SAME state section. +// Prevents collision when a same-name twin is intentionally tracked. +// 4. A local file exists at `` with any extension recognized by +// the resource loader (`VALID_EXTENSIONS` in src/resources.ts — +// .yml/.yaml/.ts/.md). Prevents creating phantom state mappings to +// slugs that have no source file. The extension set is imported, not +// hardcoded here, so the precondition stays in lockstep with the +// loader; any future loader extension is automatically respected. +// 5. NO local file exists at `-` under any +// `VALID_EXTENSIONS` shape. The UUID-suffixed file represents a +// different content snapshot; rekeying state without consolidating +// files would silently reassign which file PATCHes which dashboard +// UUID — the data-loss shape we are explicitly refusing to introduce. +// +// When (5) fails we surface a CONFLICT (both files exist, ambiguous which +// owns the dashboard UUID) so the operator can resolve it manually. We +// never auto-pick a winner — silent data loss is worse than a duplicate. +// ───────────────────────────────────────────────────────────────────────────── + +import { existsSync } from "fs"; +import { join } from "path"; +import { RESOURCES_DIR } from "./config.ts"; +import { FOLDER_MAP, VALID_EXTENSIONS } from "./resources.ts"; +import type { TouchedSets } from "./state-merge.ts"; +import type { ResourceType, StateFile } from "./types.ts"; +import { VALID_RESOURCE_TYPES } from "./types.ts"; + +const UUID_SUFFIX_RE = /^(.+)-([0-9a-f]{8})$/i; + +export interface RecanonicalizeRekey { + type: ResourceType; + fromKey: string; + toKey: string; + uuid: string; +} + +export interface RecanonicalizeConflict { + type: ResourceType; + uuidSuffixedKey: string; + canonicalKey: string; + reason: + | "canonical-slug-claimed-by-different-uuid" + | "both-local-files-exist" + | "canonical-local-file-missing"; + uuid: string; +} + +export interface RecanonicalizeReport { + rekeys: RecanonicalizeRekey[]; + conflicts: RecanonicalizeConflict[]; +} + +export interface RecanonicalizeOptions { + state: StateFile; + // DI seam — defaults to filesystem check rooted at `RESOURCES_DIR`. Tests + // pass a stub so they don't need a fixture tree. + fileExists?: (relativePath: string) => boolean; + // Restrict the pass to specific types (default: all). + types?: ResourceType[]; + // Optional `touched` set for scoped pushes. When provided, both the + // deleted UUID-suffixed key AND the new canonical key are marked so + // `mergeScoped` flushes the rename instead of silently re-persisting the + // on-disk stale key. Pull doesn't pass this — it saves wholesale. + touched?: TouchedSets; +} + +function defaultFileExists(relativePath: string): boolean { + return existsSync(join(RESOURCES_DIR, relativePath)); +} + +function localFileExistsForId( + type: ResourceType, + resourceId: string, + fileExists: (relativePath: string) => boolean, +): boolean { + const folder = FOLDER_MAP[type]; + for (const ext of VALID_EXTENSIONS) { + if (fileExists(`${folder}/${resourceId}${ext}`)) return true; + } + return false; +} + +// Mutates `state` in place. Returns the list of changes for logging and the +// list of unresolved conflicts the operator should see. Callers decide +// whether conflicts halt the run (push) or are advisory (pull). +export function recanonicalizeStateKeys( + opts: RecanonicalizeOptions, +): RecanonicalizeReport { + const { + state, + fileExists = defaultFileExists, + types = [...VALID_RESOURCE_TYPES], + touched, + } = opts; + + const rekeys: RecanonicalizeRekey[] = []; + const conflicts: RecanonicalizeConflict[] = []; + + const markTouched = (type: ResourceType, key: string): void => { + if (touched) touched[type].add(key); + }; + + for (const type of types) { + const section = state[type]; + if (!section) continue; + + // Snapshot keys upfront so we can mutate the section during iteration. + for (const stateKey of Object.keys(section)) { + const entry = section[stateKey]; + if (!entry) continue; + + const match = stateKey.match(UUID_SUFFIX_RE); + if (!match) continue; + + const [, canonicalSlug, capturedUuid8] = match; + if (!canonicalSlug || !capturedUuid8) continue; + + // Precondition 2 — only recanonicalize engine-generated suffixes. If + // the captured 8 hex chars don't match the entry's UUID prefix, this + // is a user-named resource that coincidentally looks suffixed. + // + // Mirrors generateResourceId() in src/pull.ts:265-273 — UUID dashes + // start at index 8, so `slice(0, 8)` on the raw UUID is equivalent + // to stripping dashes first; the dash-strip is defensive. + const entryUuid8 = entry.uuid.replace(/-/g, "").slice(0, 8).toLowerCase(); + if (capturedUuid8.toLowerCase() !== entryUuid8) continue; + + // Precondition 3 — canonical slot must be unclaimed in state. + // + // Special case: if the canonical slot is claimed by the SAME UUID, + // both keys are aliases for one dashboard resource. This shape is + // produced by older engine versions that wrote duplicate-aliased + // state, or by a `mergeScoped` round-trip before recanonicalize was + // touched-aware. Safe action: drop the redundant UUID-suffixed key + // (canonical wins; its metadata is presumed more authoritative). + // This is auto-resolved as a rekey for reporting purposes. + const claimedEntry = section[canonicalSlug]; + if (claimedEntry !== undefined) { + if (claimedEntry.uuid === entry.uuid) { + delete section[stateKey]; + markTouched(type, stateKey); + markTouched(type, canonicalSlug); + rekeys.push({ + type, + fromKey: stateKey, + toKey: canonicalSlug, + uuid: entry.uuid, + }); + continue; + } + // Genuinely a different UUID — same-name twin tracked legitimately. + conflicts.push({ + type, + uuidSuffixedKey: stateKey, + canonicalKey: canonicalSlug, + reason: "canonical-slug-claimed-by-different-uuid", + uuid: entry.uuid, + }); + continue; + } + + // Precondition 4 — canonical local file must exist. Otherwise we'd + // be inventing a state mapping that has no source on disk. + const canonicalFileExists = localFileExistsForId( + type, + canonicalSlug, + fileExists, + ); + if (!canonicalFileExists) { + conflicts.push({ + type, + uuidSuffixedKey: stateKey, + canonicalKey: canonicalSlug, + reason: "canonical-local-file-missing", + uuid: entry.uuid, + }); + continue; + } + + // Precondition 5 — UUID-suffixed local file must NOT exist. If both + // files exist they represent different content snapshots; silently + // rekeying would change which file PATCHes which dashboard UUID. + const uuidSuffixedFileExists = localFileExistsForId( + type, + stateKey, + fileExists, + ); + if (uuidSuffixedFileExists) { + conflicts.push({ + type, + uuidSuffixedKey: stateKey, + canonicalKey: canonicalSlug, + reason: "both-local-files-exist", + uuid: entry.uuid, + }); + continue; + } + + // All preconditions met — collapse. Mark BOTH the deleted UUID-suffixed + // key AND the new canonical key as touched, so scoped pushes flush the + // rename via `mergeScoped` instead of silently re-persisting the + // on-disk stale key. + section[canonicalSlug] = entry; + delete section[stateKey]; + markTouched(type, stateKey); + markTouched(type, canonicalSlug); + rekeys.push({ + type, + fromKey: stateKey, + toKey: canonicalSlug, + uuid: entry.uuid, + }); + } + } + + return { rekeys, conflicts }; +} + +// Human-readable summary for stdout. Empty report renders nothing — callers +// can unconditionally log. +export function formatRecanonicalizeReport( + report: RecanonicalizeReport, +): string { + if (report.rekeys.length === 0 && report.conflicts.length === 0) return ""; + + const lines: string[] = []; + if (report.rekeys.length > 0) { + lines.push( + `🔧 Recanonicalized ${report.rekeys.length} state key(s) — collapsed UUID-suffixed slug back to canonical:`, + ); + for (const r of report.rekeys) { + lines.push(` ${r.type}/${r.fromKey} → ${r.type}/${r.toKey}`); + } + } + if (report.conflicts.length > 0) { + lines.push( + `⚠️ ${report.conflicts.length} UUID-suffixed state key(s) NOT recanonicalized (manual resolution required):`, + ); + for (const c of report.conflicts) { + const hint = + c.reason === "canonical-slug-claimed-by-different-uuid" + ? `canonical slug ${c.canonicalKey} already claimed by a different dashboard UUID (legitimate same-name twin)` + : c.reason === "both-local-files-exist" + ? `both ${c.canonicalKey}.{yml,yaml,ts,md} and ${c.uuidSuffixedKey}.{yml,yaml,ts,md} exist — pick one and delete the other before next push` + : `no local file at ${c.canonicalKey}.{yml,yaml,ts,md} — state entry is stale; either restore the file or remove the state entry`; + lines.push(` ${c.type}/${c.uuidSuffixedKey}: ${hint}`); + } + } + return lines.join("\n"); +} diff --git a/src/resources.ts b/src/resources.ts index 2421a38..8b99edf 100644 --- a/src/resources.ts +++ b/src/resources.ts @@ -42,7 +42,18 @@ const FOLDER_TO_TYPE: Record = Object.entries( // Resource Loading // ───────────────────────────────────────────────────────────────────────────── -const VALID_EXTENSIONS = [".yml", ".yaml", ".ts", ".md"]; +// Single source of truth for resource file extensions. Imported by +// `recanonicalize.ts` so the precondition-5 "both files exist" check +// stays in lockstep with the loader — without this, a `.ts`-authored +// resource paired with a UUID-suffixed `.ts` twin would be invisible to +// the safety check and silently allow the data-loss shape the +// recanonicalize header explicitly refuses. +export const VALID_EXTENSIONS: readonly string[] = [ + ".yml", + ".yaml", + ".ts", + ".md", +]; /** * Parse a markdown file with YAML frontmatter diff --git a/tests/recanonicalize.test.ts b/tests/recanonicalize.test.ts new file mode 100644 index 0000000..d88555b --- /dev/null +++ b/tests/recanonicalize.test.ts @@ -0,0 +1,522 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// recanonicalize.ts → config.ts: config.ts asserts argv[2] / VAPI_TOKEN at +// module load. Prime both before importing the module under test. Mirrors +// the pattern in tests/new-file-gate.test.ts. +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { recanonicalizeStateKeys, formatRecanonicalizeReport } = await import( + "../src/recanonicalize.ts" +); + +import type { ResourceState, ResourceType, StateFile } from "../src/types.ts"; + +// ───────────────────────────────────────────────────────────────────────────── +// Helpers — DI-style, filesystem-free. The module exposes a `fileExists` +// seam; tests pass a stub keyed on the relative paths the production code +// would have probed. +// ───────────────────────────────────────────────────────────────────────────── + +function makeStateEntry(uuid: string): ResourceState { + return { uuid }; +} + +function makeStateFile(overrides: Partial = {}): StateFile { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + ...overrides, + }; +} + +function makeFileExists(paths: Set) { + return (relativePath: string): boolean => paths.has(relativePath); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Happy path — the recurring duplicate scenario the fix targets. +// ───────────────────────────────────────────────────────────────────────────── + +test("recanonicalize: collapses UUID-suffixed key to canonical when local file present and canonical slot empty (the duplicate-generation root cause)", () => { + // State has `foo-vmd-004c5108` (rekey'd by a prior pull during a name + // collision). The conflicting twin has since been deleted on the + // dashboard. Local has only `squads/foo-vmd.md`. Canonical slot in state + // is empty. We should collapse. + const state = makeStateFile({ + squads: { + "foo-vmd-004c5108": makeStateEntry( + "004c5108-aaaa-bbbb-cccc-dddddddddddd", + ), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set(["squads/foo-vmd.yml"])), + }); + assert.equal(report.rekeys.length, 1); + assert.equal(report.rekeys[0]!.fromKey, "foo-vmd-004c5108"); + assert.equal(report.rekeys[0]!.toKey, "foo-vmd"); + assert.equal(report.conflicts.length, 0); + assert.deepEqual(Object.keys(state.squads), ["foo-vmd"]); + assert.equal( + state.squads["foo-vmd"]!.uuid, + "004c5108-aaaa-bbbb-cccc-dddddddddddd", + ); +}); + +test("recanonicalize: applies uniformly across every resource type", () => { + // One stale UUID-suffixed entry per type. All should collapse — the pass + // is type-agnostic. Folder paths come from FOLDER_MAP so the fileExists + // stub keys must match those folders. + const state = makeStateFile({ + tools: { + "t-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + structuredOutputs: { + "s-bbbbbbbb": makeStateEntry("bbbbbbbb-0000-0000-0000-000000000000"), + }, + assistants: { + "a-cccccccc": makeStateEntry("cccccccc-0000-0000-0000-000000000000"), + }, + squads: { + "q-dddddddd": makeStateEntry("dddddddd-0000-0000-0000-000000000000"), + }, + personalities: { + "p-eeeeeeee": makeStateEntry("eeeeeeee-0000-0000-0000-000000000000"), + }, + scenarios: { + "sc-ffffffff": makeStateEntry("ffffffff-0000-0000-0000-000000000000"), + }, + simulations: { + "sim-12345678": makeStateEntry("12345678-0000-0000-0000-000000000000"), + }, + simulationSuites: { + "suite-87654321": makeStateEntry("87654321-0000-0000-0000-000000000000"), + }, + evals: { + "e-abcdef01": makeStateEntry("abcdef01-0000-0000-0000-000000000000"), + }, + }); + const fileExists = makeFileExists( + new Set([ + "tools/t.yml", + "structuredOutputs/s.yml", + "assistants/a.md", + "squads/q.yml", + "simulations/personalities/p.yml", + "simulations/scenarios/sc.yml", + "simulations/tests/sim.yml", + "simulations/suites/suite.yml", + "evals/e.yml", + ]), + ); + const report = recanonicalizeStateKeys({ state, fileExists }); + assert.equal(report.rekeys.length, 9); + assert.equal(report.conflicts.length, 0); + // Each section now keyed canonically. + assert.deepEqual(Object.keys(state.tools), ["t"]); + assert.deepEqual(Object.keys(state.structuredOutputs), ["s"]); + assert.deepEqual(Object.keys(state.assistants), ["a"]); + assert.deepEqual(Object.keys(state.squads), ["q"]); + assert.deepEqual(Object.keys(state.personalities), ["p"]); + assert.deepEqual(Object.keys(state.scenarios), ["sc"]); + assert.deepEqual(Object.keys(state.simulations), ["sim"]); + assert.deepEqual(Object.keys(state.simulationSuites), ["suite"]); + assert.deepEqual(Object.keys(state.evals), ["e"]); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Safety preconditions — each one MUST block the rekey. +// ───────────────────────────────────────────────────────────────────────────── + +test("recanonicalize: refuses when UUID suffix doesn't match entry's UUID prefix (user-named resource that coincidentally ends in -<8hex>)", () => { + // The key looks suffixed but the captured 8 hex chars (`deadbeef`) are + // NOT the prefix of the entry's UUID (`004c5108...`). This is a + // user-given name like "my-tool-deadbeef" — DO NOT touch it. + const state = makeStateFile({ + tools: { + "my-tool-deadbeef": makeStateEntry( + "004c5108-aaaa-bbbb-cccc-dddddddddddd", + ), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set(["tools/my-tool.yml"])), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 0); + assert.deepEqual(Object.keys(state.tools), ["my-tool-deadbeef"]); +}); + +test("recanonicalize: refuses when canonical slug is claimed by a DIFFERENT UUID (legitimate same-name twin)", () => { + // Both `foo` (uuid_A, live) and `foo-bbbbbbbb` (uuid_B, live twin) exist + // in state — the dashboard genuinely has two resources sharing a name. + // Collapsing `foo-bbbbbbbb` onto `foo` would clobber uuid_A. + const state = makeStateFile({ + squads: { + foo: makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + "foo-bbbbbbbb": makeStateEntry("bbbbbbbb-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set(["squads/foo.yml"])), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 1); + assert.equal( + report.conflicts[0]!.reason, + "canonical-slug-claimed-by-different-uuid", + ); + // State unchanged. + assert.deepEqual(Object.keys(state.squads).sort(), ["foo", "foo-bbbbbbbb"]); +}); + +test("recanonicalize: AUTO-RESOLVES when canonical slug claims the SAME UUID (duplicate alias) — the H1 self-inflicted shape", () => { + // After a scoped push that pre-dated the touched-aware fix, state on + // disk can end up with both `foo` and `foo-aaaaaaaa` pointing at the + // SAME uuid_A. This is not a twin — it's one resource aliased twice. + // Safe action: drop the UUID-suffixed key (canonical wins). Reported + // as a rekey, not a conflict. + const state = makeStateFile({ + squads: { + foo: { + uuid: "aaaaaaaa-0000-0000-0000-000000000000", + lastPulledHash: "canonical-hash", + }, + "foo-aaaaaaaa": { + uuid: "aaaaaaaa-0000-0000-0000-000000000000", + lastPulledHash: "stale-hash", + }, + }, + }); + const report = recanonicalizeStateKeys({ + state, + // No fileExists probing needed for the auto-resolve branch (skipped + // before files are checked). Pass an empty set to prove that. + fileExists: makeFileExists(new Set()), + }); + assert.equal(report.rekeys.length, 1); + assert.equal(report.rekeys[0]!.fromKey, "foo-aaaaaaaa"); + assert.equal(report.rekeys[0]!.toKey, "foo"); + assert.equal(report.conflicts.length, 0); + // Canonical entry survives unchanged (its metadata is presumed + // authoritative — we discard the stale alias, not merge metadata). + assert.deepEqual(Object.keys(state.squads), ["foo"]); + assert.equal(state.squads["foo"]!.lastPulledHash, "canonical-hash"); +}); + +test("recanonicalize: refuses when canonical local file is missing (would create phantom state mapping)", () => { + const state = makeStateFile({ + squads: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + // No local files at all. + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set()), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 1); + assert.equal(report.conflicts[0]!.reason, "canonical-local-file-missing"); + assert.deepEqual(Object.keys(state.squads), ["foo-aaaaaaaa"]); +}); + +test("recanonicalize: respects all loader-recognized extensions (.yml/.yaml/.ts/.md) for precondition 4", () => { + // VALID_EXTENSIONS in src/resources.ts includes `.ts` (TypeScript + // resources via dynamic import). Without importing the canonical + // list, precondition 4 would mis-report a `.ts`-authored canonical + // file as missing. Regression guard for the should-fix surfaced in + // post-merge review. + for (const ext of [".yml", ".yaml", ".ts", ".md"]) { + const state = makeStateFile({ + tools: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set([`tools/foo${ext}`])), + }); + assert.equal( + report.rekeys.length, + 1, + `extension ${ext} should be recognized as a canonical file`, + ); + assert.equal(report.conflicts.length, 0); + } +}); + +test("recanonicalize: refuses when BOTH local files exist as `.ts` (precondition 5 covers every loader extension)", () => { + // Direct regression guard for the data-loss shape: a `.ts`-authored + // resource paired with a UUID-suffixed `.ts` twin must trigger the + // "both files exist" conflict, not silently rekey. + const state = makeStateFile({ + tools: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists( + new Set(["tools/foo.ts", "tools/foo-aaaaaaaa.ts"]), + ), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 1); + assert.equal(report.conflicts[0]!.reason, "both-local-files-exist"); +}); + +test("recanonicalize: refuses when BOTH local files exist (would silently swap which file PATCHes which dashboard UUID)", () => { + // The user has both `foo.yml` (original content) and `foo-aaaaaaaa.yml` + // (the new twin's content) on disk. Rekeying state would make `foo.yml` + // PATCH the wrong dashboard UUID. Refuse — operator must consolidate. + const state = makeStateFile({ + squads: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists( + new Set(["squads/foo.yml", "squads/foo-aaaaaaaa.yml"]), + ), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 1); + assert.equal(report.conflicts[0]!.reason, "both-local-files-exist"); + assert.deepEqual(Object.keys(state.squads), ["foo-aaaaaaaa"]); +}); + +test("recanonicalize: ignores keys without UUID-suffix shape", () => { + // Plain canonical keys are left alone. + const state = makeStateFile({ + squads: { + "ordinary-squad": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set(["squads/ordinary-squad.yml"])), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 0); +}); + +test("recanonicalize: handles UUID prefix match case-insensitively", () => { + // UUIDs in state are typically lowercase, but be defensive. + const state = makeStateFile({ + squads: { + "foo-AAAAAAAA": makeStateEntry("AAAAAAAA-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set(["squads/foo.yml"])), + }); + assert.equal(report.rekeys.length, 1); + assert.equal(report.rekeys[0]!.toKey, "foo"); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Multi-segment-before-uuid8 — the exact shape from the user's bug report. +// ───────────────────────────────────────────────────────────────────────────── + +test("recanonicalize: collapses multi-dash base slug ('foo-vmd-') — the exact shape the orphan-gate pairing missed", () => { + // From the live incident: state key was + // `iform-voicemail-triage-squad-llm-only-vmd-004c5108`. The orphan-gate's + // extractBaseSlug pairing failed because base = "...-vmd" not "...". + // This pass operates on raw UUID-suffix shape, so it recanonicalizes + // regardless of how many dash-segments precede the UUID8 — as long as + // the canonical local file exists. + const state = makeStateFile({ + squads: { + "iform-voicemail-triage-squad-llm-only-vmd-004c5108": makeStateEntry( + "004c5108-aaaa-bbbb-cccc-dddddddddddd", + ), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists( + new Set(["squads/iform-voicemail-triage-squad-llm-only-vmd.yml"]), + ), + }); + assert.equal(report.rekeys.length, 1); + assert.equal( + report.rekeys[0]!.toKey, + "iform-voicemail-triage-squad-llm-only-vmd", + ); + assert.equal(report.conflicts.length, 0); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Scoped-push integration — H1 regression guard +// ───────────────────────────────────────────────────────────────────────────── + +test("recanonicalize: passing `touched` marks both old and new keys so mergeScoped flushes the rename (H1 regression)", async () => { + // Without `touched`, mergeScoped re-overlays the on-disk stale key over + // our in-memory rename, persisting a state file with BOTH keys pointing + // at the same UUID. With `touched`, both the deleted UUID-suffixed key + // (so the deletion flushes) and the new canonical key (so the new entry + // is overlaid) are marked. + const { mergeScoped } = await import("../src/state-merge.ts"); + const emptyTouched = () => ({ + 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(), + }); + + const onDisk = makeStateFile({ + squads: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + // Simulate the push pipeline: load disk state → recanonicalize + // (mutates in-place) → run apply phase → save via mergeScoped. + const inMemory = makeStateFile({ + squads: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + const touched = emptyTouched(); + const report = recanonicalizeStateKeys({ + state: inMemory, + fileExists: makeFileExists(new Set(["squads/foo.yml"])), + touched, + }); + assert.equal(report.rekeys.length, 1); + assert.ok(touched.squads.has("foo-aaaaaaaa")); + assert.ok(touched.squads.has("foo")); + + // The save-time merge must produce a state with ONLY the canonical key. + const merged = mergeScoped(onDisk, inMemory, touched); + assert.deepEqual(Object.keys(merged.squads), ["foo"]); + assert.equal( + merged.squads["foo"]!.uuid, + "aaaaaaaa-0000-0000-0000-000000000000", + ); +}); + +test("recanonicalize: omitting `touched` is allowed (pull path saves wholesale)", () => { + // Pull's saveState replaces the entire state file, so it doesn't need + // `touched`. The pass should not throw when omitted. + const state = makeStateFile({ + squads: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set(["squads/foo.yml"])), + }); + assert.equal(report.rekeys.length, 1); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Cross-section safety +// ───────────────────────────────────────────────────────────────────────────── + +test("recanonicalize: state.credentials section is never touched (credentials are name-keyed, not slug-uuid8)", () => { + // Credentials use credential names as keys ('twilio-prod'), not the + // engine-generated `-` shape. Even if a credential name + // happened to look UUID-suffixed, recanonicalize must not touch the + // credentials section because it's not in VALID_RESOURCE_TYPES. + const state = makeStateFile({ + credentials: { + "looks-suffixed-aaaaaaaa": makeStateEntry( + "aaaaaaaa-0000-0000-0000-000000000000", + ), + }, + }); + const report = recanonicalizeStateKeys({ + state, + fileExists: makeFileExists(new Set()), + }); + assert.equal(report.rekeys.length, 0); + assert.equal(report.conflicts.length, 0); + assert.deepEqual(Object.keys(state.credentials), ["looks-suffixed-aaaaaaaa"]); +}); + +test("recanonicalize: `types` option restricts the pass to the named types only (typeFilter wiring)", () => { + // Used by pull's typeFilter gate so a `--type squads` pull doesn't sweep + // tools/assistants that this run didn't refresh. + const state = makeStateFile({ + squads: { + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), + }, + tools: { + "bar-bbbbbbbb": makeStateEntry("bbbbbbbb-0000-0000-0000-000000000000"), + }, + }); + const report = recanonicalizeStateKeys({ + state, + types: ["squads"], + fileExists: makeFileExists(new Set(["squads/foo.yml", "tools/bar.yml"])), + }); + assert.equal(report.rekeys.length, 1); + assert.equal(report.rekeys[0]!.type, "squads"); + // Tools section unchanged because it wasn't in `types`. + assert.deepEqual(Object.keys(state.tools), ["bar-bbbbbbbb"]); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Reporter formatting +// ───────────────────────────────────────────────────────────────────────────── + +test("formatRecanonicalizeReport: empty report renders empty string (callers can unconditionally log)", () => { + const out = formatRecanonicalizeReport({ rekeys: [], conflicts: [] }); + assert.equal(out, ""); +}); + +test("formatRecanonicalizeReport: rekeys section lists each from→to mapping", () => { + const out = formatRecanonicalizeReport({ + rekeys: [ + { + type: "squads" as ResourceType, + fromKey: "foo-aaaaaaaa", + toKey: "foo", + uuid: "aaaaaaaa-0000-0000-0000-000000000000", + }, + ], + conflicts: [], + }); + assert.match(out, /Recanonicalized 1 state key/); + assert.match(out, /squads\/foo-aaaaaaaa → squads\/foo/); +}); + +test("formatRecanonicalizeReport: conflicts carry actionable per-reason hints", () => { + const out = formatRecanonicalizeReport({ + rekeys: [], + conflicts: [ + { + type: "squads" as ResourceType, + uuidSuffixedKey: "foo-aaaaaaaa", + canonicalKey: "foo", + reason: "both-local-files-exist", + uuid: "aaaaaaaa-0000-0000-0000-000000000000", + }, + ], + }); + assert.match(out, /both .* exist/); + assert.match(out, /pick one and delete the other/); +});