Conversation
…lently lose state
Hive.writeJson() and persistConfig() both wrote their target file in place
(writeFileSync straight to the real path). A crash, forced-quit, or power
loss during the write window - between truncating the file and finishing
the write - leaves it as truncated/invalid JSON.
The read side doesn't help here: readConfig()'s catch-all silently falls
back to `{ ...DEFAULTS }` on any parse failure, and Hive's readJson() falls
back to whatever empty/default value the caller passed, e.g. `{ godId:
null, agents: {} }` for the registry. Neither surfaces an error anywhere -
the visible symptom is the app quietly booting with an empty agent roster,
or settings/webhooks/missions reset to defaults, with nothing in the logs
pointing at why.
Hive already has an atomicWriteJson() (temp file + renameSync) used for
some writes (setArchived, recordSession) but not others - writeJson()
itself, used by patchAgentRole/setAgentHold/renameAgent/writeTasks and
several others, stayed on the unsafe path. Rather than hunting down each
individual call site, this makes writeJson() delegate to the same
atomicWriteJson() logic, so every writer through it becomes crash-safe at
once. persistConfig() in config.ts gets the identical temp-file+rename
treatment inline, since config.ts doesn't share Hive's helper.
renameSync is atomic on the same volume (true on Windows/macOS/Linux for
same-directory renames), so a concurrent reader only ever observes the
fully-written old file or the fully-written new one - never a partial one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
🚫 This PR is missing its before/after evidenceEvery pull request here has to show its work. Screenshots or a short screen recording, before the change and after it.
How to fix it: edit the description, keep the A bug fix with no visible surface still needs it: show the failing behaviour, then the same steps passing. A terminal recording is fine. Genuinely nothing to show — a CI tweak, a typo, a dependency bump? A maintainer can apply the |
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
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.
Summary
Hive.writeJson()(used bypatchAgentRole,setAgentHold,renameAgent,writeTasks, and others) andpersistConfig()inconfig.tsboth write their target file in place —writeFileSyncstraight to the real path. A crash, forced-quit, or power loss during the write window (between truncating the file and the write finishing) leaves it truncated or invalid JSON.The read side compounds this silently:
readConfig()'s catch-all falls back to{ ...DEFAULTS }on any parse failure.Hive.readJson()falls back to whatever empty/default value the caller passed — e.g.{ godId: null, agents: {} }for the registry.Neither surfaces an error anywhere. The visible symptom is the app quietly booting with an empty agent roster, or settings/webhooks/missions silently reset to defaults, with nothing in the logs pointing at why.
Why this is a real, not theoretical, risk
Hivealready has anatomicWriteJson()(temp file +renameSync) and uses it correctly for some writes (setArchived,recordSession) — but notwriteJson()itself, which is the one most call sites actually use. This app also gets force-killed in totally ordinary usage (closing the app while an agent is mid-write, a Windows update forcing a reboot, etc.) —writeConfig-adjacent calls in particular fire on nearly every settings/webhook/mission IPC handler, so the write window is exercised constantly.Fix
Hive.writeJson()now delegates to the existingatomicWriteJson()logic, so every caller through it (there are several) becomes crash-safe at once, without having to hunt down and individually migrate each call site.persistConfig()gets the identical temp-file+rename treatment inline (config.ts doesn't share Hive's private helper).renameSyncis atomic on the same volume, so a concurrent reader only ever observes the fully-written old file or the fully-written new one — never a partial one.Test plan
npm run typecheckpassesnpm run buildsucceedswriteJsonif there's a preferred harness for that in this repo.🤖 Generated with Claude Code
Before
A terminal repro isolating exactly the two write patterns in question - same file, same kill timing, only the write strategy differs (script attached as
demo-atomic-write.cjsin case it's useful, not part of the diff itself):The target file itself - the one every reader opens - is left truncated and unparseable.
After
Identical kill timing, only the write goes through the temp-file+rename pattern this PR applies everywhere:
The target file a reader would actually open stays valid throughout - untouched, since the kill happened before
renameSynccould run. The partial write lands only in an orphaned temp file that no code path ever reads.