Skip to content

fix(store): scope people-fact migration checks to affected tables - #965

Open
shntnu wants to merge 1 commit into
kenn-io:mainfrom
shntnu:fix/person-fact-migration-scope
Open

shntnu wants to merge 1 commit into
kenn-io:mainfrom
shntnu:fix/person-fact-migration-scope

Conversation

@shntnu

@shntnu shntnu commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

The people-fact origin migration can block startup when an archive has pre-existing foreign-key violations in unrelated tables. Its database-wide PRAGMA foreign_key_check treats those older references as failures of the people-fact table rebuild.

Check person_fact_claims, person_fact_claim_evidence, and person_fact_decisions instead. Invalid references in those tables still fail the migration and roll back the rebuild; unrelated archive data remains unchanged.

The regression cases cover an unrelated orphan reference, an invalid claim parent, and an invalid evidence-to-claim reference. They also check rollback and restoration of foreign-key enforcement.

@roborev-ci

roborev-ci Bot commented Sep 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (e36969d)

Verdict: Changes require fixes for 1 finding.

Medium

  • internal/store/migrate_person_brief_ledger.go:263: The checks on person_fact_claim_evidence and person_fact_decisions count every foreign-key violation in those tables, including unaffected links such as evidence_id, resolution_id, and person_id. Existing unrelated corruption can therefore abort this claim-origin migration despite the intended scoping behavior.

    Fix: Filter the table-valued pragma results to foreign keys whose parent is person_fact_claims while retaining checks for the rebuilt table itself, and add coverage for an unrelated violation inside an otherwise affected table.


Reviewers: codex, codex (security) | Synthesis: codex, 4s | Total: 2m26s

@wesm

wesm commented Sep 27, 2026

Copy link
Copy Markdown
Member

looking

@wesm wesm self-assigned this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants