Skip to content

Stop wiping the index when a watched folder can't be read#10

Merged
GordonBeeming merged 3 commits into
mainfrom
gb/no-wipe-on-lost-access
Jun 9, 2026
Merged

Stop wiping the index when a watched folder can't be read#10
GordonBeeming merged 3 commits into
mainfrom
gb/no-wipe-on-lost-access

Conversation

@GordonBeeming

Copy link
Copy Markdown
Owner

Summary

  • Fixes apparent total data loss on every install. The index file at ~/Library/Application Support/Vista/index.sqlite actually survives — the bug was Indexer.initialScan's reconcile deleting every row whose file wasn't seen that scan. The screenshots live in iCloud Drive (TCC-protected), and a fresh binary can lose Full Disk Access, so the folder enumerated empty and the reconcile wiped all ~6000 rows, triggering a full re-OCR.
  • A row is now deleted only when its folder was genuinely readable, the file wasn't found, and fileExists confirms it's gone (iCloud dataless placeholders report present → preserved). A readable folder that returns nothing while we hold rows for it trips a circuit breaker instead of deleting.
  • When a folder we have rows for can't be read, the menu and panel surface a "grant access" message + a button to the Full Disk Access pane, instead of a misleading "No screenshots indexed yet". Granting access resumes with zero re-OCR.
  • Grid reloads on every panel show so freshly indexed screenshots appear (previously only after the reset timeout).
  • Clearer status copy, dropping the OCR jargon: Reading text from screenshots · X of Y · Z ready, N screenshots ready. The working state shows how many are already searchable so a backlog never reads as empty.

Test plan

  • swift build clean
  • swift test — 45 pass, incl. 6 new IndexerReconcileTests covering the no-wipe guard (inaccessible root, empty-scan circuit breaker, genuinely-deleted file, orphan rows)
  • Manual: with the index populated, revoke Full Disk Access and relaunch → status shows "Can't read your screenshots folder — grant access", panel shows the CTA, sqlite3 index.sqlite "SELECT count(*) FROM screenshots" is unchanged (no wipe). Re-grant → resumes with no re-OCR.
  • Manual: open the panel during indexing, reopen → newly indexed rows appear.
  • Manual: status copy matches the "plain, no jargon" wording across scan / processing / steady states.

Notes

TCC can drop Full Disk Access when a non-sandboxed binary changes — that's macOS and can't be fully suppressed from app code. The no-wipe guard makes it harmless: the index is preserved, so re-granting access resumes instantly with zero re-processing. Follow-up (not here): confirm the release signs with a stable Developer ID / bundle id so TCC persistence is as sticky as macOS allows.

After installing a new build, Vista appeared to lose its whole index and
re-OCR everything. The index file actually survives — the bug was in the
initial-scan reconcile: it deleted every row whose file wasn't seen this
scan. The screenshots live in iCloud Drive, a TCC-protected location, and
a fresh binary can lose Full Disk Access, so the folder enumerated empty
and the reconcile wiped all ~6000 rows.

- Track per-folder scan health. Only delete a row when its folder was
  genuinely readable, the file wasn't found, and fileExists confirms it's
  actually gone (iCloud dataless placeholders still report present, so
  they're preserved). A readable folder that returns nothing while we hold
  rows for it trips a circuit breaker instead of deleting.
- When a folder we have rows for can't be read, emit .accessBlocked. The
  menu and the panel now show a 'grant access' message with a button to
  the Full Disk Access pane, instead of a misleading 'no screenshots
  indexed' empty state. Granting access resumes with zero re-OCR.
- Reload the grid on every panel show so freshly indexed screenshots
  appear, instead of only after the reset timeout.
- Clearer status copy: 'Reading text from screenshots · X of Y · Z ready',
  'N screenshots ready', drop the OCR jargon. The working state now shows
  how many are already searchable so a backlog never reads as empty.
- Pure reconcile helper with unit tests covering the no-wipe guard.

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitButler <gitbutler@gitbutler.com>
@GordonBeeming GordonBeeming marked this pull request as ready for review June 9, 2026 23:24
Copilot AI review requested due to automatic review settings June 9, 2026 23:24

@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: 13b39a44cb

ℹ️ 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 Sources/VistaCore/Indexer.swift Outdated

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a robust mechanism to handle access-blocked folders (such as those protected by TCC or iCloud) to prevent accidental data loss from the index when folders are temporarily unreadable. It adds user-friendly UI prompts in both the menu bar and search panel to guide users to restore access. The review feedback highlights a potential database leak when folders are unwatched, suggests a performance optimization in the deletion reconciliation logic to avoid redundant string allocations, recommends updating the unit tests accordingly, and advises using the modern, thread-safe .formatted() API to prevent Swift 6 concurrency warnings.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread Sources/VistaCore/Indexer.swift
Comment thread Sources/VistaCore/Indexer.swift
Comment thread Tests/VistaCoreTests/IndexerReconcileTests.swift
Comment thread Sources/Vista/MenuBarContentView.swift Outdated

Copilot AI 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.

Pull request overview

This PR hardens Vista’s indexing reconcile logic so an unreadable/empty folder scan (e.g., after losing Full Disk Access or iCloud placeholder behavior) doesn’t trigger mass deletions and expensive re-processing, and updates the UI to surface an explicit “grant access” state instead of implying an empty index.

Changes:

  • Refactors Indexer.initialScan reconcile to delete rows only when a watched root was trustworthy to read and fileExists confirms the file is gone; adds an “empty-scan circuit breaker”.
  • Adds a new .accessBlocked progress state and threads it into menu/panel UI copy + CTA to open Full Disk Access settings.
  • Adds unit coverage for the reconcile decision helper (reconcileDeletions) and refreshes the panel results on show.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Tests/VistaCoreTests/IndexerReconcileTests.swift New tests for the reconcile decision helper and “never wipe on bad scan” guardrails.
Sources/VistaCore/Indexer.swift Implements per-root scan health tracking, guarded deletion reconciliation, and new progress states/fields.
Sources/Vista/PanelController.swift Injects AppState into panel content and reloads results on show to pick up newly indexed rows.
Sources/Vista/PanelContentView.swift Shows an access-blocked empty state with a “Grant Folder Access…” CTA.
Sources/Vista/MenuBarContentView.swift Updates status copy, adds CTA when access is blocked, and formats counts with grouping.
Sources/Vista/AppState.swift Exposes accessBlockedFolders and adapts indexed count derivation to the new progress payload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Vista/MenuBarContentView.swift Outdated
Comment thread Sources/Vista/PanelController.swift
Comment thread Sources/Vista/AppState.swift
Comment thread Sources/VistaCore/Indexer.swift
Comment thread Sources/Vista/PanelController.swift
GordonBeeming and others added 2 commits June 10, 2026 09:32
- Carry access-blocked status on a dedicated stream instead of the progress
  enum. The .accessBlocked progress event was immediately overwritten by the
  .indexing/.watching events that the same scan emits next, so the 'grant
  access' CTA never rendered. It's now sticky AppState fed by accessUpdates,
  cleared only by a later scan.
- Restore cleanup of rows from unwatched folders, which the first cut leaked.
  reconcileDeletions now returns deleteMissing (gone from a readable folder)
  and deleteOrphans (no longer under any watched root) separately; orphans
  are deleted unconditionally since their files still exist on disk. Orphan
  deletion is gated on at least one accessible root so a folder list that
  failed to load can't make every row look orphaned and wipe the index.
- Group known paths by root in a single pass with prefixes computed once, so
  reconcile stays O(known) on large libraries.
- Use Int.formatted() instead of a static NumberFormatter (thread-safe,
  no non-Sendable static).
- Tests updated for the new signature, plus orphan-guard and empty-root-set
  cases.

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitButler <gitbutler@gitbutler.com>
- runQuery now performs store.search in a detached task and applies on the
  main actor, guarded by a generation token so a slower earlier search can't
  land on top of a newer one. reload() runs on every panel show, so the read
  must not block the main thread behind the indexer's writes on the shared
  serial queue (same reasoning as loadMore).
- Correct the PanelController.appState comment: the reference is unowned to
  avoid the AppState <-> PanelController retain cycle, not a strong ref.

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitButler <gitbutler@gitbutler.com>
Copilot AI review requested due to automatic review settings June 9, 2026 23:35
@GordonBeeming GordonBeeming merged commit 88f807c into main Jun 9, 2026
5 checks passed
@GordonBeeming GordonBeeming deleted the gb/no-wipe-on-lost-access branch June 9, 2026 23:37

Copilot AI 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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

// actual work, not fingerprint-check flybys.
let total = toIndex.count
progressContinuation?.yield(.indexing(done: 0, total: total))
progressContinuation?.yield(.indexing(done: 0, total: total, indexed: (try? store.count()) ?? 0))
Comment on lines +443 to +450
// Only act on orphans when the root set is trustworthy enough to trust
// the "not under any root" conclusion. With no accessible root, the
// folder list may simply have failed to resolve — preserve everything.
if roots.contains(where: { $0.accessible }) {
deleteOrphans = orphans
}

return (deleteMissing, deleteOrphans, blockedRoots)
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.

2 participants