Skip to content

Commit e6248cf

Browse files
authored
fix(push,pull): recanonicalize stale UUID-suffixed state keys (#32)
Generic, resource-type-agnostic pass that collapses engine-generated `<base>-<uuid8>` state keys back to canonical `<base>` 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).
1 parent 4c00cb4 commit e6248cf

5 files changed

Lines changed: 864 additions & 1 deletion

File tree

src/pull.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import {
1616
VAPI_TOKEN,
1717
} from "./config.ts";
1818
import { credentialReverseMap, replaceCredentialRefs } from "./credentials.ts";
19+
import {
20+
formatRecanonicalizeReport,
21+
recanonicalizeStateKeys,
22+
} from "./recanonicalize.ts";
1923
import { hashPayload, loadState, saveState, upsertState } from "./state.ts";
2024
import type { ResourceState, ResourceType, StateFile } from "./types.ts";
2125

@@ -1046,6 +1050,26 @@ export async function runPull(options: PullOptions = {}): Promise<PullResult> {
10461050
resourceIds,
10471051
});
10481052

1053+
// Collapse UUID-suffixed state keys back to canonical when the underlying
1054+
// name-collision has resolved (the conflicting twin was deleted, etc.).
1055+
// Skipped during bootstrap because bootstrap is supposed to populate state
1056+
// from scratch — there is no prior rekey to undo. Also skipped on targeted
1057+
// ID pulls so we don't sweep types we haven't fully refreshed. When the
1058+
// pull is scoped by typeFilter, the pass is restricted to those types so
1059+
// we don't touch sections this pull didn't refresh — preserves the stated
1060+
// safety boundary even though the preconditions themselves are safe.
1061+
// Pull's saveState writes wholesale, so `touched` isn't needed here.
1062+
if (!bootstrap && !resourceIds?.length) {
1063+
const report = recanonicalizeStateKeys({
1064+
state,
1065+
types: typeFilter?.length ? (typeFilter as ResourceType[]) : undefined,
1066+
});
1067+
const summary = formatRecanonicalizeReport(report);
1068+
if (summary) {
1069+
console.log(`\n${summary}`);
1070+
}
1071+
}
1072+
10491073
await saveState(state);
10501074

10511075
// Summary

src/push.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ import {
2222
} from "./dep-dedup.ts";
2323
import { checkDriftForUpdate } from "./drift.ts";
2424
import { detectOrphanYamls, formatGateMessage } from "./new-file-gate.ts";
25+
import {
26+
formatRecanonicalizeReport,
27+
recanonicalizeStateKeys,
28+
} from "./recanonicalize.ts";
2529
import { writeSnapshot } from "./snapshot.ts";
2630
import { mergeScoped } from "./state-merge.ts";
2731
import {
@@ -1367,6 +1371,35 @@ async function main(): Promise<void> {
13671371

13681372
state = await maybeBootstrapState(loadedResources, state);
13691373

1374+
// Recanonicalize stale UUID-suffixed state keys back to canonical slugs
1375+
// before the orphan-YAML gate runs. This is the safe collapse of the
1376+
// pull-side rekey-on-name-collision behavior (src/pull.ts) once the
1377+
// underlying collision has resolved (e.g. the conflicting twin was
1378+
// deleted on the dashboard). Without this pass, the orphan-YAML gate
1379+
// sees the canonical-slug local file as "new" — the recurring
1380+
// duplicate-creation root cause documented in improvements.md.
1381+
//
1382+
// Gating: `BOOTSTRAP_SYNC` (the user's explicit `--bootstrap` flag)
1383+
// skips this pass because state is being rebuilt from scratch. Note
1384+
// that the *internal* bootstrap-recovery pull triggered by
1385+
// `maybeBootstrapState` above does NOT set `BOOTSTRAP_SYNC` — so
1386+
// recanonicalize still runs after that recovery, which is intentional:
1387+
// bootstrap freshly generates UUID-suffixed keys for unresolved
1388+
// collisions, and recanonicalize collapses any that no longer have a
1389+
// live conflict.
1390+
//
1391+
// `touched` is plumbed through so scoped pushes flush the rename via
1392+
// `mergeScoped` at save time. Without this, the on-disk stale key
1393+
// would silently re-overwrite the in-memory canonical rename — H1 in
1394+
// the code review.
1395+
if (!BOOTSTRAP_SYNC) {
1396+
const recanonReport = recanonicalizeStateKeys({ state, touched });
1397+
const recanonSummary = formatRecanonicalizeReport(recanonReport);
1398+
if (recanonSummary) {
1399+
console.log(`\n${recanonSummary}`);
1400+
}
1401+
}
1402+
13701403
// Orphan-YAML pre-flight gate. Runs ONCE for ALL resource types after
13711404
// bootstrap (so state-recovery has a chance to rekey first) and BEFORE
13721405
// any apply phase. Halts push when local files exist with no state entry

src/recanonicalize.ts

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,273 @@
1+
// ─────────────────────────────────────────────────────────────────────────────
2+
// State key recanonicalization — generic across all resource types.
3+
//
4+
// The pull engine generates UUID-suffixed state keys (`<base>-<uuid8>`) when
5+
// name-collision adoption is refused (src/pull.ts:findExistingResourceId /
6+
// generateResourceId). That rekey is fire-and-forget: once the underlying
7+
// collision resolves (e.g. the conflicting twin is deleted on the dashboard),
8+
// nothing ever collapses the UUID-suffixed entry back to its canonical slug.
9+
//
10+
// Without this collapse, subsequent pushes treat the canonical-slug local
11+
// file as a brand-new resource (orphan-YAML), and the operator either hits
12+
// the orphan-YAML gate (#30) and bypasses with `--allow-new-files`, or — in
13+
// older engine versions — silently creates a third dashboard duplicate.
14+
//
15+
// This module runs a safe collapse pass at end-of-pull and start-of-push.
16+
// The pass is resource-type-agnostic: it walks every section of StateFile
17+
// uniformly, so adding a new ResourceType doesn't need a code change here.
18+
//
19+
// SAFETY MODEL — every rekey must satisfy all five preconditions:
20+
// 1. Key matches `^(.+)-([0-9a-f]{8})$` — the engine's generated shape.
21+
// 2. The captured `<uuid8>` matches the entry's UUID prefix. This rules
22+
// out resources whose user-given names legitimately end in
23+
// `-abcd1234`. We only touch keys we can prove the engine wrote.
24+
// 3. Canonical slug `<base>` is unclaimed in the SAME state section.
25+
// Prevents collision when a same-name twin is intentionally tracked.
26+
// 4. A local file exists at `<base>` with any extension recognized by
27+
// the resource loader (`VALID_EXTENSIONS` in src/resources.ts —
28+
// .yml/.yaml/.ts/.md). Prevents creating phantom state mappings to
29+
// slugs that have no source file. The extension set is imported, not
30+
// hardcoded here, so the precondition stays in lockstep with the
31+
// loader; any future loader extension is automatically respected.
32+
// 5. NO local file exists at `<base>-<uuid8>` under any
33+
// `VALID_EXTENSIONS` shape. The UUID-suffixed file represents a
34+
// different content snapshot; rekeying state without consolidating
35+
// files would silently reassign which file PATCHes which dashboard
36+
// UUID — the data-loss shape we are explicitly refusing to introduce.
37+
//
38+
// When (5) fails we surface a CONFLICT (both files exist, ambiguous which
39+
// owns the dashboard UUID) so the operator can resolve it manually. We
40+
// never auto-pick a winner — silent data loss is worse than a duplicate.
41+
// ─────────────────────────────────────────────────────────────────────────────
42+
43+
import { existsSync } from "fs";
44+
import { join } from "path";
45+
import { RESOURCES_DIR } from "./config.ts";
46+
import { FOLDER_MAP, VALID_EXTENSIONS } from "./resources.ts";
47+
import type { TouchedSets } from "./state-merge.ts";
48+
import type { ResourceType, StateFile } from "./types.ts";
49+
import { VALID_RESOURCE_TYPES } from "./types.ts";
50+
51+
const UUID_SUFFIX_RE = /^(.+)-([0-9a-f]{8})$/i;
52+
53+
export interface RecanonicalizeRekey {
54+
type: ResourceType;
55+
fromKey: string;
56+
toKey: string;
57+
uuid: string;
58+
}
59+
60+
export interface RecanonicalizeConflict {
61+
type: ResourceType;
62+
uuidSuffixedKey: string;
63+
canonicalKey: string;
64+
reason:
65+
| "canonical-slug-claimed-by-different-uuid"
66+
| "both-local-files-exist"
67+
| "canonical-local-file-missing";
68+
uuid: string;
69+
}
70+
71+
export interface RecanonicalizeReport {
72+
rekeys: RecanonicalizeRekey[];
73+
conflicts: RecanonicalizeConflict[];
74+
}
75+
76+
export interface RecanonicalizeOptions {
77+
state: StateFile;
78+
// DI seam — defaults to filesystem check rooted at `RESOURCES_DIR`. Tests
79+
// pass a stub so they don't need a fixture tree.
80+
fileExists?: (relativePath: string) => boolean;
81+
// Restrict the pass to specific types (default: all).
82+
types?: ResourceType[];
83+
// Optional `touched` set for scoped pushes. When provided, both the
84+
// deleted UUID-suffixed key AND the new canonical key are marked so
85+
// `mergeScoped` flushes the rename instead of silently re-persisting the
86+
// on-disk stale key. Pull doesn't pass this — it saves wholesale.
87+
touched?: TouchedSets;
88+
}
89+
90+
function defaultFileExists(relativePath: string): boolean {
91+
return existsSync(join(RESOURCES_DIR, relativePath));
92+
}
93+
94+
function localFileExistsForId(
95+
type: ResourceType,
96+
resourceId: string,
97+
fileExists: (relativePath: string) => boolean,
98+
): boolean {
99+
const folder = FOLDER_MAP[type];
100+
for (const ext of VALID_EXTENSIONS) {
101+
if (fileExists(`${folder}/${resourceId}${ext}`)) return true;
102+
}
103+
return false;
104+
}
105+
106+
// Mutates `state` in place. Returns the list of changes for logging and the
107+
// list of unresolved conflicts the operator should see. Callers decide
108+
// whether conflicts halt the run (push) or are advisory (pull).
109+
export function recanonicalizeStateKeys(
110+
opts: RecanonicalizeOptions,
111+
): RecanonicalizeReport {
112+
const {
113+
state,
114+
fileExists = defaultFileExists,
115+
types = [...VALID_RESOURCE_TYPES],
116+
touched,
117+
} = opts;
118+
119+
const rekeys: RecanonicalizeRekey[] = [];
120+
const conflicts: RecanonicalizeConflict[] = [];
121+
122+
const markTouched = (type: ResourceType, key: string): void => {
123+
if (touched) touched[type].add(key);
124+
};
125+
126+
for (const type of types) {
127+
const section = state[type];
128+
if (!section) continue;
129+
130+
// Snapshot keys upfront so we can mutate the section during iteration.
131+
for (const stateKey of Object.keys(section)) {
132+
const entry = section[stateKey];
133+
if (!entry) continue;
134+
135+
const match = stateKey.match(UUID_SUFFIX_RE);
136+
if (!match) continue;
137+
138+
const [, canonicalSlug, capturedUuid8] = match;
139+
if (!canonicalSlug || !capturedUuid8) continue;
140+
141+
// Precondition 2 — only recanonicalize engine-generated suffixes. If
142+
// the captured 8 hex chars don't match the entry's UUID prefix, this
143+
// is a user-named resource that coincidentally looks suffixed.
144+
//
145+
// Mirrors generateResourceId() in src/pull.ts:265-273 — UUID dashes
146+
// start at index 8, so `slice(0, 8)` on the raw UUID is equivalent
147+
// to stripping dashes first; the dash-strip is defensive.
148+
const entryUuid8 = entry.uuid.replace(/-/g, "").slice(0, 8).toLowerCase();
149+
if (capturedUuid8.toLowerCase() !== entryUuid8) continue;
150+
151+
// Precondition 3 — canonical slot must be unclaimed in state.
152+
//
153+
// Special case: if the canonical slot is claimed by the SAME UUID,
154+
// both keys are aliases for one dashboard resource. This shape is
155+
// produced by older engine versions that wrote duplicate-aliased
156+
// state, or by a `mergeScoped` round-trip before recanonicalize was
157+
// touched-aware. Safe action: drop the redundant UUID-suffixed key
158+
// (canonical wins; its metadata is presumed more authoritative).
159+
// This is auto-resolved as a rekey for reporting purposes.
160+
const claimedEntry = section[canonicalSlug];
161+
if (claimedEntry !== undefined) {
162+
if (claimedEntry.uuid === entry.uuid) {
163+
delete section[stateKey];
164+
markTouched(type, stateKey);
165+
markTouched(type, canonicalSlug);
166+
rekeys.push({
167+
type,
168+
fromKey: stateKey,
169+
toKey: canonicalSlug,
170+
uuid: entry.uuid,
171+
});
172+
continue;
173+
}
174+
// Genuinely a different UUID — same-name twin tracked legitimately.
175+
conflicts.push({
176+
type,
177+
uuidSuffixedKey: stateKey,
178+
canonicalKey: canonicalSlug,
179+
reason: "canonical-slug-claimed-by-different-uuid",
180+
uuid: entry.uuid,
181+
});
182+
continue;
183+
}
184+
185+
// Precondition 4 — canonical local file must exist. Otherwise we'd
186+
// be inventing a state mapping that has no source on disk.
187+
const canonicalFileExists = localFileExistsForId(
188+
type,
189+
canonicalSlug,
190+
fileExists,
191+
);
192+
if (!canonicalFileExists) {
193+
conflicts.push({
194+
type,
195+
uuidSuffixedKey: stateKey,
196+
canonicalKey: canonicalSlug,
197+
reason: "canonical-local-file-missing",
198+
uuid: entry.uuid,
199+
});
200+
continue;
201+
}
202+
203+
// Precondition 5 — UUID-suffixed local file must NOT exist. If both
204+
// files exist they represent different content snapshots; silently
205+
// rekeying would change which file PATCHes which dashboard UUID.
206+
const uuidSuffixedFileExists = localFileExistsForId(
207+
type,
208+
stateKey,
209+
fileExists,
210+
);
211+
if (uuidSuffixedFileExists) {
212+
conflicts.push({
213+
type,
214+
uuidSuffixedKey: stateKey,
215+
canonicalKey: canonicalSlug,
216+
reason: "both-local-files-exist",
217+
uuid: entry.uuid,
218+
});
219+
continue;
220+
}
221+
222+
// All preconditions met — collapse. Mark BOTH the deleted UUID-suffixed
223+
// key AND the new canonical key as touched, so scoped pushes flush the
224+
// rename via `mergeScoped` instead of silently re-persisting the
225+
// on-disk stale key.
226+
section[canonicalSlug] = entry;
227+
delete section[stateKey];
228+
markTouched(type, stateKey);
229+
markTouched(type, canonicalSlug);
230+
rekeys.push({
231+
type,
232+
fromKey: stateKey,
233+
toKey: canonicalSlug,
234+
uuid: entry.uuid,
235+
});
236+
}
237+
}
238+
239+
return { rekeys, conflicts };
240+
}
241+
242+
// Human-readable summary for stdout. Empty report renders nothing — callers
243+
// can unconditionally log.
244+
export function formatRecanonicalizeReport(
245+
report: RecanonicalizeReport,
246+
): string {
247+
if (report.rekeys.length === 0 && report.conflicts.length === 0) return "";
248+
249+
const lines: string[] = [];
250+
if (report.rekeys.length > 0) {
251+
lines.push(
252+
`🔧 Recanonicalized ${report.rekeys.length} state key(s) — collapsed UUID-suffixed slug back to canonical:`,
253+
);
254+
for (const r of report.rekeys) {
255+
lines.push(` ${r.type}/${r.fromKey}${r.type}/${r.toKey}`);
256+
}
257+
}
258+
if (report.conflicts.length > 0) {
259+
lines.push(
260+
`⚠️ ${report.conflicts.length} UUID-suffixed state key(s) NOT recanonicalized (manual resolution required):`,
261+
);
262+
for (const c of report.conflicts) {
263+
const hint =
264+
c.reason === "canonical-slug-claimed-by-different-uuid"
265+
? `canonical slug ${c.canonicalKey} already claimed by a different dashboard UUID (legitimate same-name twin)`
266+
: c.reason === "both-local-files-exist"
267+
? `both ${c.canonicalKey}.{yml,yaml,ts,md} and ${c.uuidSuffixedKey}.{yml,yaml,ts,md} exist — pick one and delete the other before next push`
268+
: `no local file at ${c.canonicalKey}.{yml,yaml,ts,md} — state entry is stale; either restore the file or remove the state entry`;
269+
lines.push(` ${c.type}/${c.uuidSuffixedKey}: ${hint}`);
270+
}
271+
}
272+
return lines.join("\n");
273+
}

src/resources.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,18 @@ const FOLDER_TO_TYPE: Record<string, ResourceType> = Object.entries(
4242
// Resource Loading
4343
// ─────────────────────────────────────────────────────────────────────────────
4444

45-
const VALID_EXTENSIONS = [".yml", ".yaml", ".ts", ".md"];
45+
// Single source of truth for resource file extensions. Imported by
46+
// `recanonicalize.ts` so the precondition-5 "both files exist" check
47+
// stays in lockstep with the loader — without this, a `.ts`-authored
48+
// resource paired with a UUID-suffixed `.ts` twin would be invisible to
49+
// the safety check and silently allow the data-loss shape the
50+
// recanonicalize header explicitly refuses.
51+
export const VALID_EXTENSIONS: readonly string[] = [
52+
".yml",
53+
".yaml",
54+
".ts",
55+
".md",
56+
];
4657

4758
/**
4859
* Parse a markdown file with YAML frontmatter

0 commit comments

Comments
 (0)