Show a failure screen instead of crashing when the store cannot open - #567
Merged
Merged
Conversation
VaultRoot used the factory variant whose default failure handler is fatalError, so a store that failed to open even after recovery crashed the app at launch with no user-facing path. Fall back to an empty in-memory store, skip setup() so auto-backup can never replace a good backup with the empty fallback vault, and render a static failure screen explaining that the unreadable files were archived. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bradleymackey
added a commit
that referenced
this pull request
Sep 18, 2026
## Summary
Revives a branch that sat unmerged since May and rebases it onto current
`main` (it was 55 commits behind). It makes OTP widgets act in place
instead of bouncing through the app:
- **HOTP: tap the code to advance the counter.**
`IncrementAndCopyHOTPCodeIntent` increments, renders the new code,
copies it, and reloads the timeline — without launching the app.
Previously the only path was the `vault://otp/{id}/increment` deep link,
which opened the app to do it (#517).
- **TOTP: tap the code to copy it.** `CopyTOTPCodeIntent`, also without
launching the app.
- **Tap the issuer/account labels to open the item** in the app, via a
new `openItemDetail` deep link. This is how the app stays reachable from
a widget whose code area is now a button.
- The widget no longer stores an HOTP code in its snapshot. The
persisted counter may already be stale, so every family masks the digits
until the user advances it — a stale code was worse than no code.
## Security decisions
**Home screen only.** The interactive buttons are confined to
`systemSmall`. The `accessoryCircular` and `accessoryRectangular`
families render on the Lock Screen, where a button would be reachable on
a locked device, and advancing an HOTP counter cannot be undone. Those
families keep the existing non-interactive deep link.
**Narrow write capability.** #526 deliberately narrowed the widget's
store to read-only so an extension could never trigger the recovery path
and archive or move the shared SQLite store. That protection is
preserved: the store is still opened `.openOnly`. What changed is the
capability type — `WidgetStore = VaultStoreReader &
VaultStoreHOTPIncrementer`, which grants exactly the counter increment
and nothing else. The extension still cannot insert, update, delete,
reorder, or export.
**Eligibility still gates every action.** Both intents route through
`eligibleItem(id:)`, so a locked, hidden, passphrase-only, or
killphrase-bearing item yields no code — and, for HOTP, no counter
advance. There are tests for each of those cases.
## Manifesto
Reviewed against `MANIFESTO.md`. The corollary that actually bites here
is **C8** — this reduces the steps needed to obtain a code, which is
exactly what C8 says to evaluate rather than wave through. The
judgement: the widget already renders a live TOTP code to anyone looking
at the screen, so tapping to copy discloses nothing the screen did not
already show, and the step being removed is an app launch, not an
authentication. Nothing moved out from behind device auth, because
nothing here was ever behind it. The irreversible action (HOTP
increment) is kept off the Lock Screen for that reason.
**C4** — no auth gate is removed; widget eligibility has always derived
from item state, never from authentication. **C5** — the widget shows
one user-chosen item and enumerates nothing. **C2** — missing, deleted,
and newly-ineligible items still resolve to the same `.unavailable`
state; the intents return an empty result in every failure case, so a
tap reveals nothing about why. **C7** — the copy is `.localOnly` with
the concealed-type marker, matching the app's default posture.
**C1/C3/C6/C9/C10** — untouched.
**One gap worth recording:** the app applies the user's
`pasteTimeToLive` to copies, and the widget cannot read it. Settings
live in standard `UserDefaults`, which an extension does not share, and
`PasteTTL` sits in `VaultSettings`, which the widget target does not
depend on. Since `PasteTTL.default` is `nil` (no expiry), the widget
matches the app's *default* behaviour — but a user who has chosen an
expiry will not get it on widget copies. Closing this needs an App Group
settings suite; it is deliberately not in this PR.
## Rebase notes
Three conflicts, all resolved toward main's current architecture:
`VaultMainScene` (the store-failure screen from #567 now wraps the
navigation view), `WidgetVaultLoader` (main's lazy, retry-safe store
handling kept, capability widened), and `OTPWidgetSmallView` (main's
Dynamic Type fonts kept over the branch's fixed sizes). The accessory
views were taken wholesale from main to keep them non-interactive.
The pre-rebase tip is preserved locally as
`backup/hotp-in-widget-pre-rebase` (`1749dad6`).
## Testing
- Full `iOSAllTests` plan passes locally on iPhone 18 Pro Max / iOS
27.0.
- New `WidgetVaultLoaderCodeActionTests` (11 tests) covers both
intent-facing loader paths: the counter advances exactly once and
renders the *next* counter, a TOTP item is rejected by the HOTP path and
vice versa, an unknown id is inert, and locked/killphrase/hidden items
produce no code and no increment.
- `OTPWidgetLoadingTests` updated for the widened store type.
**Not yet verified on device** — the interactive widget path needs a
manual run: place a `systemSmall` widget on the home screen for an HOTP
item, tap the code, and confirm the counter advances once, the code
lands on the clipboard, and the timeline reloads. I have not done this,
and it is the thing most worth checking before merge.
⚠️ Automatic CI is still disabled (#548), so this is local verification
only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Problem
VaultRoot.vaultStoreusedPersistedLocalVaultStoreFactory.makeVaultStore(), whose default failure handler is{ fatalError($0) }. If the on-disk store failed to open — even after the factory's archive-and-recover pass — the app hard-crashed at launch with no user-facing path. (The widget already used the throwing variant correctly.)Fix
PersistedLocalVaultStore.inMemory()public factory: an empty in-memory store used as a safe fallback — keeps the entire static composition graph valid (app, autofill extension, rehash services) while guaranteeing no writes to the broken on-disk store.VaultRoot.vaultStorenow callsmakeVaultStoreOrThrow(); on failure it recordsvaultStoreLoadFailureMessageand returns the in-memory fallback. (fatalErrorremains only for in-memory container creation failing, which has no external failure modes.)VaultMainSceneskipsVaultRoot.setup()when the failure message is set — critical: this prevents the auto-backup wiring from ever backing up the empty fallback vault over a good backup (MANIFESTO C10 blast-radius concern) — and renders the newVaultStoreFailureViewinstead of the vault.VaultStoreFailureViewis deliberately static: explains that the unreadable store files were archived beside the store (the factory already does this), advises relaunch or restore from a backup PDF, shows the error line in a Details section. No retry that could write to the broken store, no destructive "start fresh" action (needs its own design — C6), no diagnostics upload (C3).Deeper
protectedDataWillBecomeUnavailablehandling remains a report-only finding for this release.Tests
VaultStoreFailureViewSnapshotTests— light/dark × 3 type sizes, plus the no-details variant.PersistedLocalVaultStoreFactoryTests(open/recovery/archival behavior) re-run green — the factory itself is unchanged apart from the new in-memory extension.Local verification: targeted suites passed on iPhone 18 Pro Max / iOS 27.0 (snapshots recorded, then clean pass).
🤖 Generated with Claude Code