[Bug][POSIX] storage-json can overwrite a published write after directory fsync failure

[GCSimba]

Current-version update — 2026-10-07

Rechecked on 2026-10-07 against the newest public release, 0.2.1-alpha.1, and upstream master at 5badb15009ae1756c3afe0ae0cef1faafc290ccc. The original patch below applies to the historical layout. A new current-layout port covers both JSON units and domain retirement; the full follow-up records 174 passing Linux owning/consumer tests, focused coverage, application checks and the nonpassing aggregate checks. The current workspace uses single storage; projection cache uses per-record, so the historical whole-unit overwrite symptom is not generalized to that cache.

Current patch index and verification limits. No all-checks-pass, upstream acceptance or released-fix claim is made. This update is AI-assisted with Codex; the published commits retain its co-author.

The original report and its verification history are preserved below.

---

Summary

On master at 141eb6f, a POSIX parent-directory fsync() failure can arrive after rename() has made the new JSON unit file visible. JsonKvUnit then rolls its cached mutation back as though publication never happened. The next ordinary write serializes the whole stale cache and can silently overwrite the already-published value.

I searched the repository's public Discussions, Issues, pull requests, commits, and indexed fork code on 2026-08-21 and did not find an equivalent report or patch. That statement is limited to public indexed material.

Reproduction

The reproducer uses a real temporary directory, real staged-file write and sync, a real rename, and real target-file reads. It forces the POSIX branch and injects a failure only into the parent directory handle's sync() after publication.

Against the unpatched baseline:

{
  "failure": "probe directory fsync failure",
  "memoryAfterFailure": {"k":{"value":"old"}},
  "diskAfterFailure": {"k":{"value":"new"}},
  "diskAfterNextWrite": {"k":{"value":"old"},"later":{"value":true}}
}

The state transition is:

Step	Call result	Unit memory	Visible JSON file
Initial	—	k=old	k=old
put(k, new); rename succeeds; directory sync fails	rejected	rolled back to k=old	k=new
unrelated put(later, true)	resolved	k=old, later=true	k=old, later=true

The first rejection is therefore not contained: it leaves two authoritative candidates, and a later successful call destroys the value already visible on the medium.

Root cause

writeAtomic() does not distinguish failures before and after rename(). A parent-directory sync error is propagated after the visibility point, but JsonKvUnit.putRecord(), deleteRecord(), and setGlobal() unconditionally treat every rejected publication as pre-publication failure and restore the old in-memory value.

The domain remains readable and writable after that rejection. Because storage-json republishes the complete JSON unit on every mutation, the next unrelated write serializes the stale in-memory snapshot and replaces the file containing the new value.

Impact

The shipped web application configures storage-json beneath storage-domain. Domain consumers include the workspace registry and session projection cache. On the rare filesystem-error path, an operation that correctly reports failure can therefore enable a later successful operation to erase a complete value that was already published.

This is a data-consistency bug, not a claim that directory fsync() commonly fails.

Relationship to existing reports

• #1474 establishes that a final directory-sync failure must still be reported after replacement. It does not cover the downstream cache rollback and second-write overwrite after that report.
• #458 covers an atomic-write helper that omitted parent-directory synchronization. storage-json already performs that synchronization; this report concerns its failure semantics after the target has become visible.

Patch

The runtime change is limited to three files:

1. classify an error after target publication while preserving the directory-sync error as the diagnostic cause, even if directory-handle cleanup also fails;
2. keep ordinary pre-publication rollback, but retire a JSON unit after a post-publication failure and reject it with the existing StorageError('closed');
3. terminally retire the owning domain only when an exact KvUnit write rejects with closed: cached reads and new writes reject immediately, queued transforms do not run, the chain drains, the unit closes once, and the facility name is released only after teardown so a fresh open reloads the visible file.

A negative control verifies that caller code throwing StorageError('closed') from an update() transform does not retire a healthy domain.

• Immutable commit: GCSimba@c525459
• Branch: https://github.com/GCSimba/deepseek-harness/tree/codex/fix-storage-json-post-publish-failure

The repository currently does not accept external pull requests, so I am providing the isolated fork commit here.

Verification

Passed locally:

• focused real-file JSON and domain fault-injection suites: 45/45 tests;
• focused coverage for storage-json/src/atomic.ts, storage-json/src/unit.ts, and storage-domain/src/domain.ts: 100% statements, branches, functions, and lines;
• session projection cache consumer suite;
• workspace consumer suite excluding two symlink-creation cases: 41 passed, 2 skipped;
• pnpm run build;
• pnpm run lint;
• pre-commit staged lint, translation pairing, whitespace, and vendor guard;
• pre-push host build plus full host/client typecheck;
• git diff --check.

The Windows test host does not have symlink-creation privilege. Consequently, the two workspace symlink cases fail at fixture setup with EPERM; doc-sync passes 27/28 checks with only its symlink-based documentation-site check blocked, and hygiene reaches the NodeNext consumer fixture before the same EPERM. All remaining hygiene checks were run individually and passed. These are host-permission failures before the affected storage code executes.

Compatibility and limits

The patch does not change public TypeScript types, package-root exports, JSON format, schemas, configuration, dependencies, or wire behavior. It reuses the existing closed error code and requires callers to reopen after the terminal failure.

The defect and fix concern the POSIX directory-sync path. Fault injection proves the application-level visibility, rollback, retirement, and reopen behavior; it cannot prove the outcome of a physical power loss on every filesystem and storage device.

AI-assisted repository audit, reproduction, implementation, and test drafting. I reviewed the source and final diff and ran every command reported above. The commit includes Co-authored-by: Codex <noreply@openai.com>.

[luoyukun3-oss]

Order confirmed on the released line (0.1.7-rc.2) — and there is a platform boundary worth stating
as a code-level fact, because the source already draws it:

@deepseek-ai/dsh-storage-json/lib/index.js

L35:  await rename(tmp, path);
L36:  await fsyncDirectory(dirname(path));
...
L44:  async function fsyncDirectory(path) {
L45:    if (process.platform === "win32") return;

So the rename-then-sync order is exactly as you describe: the failure can arrive after the file is
visible. On Windows the second step is a no-op — fsyncDirectory() returns before doing anything —
which makes the failure path you reproduce unreachable there and reachable only on POSIX. That fits
your [POSIX] tag, and it is worth having in the report as a source fact rather than an inference
from the title.

Practical consequence for the fix: it needs no Windows branch and no change to what
fsyncDirectory() does. The distinction that matters sits inside writeAtomic(), between "failed
before rename" and "failed after rename" — which is where you put it.

[GCSimba]

Current-layout follow-up — 2026-10-07

I rechecked the newest public release, 0.2.1-alpha.1, and upstream master at 5badb15009ae1756c3afe0ae0cef1faafc290ccc. The original c525459 patch remains historical. This is a new port to the current single and per-record JSON units.

• Current immutable commit: 0a3625e84b814afc543cb8397037763079f96f76
• Current fork branch

Root cause and scope

The post-publication failure remains: rename() can expose the complete replacement before parent-directory synchronization rejects. In the single layout, rolling the unit cache back leaves old memory beside new disk state, and a later whole-unit write can silently erase that replacement. In the per-record layout there is no backend cache, but the domain can continue serving stale cached values and run an update transform from them. Per-record legacy initialization can also publish documents during loadAll().

The current workspace domain retains the single layout. Session projection cache now declares per-record storage: the unrelated-record, whole-unit overwrite symptom is not attributed to that cache. Its stale-domain failure is covered separately.

Patch behavior

The port distinguishes a failure before replacement from a directory failure after replacement. A pre-replacement failure retains rollback and retry behavior. A post-replacement failure carries the original synchronization cause, retires the affected JSON unit, drains admitted I/O, and releases the backend name exactly once. Both layouts reject old reads and writes with the existing closed code. A fresh unit reads the actual published medium.

The owning domain retires only when a concrete backend write rejects with StorageError('closed'). Cached reads and new writes reject immediately, queued jobs skip their transforms, and the failed write emits no domain/changed event. The failing and queued operations wait for teardown; successful release frees the facility name for a fresh open. Explicit close shares that teardown. If release itself fails, the name stays reserved and close exposes the release error without replacing the original write failure. An update callback throwing closed does not retire a healthy backend.

The change covers five production modules, owning tests and a POSIX Loader fixture, the affected bilingual documentation, and an Agent Note. Public root exports, public TypeScript interfaces, JSON formats, schemas, configuration and dependencies do not change.

Regression and application evidence

The focused baseline and patched reports contain:

{
  "baselineFocused": { "passed": 0, "failed": 4 },
  "patchedFocused": { "passed": 4, "failed": 0 },
  "finalLinuxOwningAndConsumers": { "passed": 174, "failed": 0, "skipped": 0 },
  "independentPosixLoaderProcesses": { "passed": 2, "failed": 0 }
}

The baseline failures occurred at the intended retirement/reopen assertions, not fixture setup. All five modified production modules reached the repository's configured per-file 100% coverage in the final Linux run.

The final Linux selection used the installed Vitest executable with the current patched checkout's configuration and these filters:

node node_modules/vitest/vitest.mjs run \
  packages/storage/storage-json/tests \
  packages/storage/storage-domain/tests \
  packages/workspace/workspace/tests/workspace.spec.ts \
  packages/session/session-projection-cache/tests/cache.spec.ts \
  --coverage \
  --coverage.include=packages/storage/storage-json/src/atomic.ts \
  --coverage.include=packages/storage/storage-json/src/single-unit.ts \
  --coverage.include=packages/storage/storage-json/src/per-record-unit.ts \
  --coverage.include=packages/storage/storage-json/src/unit-lifetime.ts \
  --coverage.include=packages/storage/storage-domain/src/domain.ts

The independent POSIX application selection was:

node node_modules/vitest/vitest.mjs run --config vitest.e2e.config.ts \
  packages/storage/storage-domain/tests/published-failure.e2e.ts

Both cases boot the real storage plugins through Loader in separate processes; neither required retries. Native Windows owning coverage passed 74 cases; the subsequently added teardown-failure case and its complete owning file passed 4/4. Windows unit tests use the existing platform-override pattern to exercise POSIX classification, not to claim native Windows directory-sync behavior.

A separate smoke imported the actual independently emitted lib/index.js package entries. It checked rejection of stale handles, the complete published file, reopening, and preservation through a later unrelated write:

{"layout":"single","built":true,"failure":"closed","reopened":"new","laterPreserved":true}
{"layout":"per-record","built":true,"failure":"closed","reopened":"new","laterPreserved":true}

It also checked that the internal error and lifecycle helpers are not package-root exports. These are modified-source and freshly built patch results, not execution of an unmodified official npm runtime.

Repository checks and remaining failures

• Full pnpm run build passed.
• Whole-repository pnpm run lint:contracts-ready passed after its required host build had passed. This reuses the existing lint lane without narrowing its source selection or disabling rules.
• Normal pre-commit and pre-push hooks passed; no hook was bypassed.
• Initial push attempts were rejected by the normal hook because installed Vite files were incomplete. Only missing package files were restored from npm archives matching the lockfile SHA-512, with no source or lockfile change; the successful push reran the normal hook.
• Live-base change-scope and git diff --check passed; the committed scope is 26 paths and the worktree is clean. The fetched fork branch SHA matches the immutable commit above.
• Windows pnpm run doc-sync: 41 checks passed, 1 failed at an unchanged EPERM image-symlink fixture. The corresponding seven-file Linux documentation-site selection passed all 151 cases. The Windows aggregate is not presented as passing.
• pnpm run hygiene: 13 checks passed, 3 failed at unchanged upstream vendor-name residues and two Windows symlink-related checks. The NodeNext failure was directly captured at directory-symlink creation; the ACP configuration symlink is checked out as ordinary link-target text. None of these files changes in this port.
• The unchanged NodeNext gate passed on Linux against this branch's actual emitted declarations: all 328 workspace package APIs compile under NodeNext.

Limits

This proves POSIX live-process consistency, not physical power-loss durability. Existing Windows publication policy, per-record deletion/backup synchronization and partial legacy-initialization policy remain unchanged. Consumers holding a retired domain must reopen it or reload/restart the owning plugin; automatic consumer retries are not introduced. No real-model API run, whole-repository coverage run, or exhaustive platform matrix is claimed.

The contribution guide still declines external PRs, so this remains a fork patch for inspection rather than an upstream PR. No upstream adoption or released fix is claimed.

Audit, implementation, validation and this follow-up were AI-assisted with Codex. The current commit includes Co-authored-by: Codex <noreply@openai.com>. Existing reports and immutable commit history are preserved.