Skip to content

Prevent provider migration row ID collisions#1416

Open
morluto wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
morluto:fix/provider-service-migration-row-ids
Open

Prevent provider migration row ID collisions#1416
morluto wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
morluto:fix/provider-service-migration-row-ids

Conversation

@morluto

@morluto morluto commented Jul 15, 2026

Copy link
Copy Markdown

Fixes #1411

Summary

The provider service split migration generated target row_id values with a 32-bit hash, then used INSERT OR IGNORE. Distinct logical rows could collide, causing SQLite to discard one target row before the migration deleted both sources.

  • Generate deterministic IDs from the full SHA-256 digest of a JSON-encoded key tuple.
  • Make idempotency explicit with conflict targets for each logical key.
  • Let unrelated primary-key conflicts fail so the tenant transaction rolls back.
  • Add regression coverage for a concrete 32-bit collision and an unrelated target-ID conflict.
  • Add an executor patch changeset.

Regression proof

On origin/main, the collision regression failed because only al4jix0b survived migration while both source rows were deleted.

With this change, both ypw3e5cb and al4jix0b are preserved as distinct migrated connections.

A second SQLite regression injects an unrelated row with the generated target ID. The migration now returns DataMigrationError, rolls back, and preserves both the source row and pre-existing target row.

Compatibility and idempotency

  • No schema or public API changes.
  • Tenants already recorded in the migration ledger remain skipped.
  • Incomplete writes remain inside the existing transaction.
  • Replaying the same logical row is a no-op through its logical unique key.
  • An unrelated primary-key conflict is now a visible failure instead of silent row loss.

Validation

  • Base regression: 1 of 5 tests failed for the intended missing-row assertion.
  • Focused migration suite: 6 of 6 tests passed.
  • Full package suite: 36 of 36 tests passed.
  • Package typecheck, build, lint, formatting check, and git diff --check passed.
  • Branch autoreview against origin/main returned no actionable findings.

Review order

  1. Collision and rollback regressions in src/sqlite.test.ts.
  2. Deterministic ID generation in src/sqlite.ts.
  3. Logical conflict targets in src/sqlite.ts.

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.

Provider service split migration can drop rows when generated IDs collide

1 participant