fix: make corrupt-cache recovery say so instead of swallowing it - #342
Open
ProfSynapse wants to merge 2 commits into
Open
fix: make corrupt-cache recovery say so instead of swallowing it#342ProfSynapse wants to merge 2 commits into
ProfSynapse wants to merge 2 commits into
Conversation
`SQLitePersistenceService.loadDatabase` handled a failed integrity check in a
bare `catch {}`. The cache was discarded and rebuilt correctly, but nothing was
written to the console — so the only thing a user ever saw was the downstream
symptom (an empty or half-populated view) with no line tying it back to a
corrupt database. That is why #209 was undiagnosable from the reporter's
console for months.
Report it at console.error, which is where a user can actually find it, and say
all three things that matter: that the integrity check failed, the sqlite
`integrity_check` output or thrown cause behind it, and that no data is lost
because the cache is a rebuildable index replayed from the JSONL event store.
The outer read/deserialize failure now goes through the same reporter, so both
recovery paths explain themselves. The blob-delete failure inside
`recreateCorruptedDatabase` warns rather than vanishing — a remove that keeps
failing is the difference between corrupted once and corruption that can never
be cleared.
Recovery behaviour is unchanged; only its visibility is. All four new tests
fail against the pre-fix service (nothing is logged, or the log carries no
cause).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #209.
Why
SQLitePersistenceServiceswallowed the integrity-check failure in a barecatch {}. The database was quietly rebuilt and the user saw only downstream symptoms — repeatedwaitForQueryReadytimeouts, embeddings skipped, tool calls hanging.That silence is why #209 went undiagnosed for months. The recovery worked; nobody could tell it had happened.
What changed
A new
reportCacheRebuildlogs atconsole.error, naming:integrity_checkoutput, or the thrown causeconsole.errorrather than the logger on purpose:logger.systemWarnandsystemLogare no-ops in this build, so routing through them would have re-hidden the thing this PR exists to surface.The outer read/deserialize catch routes through the same reporter, and the
blobStore.remove()bare catch now warns.The recovery logic itself is untouched. This only stops it being invisible.
Tests
Four, all failing pre-fix — three because nothing is logged at all, and one because the single existing line mentioned neither the rebuild nor the event store.
Not done
No live proof: reproducing it means deliberately corrupting the running vault's SQLite blob, and this branch didn't own the instance. The mechanism was reproduced during triage though — a 22.4 MB blob with a valid header and garbage pages, recovered on restart with all 803 conversations intact.
No
Noticewas added.console.errormatches the file's convention, and the brief was to stop hiding the event, not to add UI. A user-facing notice is a reasonable follow-up if you want one.🤖 Generated with Claude Code
https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
Generated by Claude Code