Skip to content

fix: tolerate unreadable prior snapshot in export - #12

Merged
tanem merged 5 commits into
mainfrom
fix/export-prior-read-resilience
Jun 10, 2026
Merged

tanem merged 5 commits into
mainfrom
fix/export-prior-read-resilience

Conversation

@tanem

@tanem tanem commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • export reads the existing snapshot before fetching (to carry account data forward and to decide whether to keep the previous file when all accounts fail). It only caught FileNotFoundError, so an existing-but-undecodable prior — most commonly after the encryption passphrase was rotated via set-encryption-passphrase — raised an uncaught ValueError and crashed every subsequent run until the file was manually deleted.
  • Fix (cli.py): catch ValueError alongside FileNotFoundError at the prior-read call site. Log a warning naming the file and cause, treat the prior as absent, and regenerate. snapshot.read() stays strict — resilience lives at the call site, which knows the prior is only an optimisation.
  • An unreadable prior leaves prior_by_login empty, so failed accounts get empty carry-forward and the all-fail "keep previous" guard is bypassed (the undecodable file is overwritten with a readable all-error snapshot).
  • Two new CLI tests; CLAUDE.md gotcha added. No README or schema change.

Test Plan

  • uv run pytest — 86 passed, 100% coverage maintained
  • uv run ruff check src/ tests/ — clean
  • uv run mypy src/mt5_pnl_exporter — clean
  • test_export_regenerates_when_prior_unreadable — garbage prior + succeeding account → exit 0, file regenerated and decryptable, warning emitted
  • test_export_overwrites_unreadable_prior_when_all_fail — garbage prior + failing account → exit 1, readable all-error snapshot replaces the garbage
  • Manual (Windows host): with a snapshot on disk, run set-encryption-passphrase to change the passphrase, then export — now warns and writes a fresh snapshot instead of crashing

Follow-ups (out of scope, noted from review)

  • Tests exercise the decrypt-stage failure (garbage bytes); the catch covers later stages (corrupt-JSON, schema-mismatch) by type (ValidationError ⊆ ValueError). A test variant for those would lock the contract explicitly.
  • Pre-existing: snapshot.read() does path.read_bytes() outside its try, so an OSError/PermissionError race (e.g. a locked file on Windows) still escapes uncaught — separate from this fix.

🤖 Generated with Claude Code

tanem and others added 5 commits June 11, 2026 05:27
Spec for making export tolerate an existing-but-unreadable prior snapshot
(wrong passphrase / corrupt / unsupported schema): warn and regenerate
instead of crashing, by catching ValueError alongside FileNotFoundError
at the prior-read call site.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Task-by-task plan: TDD the except ValueError branch in cli.export, a
regression test for the all-fail overwrite consequence, a CLAUDE.md
gotcha, and full verification (pytest/ruff/mypy/100% coverage).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
read() raises ValueError when an existing snapshot can't be decrypted
(wrong passphrase, corrupt, or unsupported schema). export only caught
FileNotFoundError, so a passphrase change bricked every subsequent run.
Catch ValueError too: warn, treat the prior as absent, and regenerate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When the prior snapshot can't be decoded and every account also fails,
the all-error snapshot replaces the unreadable file rather than the
keep-previous branch firing. Characterisation test for that consequence.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tanem
tanem merged commit 73d7479 into main Jun 10, 2026
1 check passed
@tanem
tanem deleted the fix/export-prior-read-resilience branch June 10, 2026 18:09
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.

1 participant