Skip to content

fix(integrity): safe hydration --repair with snapshot + drift detection + re-emit (#602)#607

Merged
dollspace-gay merged 3 commits into
developfrom
fix/602-integrity-repair-data-loss
May 15, 2026
Merged

fix(integrity): safe hydration --repair with snapshot + drift detection + re-emit (#602)#607
dollspace-gay merged 3 commits into
developfrom
fix/602-integrity-repair-data-loss

Conversation

@dollspace-gay
Copy link
Copy Markdown

@dollspace-gay dollspace-gay commented May 15, 2026

Summary

Fixes GH#602. crosslink integrity hydration --repair used to call clear_shared_data() followed by hydrate_to_sqlite(). Any SQLite row not represented in JSON was silently destroyed in the clear step. And the drift detection above the repair only compared issue/milestone counts, so content-level divergence (e.g. a dependencies (a, b) row in SQLite that no JSON file mentions) never registered as drift at all.

Implements all four mitigations from the issue, in the order it ranked them.

(1) Always snapshot before destroying state

Every --repair run writes .crosslink/integrity/hydration-backup-<utc-ts>.sqlite via SQLite's VACUUM INTO. Self-contained (works in WAL mode), reversible (drop it in place), and unconditional — runs even when the repair was non-destructive, as an audit trail of what state the repair was applied against.

New module: crosslink/src/db/snapshot.rs.

(2) Detect drift by content, not by count

New module commands::integrity_drift. detect() hydrates JSON into an isolated temp SQLite file (reusing the production hydrate_to_sqlite), ATTACH-es that file to the main connection, and SQL-diffs every shared table:

  • issues, labels, dependencies, relations, milestone_issues, comments, time_entries

Returns a categorized HydrationDriftReport with is_empty(), has_unrecoverable_loss(), and is_fully_re_emittable() semantics. The reproducer scenario from the issue (INSERT INTO dependencies directly into SQLite) now shows up in sqlite_only_dependencies where the old count-based check was blind to it.

(3) Re-emit SQLite-only state back to JSON when possible

re_emit() walks the drift report and calls SharedWriter::{add_label, add_blocker, add_relation, set_milestone_on_issues} for each row. The SharedWriter idempotency short-circuits from #600 (PR #605) mean these calls are safe even if the JSON side raced ahead between detection and re-emit.

Comments and time entries are NOT re-emitted:

  • Comments: re-emit would generate a new UUID and lose the original identity — the snapshot is their recovery handle.
  • Time entries: no SharedWriter API exists for them; they're written directly to SQLite by crosslink timer. Snapshot is the recovery handle.

(4) --accept-data-loss flag gates destructive repair

New CLI flag on crosslink integrity hydration. When drift contains rows that re-emit cannot represent, or when no SharedWriter is available (no agent.json / no hub branch), --repair refuses without the flag, prints the snapshot path, and exits non-zero. With the flag, proceeds destructively (snapshot is still written).

Behavior the user will notice

  • --repair always writes a snapshot at .crosslink/integrity/hydration-backup-<utc-ts>.sqlite (was: silent destroy).
  • --repair now tries to re-emit SQLite-only labels/deps/relations/milestone-links back to JSON via the writer — each becomes a real git commit on the hub branch.
  • --repair refuses when SQLite has comments or time entries that aren't in JSON, unless --accept-data-loss is also given.
  • Hydration check (without --repair) now surfaces content-level drift (e.g. "1 sqlite-only dependency(ies)") that the old count check missed.

Dependency change

tempfile moved from [dev-dependencies] to [dependencies] — the drift detector uses a temp SQLite file at runtime for the JSON-view ATTACH. No new transitive deps; the crate was already in the dev tree.

Out of scope

  • Snapshot retention / pruning. Hydration snapshots accumulate under .crosslink/integrity/. Could add a --prune-snapshots flag or a TTL in a follow-up.
  • Re-emit for sqlite-only issues. The existing #427 self-heal in hydrate_to_sqlite already preserves created_by IS NULL issues; full re-emit for SharedWriter-created issues with no JSON would mean a recursive create_issue path (parents, comments, the full graph), better as its own design.
  • Add a SharedWriter API for time entries. Same out-of-scope reasoning — separate feature.

Test plan

  • cargo test --lib --bin crosslink — 2875 passed (+9 new)
  • cargo clippy --lib --bin crosslink --tests -- -D warnings — clean
  • Snapshot tests: file created, integrity dir auto-created, UTC timestamp format
  • Drift detection tests: empty state, label-only drift, comment-only drift (unrecoverable), reproducer-scenario sqlite-only dependency
  • End-to-end re-emit integration test (test_re_emit_writes_sqlite_only_dependency_to_json): real SharedWriter + hub cache, injects SQLite-only dep, runs detect + re_emit, asserts JSON now contains the row and detect returns clean
  • Manual repro from the issue:
    crosslink issue create "first"  -q   # → L1
    crosslink issue create "second" -q   # → L2
    sqlite3 .crosslink/issues.db \
      "INSERT INTO dependencies (blocker_id, blocked_id) VALUES (1, 2);"
    crosslink integrity hydration                # should now report "1 sqlite-only dependency(ies)"
    crosslink integrity hydration --repair       # should snapshot, re-emit, and the dep should survive
    crosslink issue show L2 | grep -i blocked    # → Blocked by: L1 (preserved)

Related

🤖 Generated with Claude Code

…tion + re-emit (#602)

`crosslink integrity hydration --repair` used to call `clear_shared_data()`
followed by `hydrate_to_sqlite()`. Any SQLite row not represented in JSON
was silently destroyed in the clear step, and the drift-detection logic
above the repair only compared issue/milestone *counts*, so content-level
divergence (e.g. a `dependencies (a, b)` row in SQLite, not in either
issue's JSON `blockers`) never registered as drift at all.

This commit implements all four mitigations from the issue, in priority
order:

(1) **Always snapshot before destroying state.** Every `--repair` run
    writes `.crosslink/integrity/hydration-backup-<utc-ts>.sqlite` via
    SQLite's `VACUUM INTO`. The snapshot is self-contained (works in
    WAL mode), reversible (drop it in place), and unconditional (runs
    even when re-emit succeeds, as the audit trail of what state the
    repair was applied against).

(2) **Detect drift by content, not by count.** New module
    `commands::integrity_drift` hydrates JSON into a temp SQLite file,
    `ATTACH`-es it to the main connection, and SQL-diffs every shared
    table (issues, labels, dependencies, relations, milestone_issues,
    comments, time_entries). Returns a categorized
    `HydrationDriftReport`.

(3) **Re-emit SQLite-only state back to JSON when possible.**
    `integrity_drift::re_emit` walks the drift report and calls
    `SharedWriter::{add_label, add_blocker, add_relation,
    set_milestone_on_issues}` for each row, writing it back through
    the git event log so both sides converge to the union. Comments
    and time entries cannot be re-emitted (no writer API for time
    entries; re-emitting comments would lose original UUIDs) — the
    snapshot is their recovery path.

(4) **`--accept-data-loss` flag gates destructive repair.** When drift
    contains rows that re-emit cannot represent, or when no
    `SharedWriter` is available, `--repair` refuses without the new
    flag and points the user at the snapshot path. With the flag,
    proceeds destructively. Snapshot is always written either way.

New files:
- `crosslink/src/db/snapshot.rs` — `snapshot_to_integrity_dir`
- `crosslink/src/commands/integrity_drift.rs` — detect + re_emit

Tests: 2875 pass (+9). Three snapshot tests, five drift-detection tests
(including the issue's reproducer scenario), one end-to-end re-emit
integration test using a real `SharedWriter` + hub cache. Clippy clean
-D warnings.

`tempfile` moved from `[dev-dependencies]` to `[dependencies]` because
the drift detector uses a temp SQLite file at runtime.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dollspace-gay and others added 2 commits May 15, 2026 09:25
Apply rustfmt to integrity_drift.rs and snapshot.rs — minor whitespace
tweaks only (one closure body wrap, one with_context collapse). No
behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`cargo clippy -- -D warnings -W clippy::unwrap_used -W clippy::expect_used`
(the CI bar) now clean. Fixes:

- 26 × clippy::doc_markdown — backtick `SQLite`, `JSON`, `JSON`-derived,
  `JSON`-known, and field-name references in doc comments on the new
  integrity_drift module, snapshot module, and the new --accept-data-loss
  flag's clap doc.
- 3 × clippy::missing_const_for_fn — mark `HydrationDriftReport::is_empty`,
  `has_unrecoverable_loss`, `is_fully_re_emittable`, and `ReEmitStats::total`
  as `const fn`. They take only `&self` and read pod fields.
- 1 × clippy::map_unwrap_or — replace `.map(...).unwrap_or_else(...)` on
  the snapshot_rel path strip with `.map_or_else(...)`.

No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dollspace-gay dollspace-gay merged commit 07d791d into develop May 15, 2026
10 checks passed
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.

crosslink integrity hydration --repair deletes SQLite rows when the JSON files do not have them

1 participant