Skip to content

fix(push,pull): recanonicalize stale UUID-suffixed state keys — root-cause duplicate generation#32

Merged
dhruva-reddy merged 1 commit into
mainfrom
pull-rekey-one-way-bug
May 15, 2026
Merged

fix(push,pull): recanonicalize stale UUID-suffixed state keys — root-cause duplicate generation#32
dhruva-reddy merged 1 commit into
mainfrom
pull-rekey-one-way-bug

Conversation

@dhruva-reddy
Copy link
Copy Markdown
Contributor

Summary

Generic, resource-type-agnostic state-key recanonicalization pass that root-causes the recurring duplicate-generation pattern. 10+ prior PRs (#23 tool dedup, #30 orphan-YAML gate, etc.) addressed this symptomatically; this fixes the underlying state-file asymmetry.

The bug class. Pull rewrites state keys to UUID-suffixed form (<base>-<uuid8>) when name-collision adoption is refused — that's intentional. But nothing ever undoes that rekey when the conflict resolves (e.g. the conflicting dashboard twin is deleted). Next push sees the canonical-slug local file as an orphan-YAML, the operator passes --allow-new-files to bypass the gate, and the engine silently creates a third dashboard duplicate. The cycle repeats indefinitely.

The fix. A single generic pass walks every section of StateFile uniformly, with 5 safety preconditions for each rekey:

  1. Key matches the engine-generated shape ^(.+)-([0-9a-f]{8})$
  2. Captured suffix matches the entry's UUID prefix (proves engine wrote it)
  3. Canonical slug is unclaimed in state (no same-name twin collision)
  4. Local file exists at canonical slug under any VALID_EXTENSIONS shape (.yml/.yaml/.ts/.md)
  5. NO local file at UUID-suffixed slug (avoid silent data-loss when both snapshots exist)

Same-UUID self-aliases auto-resolve (the redundant entry is dropped). Different-UUID twins are refused with canonical-slug-claimed-by-different-uuid. The both-local-files-exist case is surfaced for manual resolution — we never silently pick a winner.

Where it runs.

  • End of runPull, gated on !bootstrap && !resourceIds, honors typeFilter
  • Start of runPush after maybeBootstrapState, before the orphan-YAML gate
  • touched is plumbed through on the push side so scoped pushes flush the rename via mergeScoped (H1 from the in-branch code review)

Why it's resource-type-agnostic. Iterates VALID_RESOURCE_TYPES and uses FOLDER_MAP + VALID_EXTENSIONS from src/resources.ts. Adding a new ResourceType requires zero changes to this file.

Files changed

File Status LOC
src/recanonicalize.ts NEW ~290
tests/recanonicalize.test.ts NEW (17 cases) ~370
src/pull.ts Wires recanonicalize at end of runPull +20
src/push.ts Wires recanonicalize after bootstrap, before orphan gate; passes touched +30
src/resources.ts Exports VALID_EXTENSIONS (was private) so precondition 5 stays in lockstep with the loader +6

Test plan

  • npm run build (tsc --noEmit) — clean
  • npm test208/208 pass (191 prior + 17 new)
  • npx @biomejs/biome check --write — clean
  • Happy path: collapses UUID-suffixed slug when canonical local file present + canonical slot empty
  • Every-type uniformity: all 9 resource types collapse with a single pass
  • Precondition tests: 1 (regex shape), 2 (UUID prefix match — rejects user-named my-tool-deadbeef), 3 (different-UUID twin), 4 (canonical local file missing), 5 (both files exist — all VALID_EXTENSIONS)
  • H1 regression: touched plumbing exercised through mergeScoped end-to-end
  • H2 regression: same-UUID self-alias auto-resolves (not a conflict)
  • Cross-section: state.credentials never touched (not in VALID_RESOURCE_TYPES)
  • Multi-segment slug: the exact iform-voicemail-triage-squad-llm-only-vmd-004c5108 shape from the 2026-05-14 incident
  • .ts extension regression guard (post-merge review surfaced this gap before merge)

Code review

Two rounds of in-branch review:

Round 1 (pre-push). Code-reviewer surfaced:

  • H1 (blocking): scoped push silently discarded the rekey via mergeScoped because touched wasn't plumbed. Fixed by adding touched parameter and marking both old + new keys. Regression test added.
  • H2 (blocking): canonical-slug-claimed conflict reason was misleading when both keys aliased the same UUID. Fixed by detecting the self-alias case and auto-resolving as a rekey (drop the redundant entry).
  • M1 (non-blocking): clarified comment that internal bootstrap-recovery pull DOES run recanonicalize (only explicit --bootstrap skips it).
  • M2 (non-blocking): typeFilter now restricts the pass to refreshed types on scoped pulls.

Round 2 (post-merge audit). Reviewer surfaced:

  • Should-fix: RESOURCE_EXTENSIONS was a 3-entry local constant, omitting .ts. Fixed by importing VALID_EXTENSIONS from src/resources.ts — the loader's canonical list — so any future loader extension is automatically respected. Two regression tests added (precondition 4 + precondition 5 on .ts pairs).

Related

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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant