fix(storage): unify atomic writes for JSON config stores

[chinawch007]

Summary

#4285 (write-side half; this is PR 1 of the two planned in the fix plan on the issue) reports that settings-store and mcp-config-store persist their JSON with a bare writeFile + rename and no fsync, so a crash can leave the renamed file zero-length or partial, and that the three JSON stores had drifted apart in write strictness — settings.json even landed at the umask default (typically 0644) despite carrying plaintext bot app secrets and proxy passwords.

This PR is the write-side first step of #4285. It consolidates the three legacy JSON config stores — settings.json, mcp.json, and credentials.json — on one shared internal writer, writeAtomicFile.

This scope is intentionally limited to those three config stores. marker-file.ts remains separate because it also owns create-via-link publication semantics and a caller-supplied pre-publication hook. runtime-policy/document-io.ts remains separate because it owns bounded document serialization, Runtime Policy-specific error types, stale-temp
recovery, and an explicit commit-outcome contract. writeAtomicFile (packages/storage/src/atomic-file-write.ts):

• exclusive 'wx' temp open, so a pre-planted symlink at the temp path is refused rather than followed;
• a pre-publication file synchronization step: the temporary file is synchronized before the atomic rename;
• a post-publication parent-directory synchronization step on platforms where Node exposes a usable directory-handle sync operation;
• an umask-independent chmod to 0600, so a pre-existing looser file is re-tightened on the next write;
• failure cleanup removes only the temp this call created and has not renamed away, and never lets a cleanup failure mask the original error (a small latent defect in the previous credential-store copy, fixed here).

credential-store's writeSecretFileAtomic becomes a thin wrapper and drops its private syncDirectory/chmodStrict copies; withCredentialFileLock and mcp-config-store's ensureDirectory share the exported hardenDirectory, so directory hardening can no longer drift between stores. One deliberate behavior change: settings.json is now written 0600 (details under Security).

These steps establish the strongest ordering currently available through the Node filesystem APIs used by Maka; they are not a platform-uniform power-loss guarantee.

On Linux, the guarantee remains conditional on the filesystem and storage hardware honoring the file/directory fsync contract. On macOS, Node's FileHandle.sync() maps to ordinary fsync rather than F_FULLFSYNC. On Windows, file contents are synchronized, but syncDirectory() remains a no-op because Node does not expose a usable POSIX-equivalent directory synchronization operation.

Read paths are intentionally unchanged: a corrupt file still rejects, as pinned by the existing tests. The read-side recovery (backup + defaults + notification) is the planned second PR, so this PR uses Refs rather than Fixes.

Refs #4285

Scope and limitations

This PR deliberately covers only the write path of the three legacy JSON config stores: settings, MCP config, and credentials.

It does not:

• replace the marker-file publication authority;
• replace the bounded Runtime Policy document authority;
• implement corrupt-document recovery, which remains the planned read-side
  follow-up for bug(storage): settings.json, atomic replace has no durability fence and a corrupted file is unrecoverable; the atomic-write pattern drifts across stores #4285;
• claim that fault-injection or process-crash tests prove recovery from
  sudden operating-system or device power loss.

The tests verify the writer's operation ordering, cleanup behavior, permission handling, and propagation/classification of injected failures.

Verification

• npm run lint, npm run format:check, npm run typecheck, npm run build — all clean at the repo root.
• npm --workspace @maka/storage run test:dist — 1106 tests, 0 failures (8 skips are win32-only guards); includes the public-entrypoints guards (exports map and the node:sqlite module-graph boundary).
• npx knip --workspace apps/desktop and npx knip --workspace packages/ui — clean.
• New fault-injection tests make the fence load-bearing: removing the post-rename directory fsync fails the dirSync phase test, which also asserts the replaced file is already live with the new bytes when that fence fails (rename is the commit point).
• Not run: the full multi-workspace npm test matrix and desktop E2E — no runtime or UI surface changed.

Security

Deliberate behavior change: settings.json is now created 0600 and re-tightened from the previous umask default (typically 0644) on the next write. The file stores plaintext credentials (bot app secret/token, proxy password), so the loose mode was one of the issue's findings. mcp.json and credentials.json modes are unchanged.

Follow-up (second PR, same issue)

• Read-side recovery for a corrupted settings.json: back up the original bytes, reset to defaults, and notify (desktop notification, skipped under isolated E2E).
• The issue's typed-error expectation for store reads is tracked there as well (or moved to the explicitly-not-done list on the issue, per discussion).

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: ZCode (autonomous coding agent) implemented the write-path unification, its tests, and the commit under the contributor's direction; the human contributor reviewed the work and owns the submission. The commit carries a Generated-by: ZCode trailer, retained through squash.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

The three config stores really do end up on one writer and the credential-store copies are gone, but the authority claim is wider than the package's actual state and two things need a change before merge.

P2: "the single write-side authority for the package's JSON stores" holds for settings/mcp/credentials, and not for packages/storage. marker-file.ts:81 (publishMarkerFile, replace branch) is the same recipe with the same tempCreated flag, the same inner close().catch(() => {}), and the same Partial<Dependencies> injection shape, differing only by chmod and link-vs-rename. runtime-policy/document-io.ts:162 (writeJsonDocument) is the same recipe at the same semantic layer (a bounded JSON document), and it already distinguishes pre-publication failure from post-rename failure. So the package went from roughly four copies to three, not to one. Either build writeAtomicFile on writeJsonDocument (which also settles the commit-outcome point below), or keep the new module and narrow the Summary to the three config stores, saying what prevents folding marker-file and document-io in.

P2: crash consistency is asserted, not demonstrated, and that is worth saying plainly in the Summary rather than fixing with a test. The fault-injection tests pin that both fsyncs are called and that a failure propagates, which is real and useful, but no test crashes a process, and a SIGKILL test would not prove the fence anyway since rename supplies the atomicity on its own. The repo already states the honest position in docs/architecture/managed-dependency-storage-authority-v1.zh-CN.md:97: macOS fsync is not F_FULLFSYNC, Node's Windows fs surface cannot give POSIX-equivalent directory-entry durability, and a green child-process crash test is not power-loss evidence. Aligning the PR wording with that doc costs nothing and stops the next reader from over-trusting the fence.

The rest is confirmed good and does not need re-checking: the fchmod on the handle removes a path race the old credential-store chmod had, the wx symlink test correctly asserts that cleanup touches only the entry this call created, the settings.json 0600 test runs through createSettingsStore and fails on main, and the windows-recovery path swap is generated output that windows-package-source-closure.test.mjs verifies in both directions.

Verification note: I did not build or run the suites locally, so test results rest on the green test and windows_recovery checks. I did run scripts/windows-test-inventory.mjs standalone at this head, which is the evidence for the inline finding on the test file.

[Astro-Han]

P2: these six skip: !isPosix declarations (lines 83, 159, 168, 181, 193, 207) never reach docs/windows-test-inventory.md. excludesWindows() in scripts/windows-test-inventory.mjs:243 matches only the literal process.platform === 'win32' or !== 'darwin' text, so the isPosix alias defined on line 41 hides them from the generator. The guard still passes, which is why CI is green: regenerating the inventory at this head reproduces the committed file byte for byte, with zero atomic-file-write rows and the total still at 77. The new settings-store-onboarding.test.ts case in this same PR uses the inline form and does get inventoried, so the two files disagree on the convention.

Smallest fix: write these six as inline process.platform === 'win32' (keep isPosix for the non-skip branch assertions if you like), then rerun node scripts/windows-test-inventory.mjs --write.

[Astro-Han]

P2: a failure here throws the raw error, so callers cannot tell "nothing was written" from "the rename committed and only the durability fence failed". settings-store.ts update() / updateIf() / upsertOnboardingMilestone() will all reject while settings.json on disk already holds the new value, and the IPC caller reports a failed save for a write that landed. main had no post-rename step in settings-store, so this failure mode is new here. The test on the dirsync phase pins the behavior as expected rather than flagging it.

runtime-policy/document-io.ts:206 in this same package already models it: a published flag, and commitOutcomeUnknown('... reload before retrying') after the rename.

Smallest fix: wrap post-rename failures in a distinguishable error (even just a published: true property), or at minimum document in the module JSDoc that rename is the commit point and a throw does not mean the write was skipped.

[Astro-Han]

P3: dir and dirMode have exactly one caller. credential-store.writeSecretFileAtomic passes dir: 'harden'; mcp-config-store.ts:206 calls the exported hardenDirectory itself, and settings-store.ts:196 does its own mkdir. Dropping both options and writing await hardenDirectory(dirname(path)); await writeAtomicFile(path, contents, { fileMode: 0o600 }) in the credential store is behaviorally identical, removes two concepts from the interface, and makes all three stores agree that the directory is the caller's business.

[Astro-Han]

P3: "a durability fence before AND after the atomic rename" reads as platform-uniform, but the JSDoc only carries a Windows caveat for chmod. On Windows syncDirectory (stable-storage.ts:125) returns on the first line, so there is no post-rename fence at all and only the temp handle.sync() applies. On macOS handle.sync() is fsync(2), not F_FULLFSYNC, which Node does not expose. docs/architecture/managed-dependency-storage-authority-v1.zh-CN.md:97 already spells out this matrix. Smallest fix: add the platform matrix to this JSDoc block, since it is the comment that defines the authority.

[Astro-Han]

P3: this changes the mcp temp from .mcp-<uuid>.tmp (dot-prefixed, hidden) to mcp.json.<uuid>.tmp in the workspace root. The happy path cleans up, but a killed process now leaves a user-visible stray in a directory the user browses, and neither settings-store nor mcp-config-store has a startup sweep like document-io.ts:40's cleanupRuntimePolicyDocumentTemps. Low impact, noting it rather than asking for a change.

[Astro-Han]

P3: this comment states a directory policy that the sibling store overrides on the same directory. createMcpConfigStore resolves to join(workspaceRoot, 'mcp.json'), so mcp-config-store.ts:206's hardenDirectory(dirname(this.path), 0o700) chmods this exact workspace root to 0700 on every mcp write. The contradiction predates this PR, but the new comment freezes a claim that does not hold. Either describe the actual state (directory hardening is owned by the mcp store; settings-store does not re-apply it) or settle it in one place here.

[chinawch007]

Thanks for the detailed review. I addressed the findings in separate follow-up commits, rebased the branch onto the latest main (dbf5afd73), and force-pushed the updated branch with lease protection.

Scope and durability claims

I narrowed the scope to the three legacy JSON config stores covered by this change: settings.json, mcp.json, and credentials.json. The rewritten commit message and the helper JSDoc no longer describe this as the package-wide write authority.

marker-file remains separate because it has a create-via-link publication contract, while the Runtime Policy document writer retains its bounded serialization, typed commit-outcome errors, and stale-temp recovery. I will use this same narrower wording in the PR summary.

I also removed the platform-uniform crash-consistency and power-loss guarantee language. The rewritten commit message and helper JSDoc now describe the actual platform matrix: the temporary file is synchronized before rename; POSIX platforms additionally attempt a parent-directory fsync; Windows has no equivalent directory fence in the current Node API; and macOS receives ordinary fsync, not F_FULLFSYNC. The documentation also states that persistence ultimately depends on the filesystem, mount, device, and hardware honoring those operations. The fault-injection tests verify operation ordering and failure semantics rather than claiming to prove recovery from arbitrary power loss.

Windows test inventory

Fixed in 21212059f. I replaced all six skip: !isPosix declarations with the literal process.platform === 'win32' form recognized by windows-test-inventory.mjs, while retaining isPosix for the non-skip ordering assertion.

I regenerated docs/windows-test-inventory.md; the total is now 83 declarations and npm run windows:inventory passes.

Post-rename synchronization failure

Fixed in 44b2d2ff7, following the pattern in runtime-policy/document-io.ts. The writer now treats rename as the publication point and tracks it with a published flag.

A failure before publication still propagates as the original error. A directory-sync failure after publication is wrapped in AtomicFileWriteCommitUnknownError, which exposes published: true, retains the original failure as cause, and instructs callers to reload before retrying.

The test now verifies both sides of that contract: the new bytes are already visible on disk, while the caller receives the distinguishable commit-outcome-unknown error.

Directory policy ownership

Addressed in d79ceb6e2. I removed dir and dirMode from AtomicFileWriteOptions; directory creation and permission policy are now explicitly caller-owned.

credential-store performs hardenDirectory(dirname(path)) before calling writeAtomicFile, while settings and MCP retain their existing directory policies. This preserves the behavior while reducing the shared helper interface.

Platform-specific JSDoc

Also addressed in d79ceb6e2. The helper JSDoc now includes the Linux/POSIX, macOS, and Windows synchronization matrix and explicitly avoids describing the sequence as a uniform power-loss guarantee.

Hidden temporary files

Also addressed in d79ceb6e2. The shared writer now creates adjacent hidden temporary entries using .<target-basename>.<uuid>.tmp. For MCP config this becomes .mcp.json.<uuid>.tmp, so a process killed before cleanup no longer leaves a visible file in the workspace root.

The fault-injection and pre-planted-symlink tests were updated to cover the hidden naming scheme.

Settings directory-policy comment

Also addressed in d79ceb6e2. The comment now describes the actual ownership boundary: SettingsStore does not apply a workspace-directory permission policy itself, while sibling stores such as MCP config may independently harden the same workspace root. The settings file itself remains written with mode 0600.

Verification

After rebasing onto the latest main, I reran the relevant validation locally:

• @maka/core and @maka/storage builds
• 66 storage tests covering the atomic writer, credentials, MCP config, and settings
• npm run windows:inventory
• 43 Windows package-closure and CI workflow-policy tests

All checks passed.

[Astro-Han]

Re-reviewed at d79ceb6. All eight points from the last round are closed in code, not in prose: the authority claim is narrowed to the three legacy JSON config stores with the reasons marker-file and document-io stay separate; the power-loss wording is replaced by the platform matrix; the six skip: !isPosix are literal win32 checks and the inventory arithmetic (76 → 83, portable-candidate 18 → 25) matches; published plus AtomicFileWriteCommitUnknownError separates "nothing written" from "rename landed, then failed"; dir / dirMode are gone and credential-store hardens its own directory; temp names are hidden everywhere; the settings comment describes ownership instead of policy. The settings 0600 test goes through createSettingsStore().get() and fails on main, so it is a real regression line. The windows-recovery.yml change is the closure test doing its job: credential-store.ts loses its win32 literals and atomic-file-write.ts gains them, one in, one out. CI green on both lanes.

No P0–P2. Three small ones, optional:

• fileMode has one value. The default is 0o600 and all three callers pass { fileMode: 0o600 }, while the module's own JSDoc says chmod policy is an invariant, not a per-call choice. Delete the options bag and hard-code it, same ablation you did for dir / dirMode.
• hardenDirectory lives in a module whose JSDoc says directory policy belongs to callers. stable-storage.ts already houses syncDirectory, which this PR imports from there; move it next to that.
• The body's list of deliberately unfolded copies misses one: encrypted-file-managed-secret-store.ts:629-658 is the same recipe step for step (wx 0600 → write → sync → rename → published → syncDirectory, even the "commit outcome is unknown; reload before retrying" wording), and :666-667 is hardenDirectory by hand. Either add it to the list with its constraint (bounded bytes + ManagedSecretError classification, like document-io), or at least point :666 at hardenDirectory. Not asking for the write path to fold here.

One deliberate behavior change to confirm before merge: settings.json tightens from 0644 to 0600 on POSIX. That only matters if some process under a different uid reads it (external CLI, service account, shared container mount); credentials.json in the same directory is already 0600, so the risk is small, but it is the one line that can make an existing deployment stop reading a file.

Evidence boundary: static read at d79ceb6 against main; test results are the green lanes, not a local run.

AI-assisted review: drafted with Maka; I verified each prior point against the current diff, the win32 closure delta and the fourth copy myself.