Skip to content

Harden account and auth persistence with atomic writes - #14

Closed
thatdaveguy1 wants to merge 1 commit into
Lampese:mainfrom
thatdaveguy1:codex/atomic-storage-writes
Closed

thatdaveguy1 wants to merge 1 commit into
Lampese:mainfrom
thatdaveguy1:codex/atomic-storage-writes

Conversation

@thatdaveguy1

@thatdaveguy1 thatdaveguy1 commented Mar 6, 2026 •

Copy link
Copy Markdown
Contributor

Why

The account database and active auth file are written directly today. If the app crashes mid-write or multiple operations overlap, that can leave accounts.json or auth.json in a partially written state.

This PR hardens those persistence paths with atomic writes, simple locking, and backup files so storage updates are more crash-resistant.

What changed

  • add shared filesystem helpers in fs_utils.rs for locked atomic writes
  • update storage.rs to use the new write path for account persistence
  • update switcher.rs to write active auth data atomically and emit .bak backups
  • ensure lockfile parent directories are created for first-run paths

Reviewer notes

  • this PR is intentionally limited to persistence safety
  • backup security mode selection and process restart behavior are handled in separate PRs

Verification

  • cargo check in src-tauri passes on this branch

Maintenance triage — 2026-09-30

Continue in: #182.

Atomic persistence and cross-process single-writer work is consolidated in #159 / #182. The previous stale lockfile and unused .bak recovery concerns remain documented in this PR's review. #182 is the current implementation to review; neither this closure nor #182 repairs an already-corrupted store.

@Lampese

Lampese commented Mar 7, 2026

Copy link
Copy Markdown
Owner

plz resolve the conflicts

@Lampese Lampese left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Requesting changes for two behavioral issues:

  1. The new .lock file approach can deadlock future writes after a crash. FileLock creates a sibling lock file with create_new(true) and only removes it in Drop. If the process is killed or crashes after acquiring the lock, the stale .lock file remains behind permanently, and every later write path will just time out waiting for it. That means the crash-hardening path introduces a new failure mode exactly in the scenario this PR is trying to improve.

  2. The new .bak files are written, but none of the read paths actually use them for recovery. accounts.json.bak and auth.json.bak are created, but loading still reads only the primary file and fails immediately if it is corrupted or partially unreadable. So the backup mechanism currently increases file churn without providing an actual recovery path.

I think both issues should be addressed before merging, otherwise this change improves write atomicity but still leaves crash recovery incomplete and can also introduce stale-lock write failures.

@Lampese Lampese added bug Something isn't working duplicate This issue or pull request already exists area:storage Account/settings persistence, data recovery, imports and backups. priority:P1 Address first: auth, data integrity, security, startup or release validation. status:superseded Review is consolidated in a linked newer PR; this does not imply a released fix. labels Sep 30, 2026
@Lampese Lampese closed this Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:storage Account/settings persistence, data recovery, imports and backups. bug Something isn't working duplicate This issue or pull request already exists priority:P1 Address first: auth, data integrity, security, startup or release validation. status:superseded Review is consolidated in a linked newer PR; this does not imply a released fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants