Skip to content

fix(db): keep FTS repair out of search reads#435

Merged
ScriptedAlchemy merged 1 commit into
masterfrom
codex/fts-read-path-safety
Jul 10, 2026
Merged

fix(db): keep FTS repair out of search reads#435
ScriptedAlchemy merged 1 commit into
masterfrom
codex/fts-read-path-safety

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Summary

  • keep search paths read-only when FTS is corrupt
  • distinguish FTS-only damage from whole-database corruption
  • preserve explicit maintenance-only FTS rebuild API

Tests

  • cargo test --test storage_suite corruption_test -- --nocapture
  • cargo fmt --all -- --check

Live dogfood: prevents the generic corruption misclassification observed during the v0.0.52 recovery incident.

@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5e028b4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0654bf74a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/db/search.rs Outdated
async fn primary_nodes_intact(&self) -> Result<bool> {
let mut rows = self
.conn()
.query("PRAGMA quick_check('nodes')", ())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check more than nodes before treating corruption as FTS-only

When an FTS MATCH fails with a corruption error, this table-scoped quick_check only validates nodes. If the same crash also damaged another table/page such as edges or unresolved_refs, primary_nodes_intact can still return true, causing search_nodes to fall through to the LIKE path and return successful partial results instead of the recovery-required error promised for whole-database corruption. Classify corruption as FTS-only only after validating the rest of the store, or at least all non-FTS tables read by later graph workflows.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5e028b4. The corruption fallback now enumerates every non-FTS table from sqlite_schema, safely quotes each identifier, and runs table-scoped PRAGMA quick_check so its associated indexes are validated too. Any missing required schema, non-FTS damage, or check error propagates the original corruption error. The regression now corrupts both the FTS segment and the unrelated edges table while nodes remains intact, and proves search returns the recovery-required error without writes. All 19 corruption tests and cargo fmt pass.

@ScriptedAlchemy ScriptedAlchemy force-pushed the codex/fts-read-path-safety branch from a0654bf to 5e028b4 Compare July 10, 2026 16:44
@ScriptedAlchemy ScriptedAlchemy merged commit 4f0d1b4 into master Jul 10, 2026
10 of 16 checks passed
@ScriptedAlchemy ScriptedAlchemy deleted the codex/fts-read-path-safety branch July 10, 2026 16:52
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