fix(store): let the edge table rebuild run, and survive orphaned edges

[audreyt]

Two defects in migrateRemoveNarrativeEdges, both caused by foreign key enforcement being on while the migration works.

The migration never runs

It probes for the old schema by inserting a sentinel edge whose endpoints do not exist:

INSERT INTO edges VALUES ('__test','__test','narrative', ...)

The driver opens the database with foreign_keys(1), so that insert fails on the foreign key long before the CHECK constraint is consulted — and the error is read as "the schema already rejects 'narrative'". Every database opened through the driver therefore skips the migration and keeps the old constraint indefinitely.

If it did run, it could still abort

The rebuild copies every row into a fresh table, and that copy is rejected by any pre-existing dangling edge. Orphans are not exotic: a .recover after corruption, or any write made while enforcement was off, leaves edges pointing at rows that are gone. One store examined while investigating this held 51. Because the migration runs on every open, one such row strands the database permanently.

The fix

Follow SQLite's documented table-rebuild procedure: disable enforcement across both the probe and the rebuild, restore it after. The pragma is a no-op inside a transaction, so it is toggled around one; the pool is capped at a single connection, so it applies to the connection doing the work.

Orphans are copied verbatim rather than filtered out. Dropping them would be a silent data deletion justified only by the convenience of this migration, and their presence is evidence worth keeping.

Test: TestMigrateRemoveNarrativeEdges_ToleratesOrphanedEdges reproduces both defects. Without the change it fails on narrative edge type must be rejected after migration — i.e. it proves the migration silently did nothing.

[Copilot]

Pull request overview

Fixes migrateRemoveNarrativeEdges so the edges-table rebuild migration actually runs under driver-enabled FK enforcement and can complete even when legacy stores contain orphaned edges (dangling source_id/target_id references).

Changes:

• Disable SQLite foreign key enforcement around the probe and table rebuild portions of migrateRemoveNarrativeEdges, restoring it afterward.
• Add a regression test that downgrades the edges CHECK constraint to re-allow narrative, injects an orphan edge with enforcement off, and verifies (a) the orphan survives and (b) narrative becomes rejected after reopen/migration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/memory/store/db.go	Disables FK enforcement around the narrative-edge probe and rebuild to ensure the migration runs and tolerates orphaned edges.
internal/memory/store/store_test.go	Adds a regression test that reproduces the FK/probe issue and validates orphan tolerance + narrative rejection post-migration.

Suppressed comments (1)

internal/memory/store/db.go:469

• The probe insert is executed outside the migration transaction, and it uses fixed IDs ('__test', '__test'). If the migration later errors/crashes after the probe succeeds, a sentinel row can be left behind; on the next open the probe can then fail with a UNIQUE/PK constraint and this code will incorrectly treat it as “schema already rejects narrative”, permanently skipping the migration. Also, because insight IDs are caller-provided, using fixed sentinel IDs risks colliding with real data.

Consider probing inside a short transaction/savepoint and rolling it back, and use random IDs for the probe row so it can’t collide. With a rollback-based probe, the migration steps no longer need to delete rows by a sentinel ID (dropping the DELETE ... source_id='__test' step avoids accidental data loss if an insight id ever equals '__test').

	// Probe whether the old schema allows 'narrative'
	_, testErr := db.conn.Exec(`INSERT INTO edges VALUES ('__test','__test','narrative',0,'{}',datetime('now'))`)
	if testErr != nil {
		return nil // current schema already rejects 'narrative', nothing to do
	}

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[audreyt]

Updated. The automated review raised the probe as a suppressed comment, and it was right — sharper than it looked, because this PR is what makes that hazard reachable.

Before the change the probe insert always failed on the foreign key, so no sentinel row was ever written. With enforcement off it succeeds, in autocommit, and only the rebuild transaction deletes it. If any step fails or the process dies, the row stays. The next open's probe then collides with it on the primary key, this function reads any error as "the schema already rejects narrative", and the database is stranded on the old schema permanently — the exact failure this PR exists to repair, arriving through a different door.

The probe now runs in a transaction that is always rolled back, so nothing survives it however the migration ends, and its id is '__probe_'||hex(randomblob(8)) rather than the fixed '__test'. Insight ids are caller-supplied, so a fixed sentinel can name a real row — and the DELETE FROM edges WHERE source_id = '__test' step could take real edges with it. That step is gone; a sentinel left behind by the older shape is a narrative row, so the existing delete sweeps it up. A store already stranded this way now recovers on its next open.

Two tests, both failing without the change:

• TestMigrateRemoveNarrativeEdges_ProbeSurvivesNothing — blocks the rebuild the way a crash would (after the probe), then reopens. Fails on the leftover (probe row outlived a failed migration: 1 left behind) and, once unblocked, on the silent skip (narrative edge type must be rejected after migration).
• TestMigrateRemoveNarrativeEdges_KeepsRealEdgesNamedLikeTheSentinel — a real insight with id __test and a real semantic self-edge. Fails with got 0: the cleanup deleted live data.

Also verified end to end against a synthetic legacy store: narrative rejected afterwards, the orphaned edge preserved verbatim, zero probe rows in edges, PRAGMA integrity_check ok.

[Grivn]

LGTM — thanks for hardening this migration!