Fix the real bugs hiding in the detekt baseline#11
Merged
Conversation
A process kill during the non-atomic writeText() left the file truncated; getHistory() swallowed the parse error and returned an empty list, which the next saveItem() persisted — destroying every stored scan. Now writes go through a tmp-file rename, unparseable history is preserved as scan_history.corrupt.json instead of being overwritten, and the JSON codec is extracted into pure functions with regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- KeyCipher: narrow decrypt() catch and log the failure class (never the payload); call out KeyPermanentlyInvalidatedException so silent BYOK key loss after a lock-screen change is at least diagnosable - AiScanViewModel/OcrViewModel: rethrow CancellationException before the generic catch and log history persistence failures - ActionExecutor: narrow the nine intent-launch catches to ActivityNotFoundException/SecurityException/IllegalArgumentException and log alongside the existing toasts; parseEventMillis catches ParseException - ProviderClient: chain the SerializationException into TransientAiException; rethrow cancellation as a catch clause - ScanExporter/PdfHelper: log why a gallery save rolled back / narrow the PDF-viewer catch - RetryUtils/OpenLibraryEngine/KeyVerifier/ProviderExecutor: smaller hygiene fixes Regenerated detekt-baseline.xml: 207 -> 191 entries, deletions only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Summary
PR #10 baselined 258 detekt findings to get the Quality Gate green. Buried in the exception-handling findings were three genuine bugs — one of which destroys user data. This PR fixes them plus the surrounding swallowed-exception findings, and shrinks the baseline accordingly (207 → 191, deletions only).
1. History data-loss chain (severe)
HistoryManager.saveHistory()did a non-atomic truncate-then-write; a process kill mid-write leftscan_history.jsonunparseable,getHistory()swallowed the error and returnedemptyList(), and the nextsaveItem()wrote that empty list back — permanently destroying all stored scans. Now:scan_history.corrupt.jsoninstead of overwrittenparseHistoryJson/historyToJsonseams with JVM regression tests (truncated JSON →null,[]→ empty list, 50-item round-trip)2. Silent API-key loss
KeyCipher.decrypt()returnednullon any exception. When Android invalidates the Keystore key (e.g. lock-screen change), saved BYOK keys silently became "not configured". Catches are narrowed, and the failure class (never the payload) is logged, withKeyPermanentlyInvalidatedExceptioncalled out explicitly.3. Broken coroutine cancellation
AiScanViewModelandOcrViewModelcaughtExceptionaround coroutine bodies, swallowingCancellationException. Both now rethrow it first, per the existingKeyVerifier/ProviderExecutoridiom.Hygiene (behaviour unchanged)
ActionExecutor(9 sites narrowed + logged alongside existing toasts),ScanExportergallery-save rollback logged,PdfHelpernarrowed,ProviderClientchains theSerializationExceptioncause intoTransientAiException, plus smaller cleanups inRetryUtils,OpenLibraryEngine,KeyVerifier,ProviderExecutor.Out of scope (stay baselined): complexity and MagicNumber findings.
Verification
./gradlew testDebugUnitTestgreen, including the newHistoryManagerJsonTest./gradlew lintgreenprintStackTrace+ magic-number probe still fails with exit 2scan_history.corrupt.json; a subsequent real gallery-image OCR scan wrote a freshscan_history.jsonwithout touching the corrupt file🤖 Generated with Claude Code