Don't let a corrupt JSON state file crash server startup (#204)#205
Open
KunjShah95 wants to merge 1 commit into
Open
Don't let a corrupt JSON state file crash server startup (#204)#205KunjShah95 wants to merge 1 commit into
KunjShah95 wants to merge 1 commit into
Conversation
The small file-backed stores (Inbox, inbox routing, mention threads, channel subscriptions, unrouted/dead-letter, unattended flags, per-persona/per-session connection maps, and user risk overrides) each loaded their JSON file with an unguarded json.loads inside __init__ — and every one is constructed eagerly in SessionManager.__init__. So a single truncated or malformed file turned into an exception that aborted server startup entirely, recoverable only by finding and deleting the file by hand. Several of the same stores also wrote their files non-atomically (write_text in place), which is exactly what produces the corrupt file after a crash/kill/disk-full mid-write. This generalizes the resilient pattern already used inline by ChannelBuffer and WorkspaceTrustStore into two shared helpers (coworker/jsonstore.py): - read_json(): missing/corrupt file -> caller default, moving the bad file aside to <name>.corrupt so it is neither lost nor re-hit on the next boot. - write_json_atomic(): temp sibling + os.replace, atomic on POSIX and Windows. Every affected store now uses them. Adds tests covering the helpers and each store's construct-from-corrupt-file path plus a save round-trip. Related to the WakeStore-specific report in andrewyng#158 (that fix, andrewyng#159, covers only selfwake.py); this addresses the same failure mode across the rest of the persistence layer. Co-Authored-By: Claude Opus 4.8 <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.
What
Makes the small JSON-backed stores survive a corrupt/truncated state file instead of crashing server startup, and makes their writes atomic so they stop producing those corrupt files.
Fixes #204. Same failure mode as #158, generalized beyond
selfwake.py(which #159 handles).Why
Every one of these stores loaded its file with an unguarded
json.loadsin__init__, and all are constructed eagerly inSessionManager.__init__. So one malformed file raised out of the constructor and aborted startup — recoverable only by manually deleting the file. Several also wrote non-atomically (write_textin place), which is exactly what leaves a truncated file after a crash/kill/disk-full mid-write. The two defects compound.How
New
coworker/jsonstore.pywith two helpers, generalizing the pattern already used inline byChannelBufferandWorkspaceTrustStore:read_json(path, default)— missing/corrupt file →default, moving the bad file aside to<name>.corrupt(preserved for inspection, not re-hit on the next boot).write_json_atomic(path, data)— temp sibling +os.replace(atomic on POSIX and Windows).Applied to:
InboxStore,InboxRouting,MentionSessionStore,SubscriptionStore,UnroutedStore,UnattendedRegistry,PersonaConnectionStore,SessionConnectionStore,RiskOverrideStore.Left untouched (already resilient — the precedent this follows):
ChannelBuffer,ParkedStore,WorkspaceTrustStore. Not touched:selfwake.py(handled by #159, to avoid conflict).Tests
tests/test_json_store_resilience.pycovers the helpers (missing/valid/corrupt/atomic/no-temp-leftover) and each store's construct-from-corrupt-file path plus a save round-trip.16 passedlocally, along with the existingtest_inbox_routingsuite.Out of scope
Schema-drift tolerance (a dataclass gaining/losing a field across versions) is a separate concern and not addressed here.
🤖 Generated with Claude Code