fix: quarantine stale sqlite native binding

[RerankerGuo]

Description

Related Issue (Required): Fixes #1343

Handles stale better-sqlite3 native bindings left behind after OpenClaw plugin updates.

Postinstall already detects NODE_MODULE_VERSION mismatches and attempts npm rebuild better-sqlite3, but the stale .node file was left in place. On Windows update paths this can leave the old ABI artifact visible to the next startup/rebuild check. This change quarantines the mismatched native binding before running the rebuild, falling back to removal if the rename fails.

The existing native binding validator remains the source of truth for deciding whether the binding is loadable. Other load errors are not quarantined.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

• Unit Test
• Test Script Or Test Steps (please provide)
• Pipeline Automated API Test (please provide)

Commands run:

node - <<'NODE'
const { quarantineNativeBinding, validateNativeBinding } = require('./apps/memos-local-openclaw/scripts/native-binding.cjs');
const mismatch = validateNativeBinding('/tmp/fake.node', () => { throw new Error('NODE_MODULE_VERSION 127 requires 137'); });
if (mismatch.ok || mismatch.reason !== 'node-module-version') throw new Error('mismatch not detected');
const moved = quarantineNativeBinding('/tmp/better_sqlite3.node', { existsSync: () => true, renameSync() {}, unlinkSync() {} }, 123);
if (!moved.ok || moved.reason !== 'renamed' || !moved.quarantinedPath.includes('abi-mismatch-123')) throw new Error('quarantine rename failed');
const removed = quarantineNativeBinding('/tmp/better_sqlite3.node', { existsSync: () => true, renameSync: () => { throw new Error('EPERM'); }, unlinkSync() {} }, 124);
if (!removed.ok || removed.reason !== 'removed') throw new Error('quarantine unlink fallback failed');
console.log('native binding quarantine checks passed');
NODE

Result: native binding quarantine checks passed.

Also attempted:

cd apps/memos-local-openclaw
npm test -- --run tests/postinstall-native-binding.test.ts
npm run build
make format

npm test is blocked because vitest is not installed. npm run build is blocked because tsc is not installed. make format is blocked because poetry is not installed.

Checklist

• I have performed a self-review of my own code | 我已自行检查了自己的代码
• I have commented my code in hard-to-understand areas | 我已在难以理解的地方对代码进行了注释
• I have added tests that prove my fix is effective or that my feature works | 我已添加测试以证明我的修复有效或功能正常
• I have created related documentation issue/PR in MemOS-Docs (if applicable) | 我已在 MemOS-Docs 中创建了相关的文档 issue/PR（如果适用）
• I have linked the issue to this PR (if applicable) | 我已将 issue 链接到此 PR（如果适用）
• I have mentioned the person who will review this PR | 我已提及将审查此 PR 的人

[RerankerGuo]

This draft is now Ready for review. Cloud test-engine rerun on dev-v2.0.22 already returned PASSED (memos_local_openclaw/unit or memos_local_plugin/unit). Friendly ping @Memtensor-AI / @MemTensor/maintainers — could a maintainer take a look when convenient?

[RerankerGuo]

Hi @Memtensor-AI / @CarltonXiang — friendly ping on this draft.

Quick observation: this PR still has base branch main, but MemTensor/MemOS's current merge flow is into dev-v2.0.22 (every recent PR #2065 / #2058 / #2047 / #2046 / #2042 / #2034 / #2033 / #2026 lands on dev-v2.0.22). The bot already retargeted the base SHA for testing and cloud test-engine returned PASSED against dev-v2.0.22 on every rerun.

Could a maintainer please change the base branch from main → dev-v2.0.22 (Edit → base branch dropdown) so this can enter the normal merge queue? The head commit is a single focused change and does not depend on any main-only commits.

Thanks!

[RerankerGuo]

Hi @syzsunshine219 — looks like you've merged 35 of the last 50 PRs on this repo. Friendly ping on this one.

This draft (and the other 21 OPEN PRs I have here) is small, single-commit, has cloud test-engine PASSED, and is fully mergeable. The only thing blocking it is that the base branch dropdown still says main — every recent merge (e.g. #2038, #2037, #2034, #2032) landed on dev-v2.0.22.

When you have a moment, would you mind one of:

1. Flipping the base branch on this PR from main → dev-v2.0.22 (Edit → base branch dropdown), or
2. Telling me which base branch you'd prefer and I'll re-push accordingly.

Both fix and feature PRs in this batch are independent — each can be reviewed/merged on its own. Happy to rebase / split / drop any that don't fit the direction.

Thanks!

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2049
Task: manual-ocr:MemTensor/MemOS#2049
Base: main
Head: fix/quarantine-stale-sqlite-binding

🔍 OpenCodeReview found 4 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.

---

1. apps/memos-local-openclaw/scripts/native-binding.cjs (L47-L52)

Bug: Wrong error captured in the fallback catch block.

The inner catch {} block returns errorMessage(error) from the outer catch (error) scope (i.e., the rename failure reason), which is a different error than the one thrown by unlinkSync. This is misleading — if unlinkSync also fails, the reason should describe why the unlink failed, not why the prior rename failed.

Suggested fix: capture the unlink error separately.

💡 Suggested Change

Before:

    try {
      fsImpl.unlinkSync(bindingPath);
      return { ok: true, quarantinedPath: "", reason: "removed" };
    } catch {
      return { ok: false, quarantinedPath: "", reason: errorMessage(error) };
    }

After:

    try {
      fsImpl.unlinkSync(bindingPath);
      return { ok: true, quarantinedPath: "", reason: "removed" };
    } catch (unlinkError) {
      return { ok: false, quarantinedPath: "", reason: errorMessage(unlinkError) };
    }

---

2. apps/memos-local-openclaw/scripts/native-binding.cjs (L32)

Correctness: path module is not injected, breaking testability.

path is a top-level module-level import but is used directly (hardcoded) inside quarantineNativeBinding, while fs is correctly injected via the fsImpl parameter. If a caller needs to override or mock path behaviour in tests, there is no way to do so. For symmetry and full testability, consider either accepting path as an injectable parameter as well, or using path consistently as a module-level dependency that callers understand is not injectable. The current mixed approach is inconsistent.

💡 Suggested Change

Before:

function quarantineNativeBinding(bindingPath, fsImpl = fs, now = Date.now()) {

After:

function quarantineNativeBinding(bindingPath, fsImpl = fs, now = Date.now(), pathImpl = path) {

---

3. apps/memos-local-openclaw/scripts/native-binding.cjs (L38-L41)

Correctness: Timestamp collision risk when now is reused within the same millisecond.

now = Date.now() can produce identical values if quarantineNativeBinding is called more than once within the same millisecond (e.g., during concurrent or rapid postinstall re-runs). This leads to a name clash, causing renameSync to silently overwrite a previously quarantined file. Consider appending a random suffix or a monotonically incrementing counter to guarantee uniqueness.

💡 Suggested Change

Before:

  const quarantinedPath = path.join(
    parsed.dir,
    `${parsed.name}.abi-mismatch-${now}${parsed.ext}`,
  );

After:

  const uniqueSuffix = `${now}-${Math.random().toString(36).slice(2, 8)}`;
  const quarantinedPath = path.join(
    parsed.dir,
    `${parsed.name}.abi-mismatch-${uniqueSuffix}${parsed.ext}`,
  );

---

4. apps/memos-local-openclaw/scripts/native-binding.cjs (L50-L52)

Wrong error captured in the inner catch block.

The inner catch {} (for unlinkSync) has no binding, so error still refers to the outer catch (error) scope — the rename failure. If unlinkSync itself fails, reason will incorrectly report the rename error instead of the unlink error, making debugging misleading.

Fix: capture the unlink error with its own variable.

💡 Suggested Change

Before:

    } catch {
      return { ok: false, quarantinedPath: "", reason: errorMessage(error) };
    }

After:

    } catch (unlinkError) {
      return { ok: false, quarantinedPath: "", reason: errorMessage(unlinkError) };
    }

---

🧹 Filtered 3 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 1, duplicate: 2).

Generated by cloud-assistant via Open Code Review.