fix(embedding): fail closed instead of wiping a corrupt RAG index

[kojiwakayama]

Ported from codex/module-reconcile-20260723 as a focused data-integrity fix.

The bugs

A corrupt or oversized index.json—malformed JSON, invalid UTF-8, structurally invalid entries, duplicate identities, or broken chunk relationships—could be treated as an empty or mutable store and later overwritten. Operational read failures could also become ordinary absence. Atomic replacement failures could fall back to rewriting the live index, and concurrent store instances or processes could both report success while silently losing one update.

The original stale-lock recovery also observed only the owner-selected lease. A foreign or replacement lease could therefore be missed, and owner equality alone did not bind cleanup to the exact lease generation observed before the lock directory moved.

The fix

The local JSON store now fails closed and preserves persisted bytes:

• malformed JSON, invalid UTF-8, oversized snapshots, and invalid aggregate document/chunk relationships raise RAG_STORE_CORRUPT, including when the live index changes to invalid UTF-8 or oversize during save;
• operational and lease-gap failures raise path-free RAG_STORE_UNAVAILABLE, and only canonical runtime not-found errors mean absence;
• reads use the native exact no-follow snapshot capability with a 64 MiB admission budget and fatal UTF-8 before JSON parsing;
• exact raw source bytes provide a byte-for-byte stale-write comparison before publication;
• one path-scoped in-process queue and an adjacent cross-process lease serialize cooperating writers;
• every lock observation enumerates and validates the complete bounded entry set, including every token-specific lease, rather than trusting owner metadata to select one;
• the newest lease mtime governs staleness; any present lease with unavailable mtime makes the whole generation non-recoverable;
• stale recovery compares the exact owner text, owner location, directory generation, entry names, lease tokens, and lease mtimes across observed, current, and atomically moved states;
• active ownership requires the owner's exact token-specific lease and rejects extra or replacement leases and foreign temporary files;
• cleanup first claims owner metadata into a generation-specific marker, re-observes the exact fenced set, and removes only that set non-recursively;
• deterministic add, remove, and replace races restore the moved lock instead of deleting an unobserved generation;
• publication temps live inside the owned lease directory so a fenced writer cannot publish through a check-to-rename gap;
• configured storage paths must be regular files or absent, and writes use native atomic rename with no direct-write or last-writer-wins fallback;
• Node publishes Windows snapshot authority only with its bigint identity/generation contract; Deno and Bun on Windows now receive an explicit unsupported local-json diagnosis with Node.js and veryfront-cloud alternatives.

The public RAG guide and generated embedding/error references document the local persistence constraints and current source links. No third-party core dependency was added.

Verification

On exact head 16ce5f3ebe234dc52a287d7ab2d66f44d8bf8679, based on origin/main at bf06bf85a58e44184baa91b22cc16e5c6a91a179:

• full RAG, lock-generation, and runtime-support suites: 3 suites, 43 steps passed;
• Node-compatible, Deno, Node, Bun, SecureFs, and error-registry suites: 6 suites, 169 steps passed;
• deterministic lease add/remove/replace, null-mtime, and exact-owned-lease cases passed 20 consecutive stress runs (100 adversarial steps total);
• save-time invalid UTF-8 and oversized replacement tests prove corrupt classification and byte preservation;
• changed production files and tests pass deno check;
• test-typecheck baseline holds at 57 grandfathered files with 0 new failures;
• deno task verify:quick passes, including formatting, lint, core-dependency, dependency-boundary, module-boundary, extension-contract, generated-reference, public-doc, link, and production typecheck gates;
• deno task lint:core-deps reports no disallowed third-party imports;
• no dependency manifest or lockfile changed.

Fresh exact-head GitHub CI and independent exact-head re-review are required before merge.

Summary by CodeRabbit

• Bug Fixes

  • Improved local JSON RAG storage reliability with safer locking, atomic updates, corruption detection, validation, and concurrent-write protection.
  • Added clearer errors and recovery guidance for corrupted or unavailable RAG storage.
  • Improved filesystem safety and platform-aware behavior, including Windows support limitations.

• Documentation

  • Refreshed API reference links and expanded coverage of available exports.
  • Added guidance for local JSON storage requirements and supported runtimes.

• Tests

  • Added extensive coverage for locking, recovery, filesystem safety, platform behavior, and RAG-store integrity.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

This PR changes the local JSON RAG store load behavior to fail closed (throw a typed registry error) instead of silently resetting and overwriting a corrupted index.json, preventing data loss and surfacing corruption to callers.

Changes:

• Added a new registry error RAG_STORE_CORRUPT and registered it under the SERVER category.
• Updated rag-store load logic to throw on malformed JSON or failed shape validation instead of resetting to an empty store.
• Updated RAG store tests and error-registry counts to reflect the new behavior and new registered error.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/errors/index.ts	Re-exports the new RAG_STORE_CORRUPT registry error.
src/errors/error-registry/server.ts	Defines and registers the new rag-store-corrupt SERVER error.
src/errors/error-registry.test.ts	Updates the registry total and SERVER category count assertions.
src/embedding/rag-store.ts	Changes store loading to throw on corruption instead of wiping; adds helper to create the new error.
src/embedding/rag-store.test.ts	Replaces “reset to empty” expectations with “fail closed and do not overwrite” assertions.

Suppressed comments (1)

src/embedding/rag-store.ts:389

• load() rethrows non-NotFound filesystem errors, so permission/IO failures will bypass the new RAG_STORE_CORRUPT registry error (and may get wrapped later as unknown-error). This contradicts the stated goal of failing closed with a typed registry error for IO failures as well. Consider wrapping these errors with corruptStoreError(...) so the store is preserved and the caller gets a stable slug.

    try {
      snapshot = await readStoreFileSnapshot();
    } catch (err) {
      // Expected on first run, and when the file disappears between stat and read.
      if (!isNotFoundError(err)) throw err;
      snapshot = null;
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/embedding/rag-store.ts:396

• The comment in this catch block says the NotFound path is "expected on first run", but first-run behavior is handled by getStoreFileMetadata() returning null (no exception). This catch is only reached for race conditions like the file disappearing between stat and readTextFile. Updating the comment will avoid misleading future readers about the control flow.

    } catch (err) {
      // Expected on first run, and when the file disappears between stat and read.
      if (isNotFoundError(err)) {
        snapshot = null;

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/embedding/rag-store.ts:448

• save() switched the temp path from a stable ${storagePath}.tmp to ${storagePath}.tmp.<uuid>. This reduces temp-name collisions across concurrent processes, but it also increases the chance of orphaned temp files accumulating after crashes (each crash can leave a unique *.tmp.* that later saves will not overwrite).

If the intended deployment is single-process (the in-file mutex already serializes writes), consider either returning to a stable temp filename or adding a small cleanup step for stale ${storagePath}.tmp.* files to avoid long-term disk clutter.

    // Atomic write: write to temp file then rename to prevent corruption on crash
    const tmpPath = `${storagePath}.tmp.${crypto.randomUUID()}`;
    await writeTextFile(tmpPath, payload);

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/embedding/rag-store.ts:532

• save() throws RAG_STORE_UNAVAILABLE when the in-memory update fails persisted-data validation. This error’s title/suggestion (“file is unavailable”, “check storage availability and file permissions”) does not match a validation/invariant failure, making it harder to diagnose. Consider using a dedicated invariant/limit error (or INVALID_ARGUMENT for quota/limit overruns) so the title/suggestion align with the failure mode.

    if (!isRagStoreData(data)) {
      throw RAG_STORE_UNAVAILABLE.create({
        detail: "The RAG store update violated persisted-data limits or relationships.",
        context: { storagePath },
      });
    }

[Copilot]

Pull request overview

Copilot reviewed 51 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/embedding/local-json-store-lock.ts:423

• The doc comment for tryRecoverStaleLock() says the post-rename check "restores a newer generation" when ownership changes, but restoreUnexpectedRecovery() never restores anything (it only throws). This makes the comment misleading for future maintainers; either implement restoration or update the comment to describe the current fail-closed behavior.

 * Move a stale lock through one deterministic recovery directory before
 * deleting it. The post-rename owner check restores a newer generation if
 * ownership changed in the observation-to-rename window. Recovery never
 * writes inside the observed directory, because doing so would refresh the
 * only safe fallback timestamp for an ownerless partial acquisition.

[kojiwakayama]

Reconciled this PR with current main without a force push at exact head cc199c5.

The broader RAG lock and snapshot hardening had already reached main through the merged dependent branch. The updated PR therefore keeps the newer main implementation and reduces the effective diff to the still-missing CI race fix:

• treat canonical NotFound while opening or iterating the lock directory as a changed or absent observation, so acquisition retries instead of leaking the raw filesystem error;
• deterministic regressions for disappearance at both readDir phases;
• regression proving a failed lease write removes its exact owner-only generation.

This directly covers the failure in main run 30804720593, job 91657279661. Its failed-job rerun is green; this fix removes the race instead of relying on retry luck.

Verification on the reconciled tree:

• focused RAG suite: 43 steps passed;
• worker-pool location diagnostic: 63 steps passed after placing the worktree under the repository. The earlier temporary worktree made the temporary root legitimately subsume the extension read root;
• pinned Deno docs generator test passed;
• touched-file format, lint, and typecheck passed;
• full pre-push gate passed: 3,722 tests, 26,790 steps, 0 failures;
• git diff --check against origin/main passed.

Fresh exact-head CI and independent review are still required before any merge-confidence comment or queueing.

[Copilot]

Pull request overview

Copilot reviewed 139 out of 141 changed files in this pull request and generated no new comments.

[kojiwakayama]

Merge confidence: 94%.

Reasoning before scheduling:

• Current head is cc199c5824f0814120768424c3709513b9ba73ae; the PR is mergeable and no longer draft.
• Effective diff against current origin/main is narrow: src/embedding/local-json-store-lock.ts and src/embedding/rag-store.test.ts only, 113 insertions and 8 deletions. The broader RAG hardening already landed through main, so this branch is now only the missing lock-directory disappearance race fix plus regressions.
• All exact-head GitHub checks are green: format, lint, typecheck, integration, unit, all 8 coverage shards, coverage gate, RSC browser e2e, binary e2e, npm install smoke, sentry runtime packages, security audit, CLA, CodeQL, and CodeRabbit. Workflow-skipped release/split/build jobs are expected for this PR context.
• Review threads are at 0 unresolved; Copilot's exact-head review generated no new comments.
• Fresh local verification on this exact tree: DENO_TESTING=1 npx --yes deno@2.7.7 test --no-check --allow-all src/embedding/rag-store.test.ts passed with 43 steps, and git diff --check origin/main...HEAD passed.
• The stale CHANGES_REQUESTED review was for obsolete head b614b7db2... and addressed broad-review issues that are either already in main or explicitly covered by the reconciled narrow diff. The previous owner-only-lock suppressed finding is covered by the new removes an owner-only lock when the lease write fails regression.

Residual risk is low and localized to the local JSON RAG lock observation path. The change only converts canonical lock-directory NotFound races during observation into a retry/absent-observation outcome; non-NotFound filesystem errors still propagate.

Dismissing stale changes-requested review for obsolete head b614b7d. Current head cc199c5 has a narrow two-file diff, zero unresolved threads, green exact-head CI, fresh local RAG regression verification, and a posted 94% merge-confidence rationale.

[kojiwakayama]

Merge confidence: 94%.

Reasoning: exact head cc199c5 is clean against current main, has zero unresolved review threads, and GitHub reports no failing or pending required checks. The effective diff is narrow: src/embedding/local-json-store-lock.ts now treats a lock directory that disappears during readDir as a retryable missing-observation case, and src/embedding/rag-store.test.ts adds deterministic coverage for both open-time and iteration-time readDir disappearance plus lease-write cleanup. Local verification on the exact worktree passed with npx --yes deno@2.7.7 test --no-check --allow-all src/embedding/rag-store.test.ts (43 steps, 0 failed) and npx --yes deno@2.7.7 check --config=deno.json src/embedding/local-json-store-lock.ts src/embedding/rag-store.test.ts.

Residual risk: narrow filesystem race behavior across platforms, but the change is scoped to lock observation and the regression now covers the CI failure mode seen on main.