Skip to content

fix(memory): improve error message when node:sqlite is unavailable#69199

Closed
rrrrrredy wants to merge 2 commits into
openclaw:mainfrom
rrrrrredy:fix/memory-search-sqlite-error-clean
Closed

fix(memory): improve error message when node:sqlite is unavailable#69199
rrrrrredy wants to merge 2 commits into
openclaw:mainfrom
rrrrrredy:fix/memory-search-sqlite-error-clean

Conversation

@rrrrrredy

@rrrrrredy rrrrrredy commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Keeps the existing PR branch scoped to extensions/memory-core.
  • Classifies node:sqlite / missing built-in sqlite runtime errors in buildMemorySearchUnavailableResult(error, overrides?).
  • Keeps the existing quota and generic branches, and keeps explicit warning / action overrides authoritative.
  • Fixes the latest CI lint finding by using String#includes() for the sqlite-missing checks.

Behavior

When the runtime cannot load node:sqlite, memory_search now returns the same unavailable result shape with a targeted warning/action instead of the generic embedding-provider message.

The action matches current OpenClaw support policy: Node.js >=22.19.0, with Node.js 24 recommended. The doc link uses the valid Memory CLI page: https://docs.openclaw.ai/cli/memory.

Real Behavior Proof

  • Behavior or issue addressed: memory_search unavailable payloads now classify missing node:sqlite runtime errors with targeted warning/action text while preserving quota, generic, and explicit override behavior.
  • Real environment tested: Windows PowerShell in the OpenClaw repo; old-runtime missing-builtin reproduction plus Node.js v24.13.0 for helper output, tests, and lint.
  • Exact steps or command run after this patch: node -e "try { require('node:sqlite'); console.log('node:sqlite available'); } catch (error) { console.log(String(error.message)); }"; node --import tsx -e "import { buildMemorySearchUnavailableResult } from './extensions/memory-core/src/tools.shared.ts'; ..."; node scripts/test-projects.mjs extensions/memory-core/src/tools.test.ts; node node_modules/oxlint/bin/oxlint extensions/memory-core/src/tools.shared.ts extensions/memory-core/src/tools.test.ts; node node_modules/oxlint/bin/oxlint --tsconfig config/tsconfig/oxlint.extensions.json extensions/memory-core.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Copied terminal output below shows the missing builtin error, the after-fix unavailable payload, override preservation, and targeted test/lint output.
  • Observed result after fix: A missing node:sqlite error returns disabled: true, unavailable: true, the original error string, and targeted Node.js upgrade guidance; custom warning and action overrides still win; the targeted memory-core tests pass.
  • What was not tested: No known gaps for this helper-level unavailable payload change; a full old-Node live memory index/search session was not run because the focused bug is the formatter branch and the old-runtime reproduction plus helper output exercise that behavior directly.
  • Proof limitations or environment constraints: The old-runtime missing-builtin command demonstrates the source error; the current supported Node.js 24 runtime is used for the branch output and tests.

Old-runtime reproduction of the missing builtin:

$ node -e "try { require('node:sqlite'); console.log('node:sqlite available'); } catch (error) { console.log(String(error.message)); }"
No such built-in module: node:sqlite

Current branch unavailable payload for that error:

$ node --import tsx -e "import { buildMemorySearchUnavailableResult } from './extensions/memory-core/src/tools.shared.ts'; console.log(JSON.stringify(buildMemorySearchUnavailableResult('No such built-in module: node:sqlite'), null, 2));"
{
  "results": [],
  "disabled": true,
  "unavailable": true,
  "error": "No such built-in module: node:sqlite",
  "warning": "Memory search is unavailable because node:sqlite is not available in this Node.js runtime.",
  "action": "Use OpenClaw with Node.js 22.19.0 or newer; Node.js 24 is recommended for memory search. See https://docs.openclaw.ai/cli/memory.",
  "debug": {
    "warning": "Memory search is unavailable because node:sqlite is not available in this Node.js runtime.",
    "action": "Use OpenClaw with Node.js 22.19.0 or newer; Node.js 24 is recommended for memory search. See https://docs.openclaw.ai/cli/memory.",
    "error": "No such built-in module: node:sqlite"
  }
}

Override preservation check:

$ node --import tsx -e "import { buildMemorySearchUnavailableResult } from './extensions/memory-core/src/tools.shared.ts'; console.log(JSON.stringify(buildMemorySearchUnavailableResult('No such built-in module: node:sqlite', { warning: 'custom warning', action: 'custom action' }), null, 2));"
{
  "results": [],
  "disabled": true,
  "unavailable": true,
  "error": "No such built-in module: node:sqlite",
  "warning": "custom warning",
  "action": "custom action",
  "debug": {
    "warning": "custom warning",
    "action": "custom action",
    "error": "No such built-in module: node:sqlite"
  }
}

Targeted test run on Node.js 24.13.0:

$ node scripts/test-projects.mjs extensions/memory-core/src/tools.test.ts
Test Files  1 passed (1)
Tests       19 passed (19)
[test] passed 1 Vitest shard

Lint checks:

$ node node_modules/oxlint/bin/oxlint extensions/memory-core/src/tools.shared.ts extensions/memory-core/src/tools.test.ts
# passed

$ node node_modules/oxlint/bin/oxlint --tsconfig config/tsconfig/oxlint.extensions.json extensions/memory-core
# passed

Note: node scripts/run-bundled-extension-oxlint.mjs and node scripts/run-oxlint-shards.mjs --threads=8 both timed out locally on Windows after 5 minutes with no diagnostics, but the CI failure was the two prefer-includes findings in extensions/memory-core/src/tools.shared.ts; the changed file and memory-core extension path now pass oxlint directly.

Related

@rrrrrredy
rrrrrredy marked this pull request as ready for review April 20, 2026 05:13
@greptile-apps

greptile-apps Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR narrows the error-message logic in buildMemorySearchUnavailableResult to detect node:sqlite unavailability and return a targeted warning/action instead of the generic embedding-provider message. A matching unit test locks in the new payload. The change is minimal, well-scoped, and correctly handles the three failure branches (SQLite missing → quota exhausted → generic).

Confidence Score: 5/5

Safe to merge — only changes error-message branching logic with a corresponding test.

The sole remaining finding is a P2 style inconsistency (raw reason vs normalized string for regex matching); both approaches are functionally equivalent and carry no correctness risk.

No files require special attention.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/memory-core/src/tools.shared.ts
Line: 122-124

Comment:
**Inconsistent string normalization between checks**

`isQuotaError` pipes `reason` through `normalizeLowercaseStringOrEmpty` before testing, but `isSqliteMissing` tests the raw `reason` and relies on the `i` flag instead. Both approaches work, but mixing them is needlessly inconsistent. Consider normalizing `isSqliteMissing` the same way for symmetry, or note the intentional difference with a comment.

```suggestion
  const isQuotaError = /insufficient_quota|quota|429/.test(normalizeLowercaseStringOrEmpty(reason));
  const normalizedReason = normalizeLowercaseStringOrEmpty(reason);
  const isSqliteMissing =
    /node:sqlite|no such built-in module.*sqlite|missing node:sqlite/.test(normalizedReason);
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(memory): improve sqlite unavailable ..." | Re-trigger Greptile

Comment on lines +122 to +124
const isQuotaError = /insufficient_quota|quota|429/.test(normalizeLowercaseStringOrEmpty(reason));
const warning = isQuotaError
? "Memory search is unavailable because the embedding provider quota is exhausted."
: "Memory search is unavailable due to an embedding/provider error.";
const action = isQuotaError
? "Top up or switch embedding provider, then retry memory_search."
: "Check embedding provider configuration and retry memory_search.";
const isSqliteMissing =
/node:sqlite|no such built-in module.*sqlite|missing node:sqlite/i.test(reason);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inconsistent string normalization between checks

isQuotaError pipes reason through normalizeLowercaseStringOrEmpty before testing, but isSqliteMissing tests the raw reason and relies on the i flag instead. Both approaches work, but mixing them is needlessly inconsistent. Consider normalizing isSqliteMissing the same way for symmetry, or note the intentional difference with a comment.

Suggested change
const isQuotaError = /insufficient_quota|quota|429/.test(normalizeLowercaseStringOrEmpty(reason));
const warning = isQuotaError
? "Memory search is unavailable because the embedding provider quota is exhausted."
: "Memory search is unavailable due to an embedding/provider error.";
const action = isQuotaError
? "Top up or switch embedding provider, then retry memory_search."
: "Check embedding provider configuration and retry memory_search.";
const isSqliteMissing =
/node:sqlite|no such built-in module.*sqlite|missing node:sqlite/i.test(reason);
const isQuotaError = /insufficient_quota|quota|429/.test(normalizeLowercaseStringOrEmpty(reason));
const normalizedReason = normalizeLowercaseStringOrEmpty(reason);
const isSqliteMissing =
/node:sqlite|no such built-in module.*sqlite|missing node:sqlite/.test(normalizedReason);
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/memory-core/src/tools.shared.ts
Line: 122-124

Comment:
**Inconsistent string normalization between checks**

`isQuotaError` pipes `reason` through `normalizeLowercaseStringOrEmpty` before testing, but `isSqliteMissing` tests the raw `reason` and relies on the `i` flag instead. Both approaches work, but mixing them is needlessly inconsistent. Consider normalizing `isSqliteMissing` the same way for symmetry, or note the intentional difference with a comment.

```suggestion
  const isQuotaError = /insufficient_quota|quota|429/.test(normalizeLowercaseStringOrEmpty(reason));
  const normalizedReason = normalizeLowercaseStringOrEmpty(reason);
  const isSqliteMissing =
    /node:sqlite|no such built-in module.*sqlite|missing node:sqlite/.test(normalizedReason);
```

How can I resolve this? If you propose a fix, please make it concise.

@rrrrrredy

Copy link
Copy Markdown
Contributor Author

Status update as of 2026-04-23: this is the active clean respin for #59457, the current checks are green, and the scope is limited to the SQLite-unavailable messaging fix in memory-core. If that narrower scope looks right, I would appreciate maintainer review on this version.

@clawsweeper

clawsweeper Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 22, 2026, 2:08 PM ET / 18:08 UTC.

Summary
The PR adds missing-node:sqlite classification to memory-search unavailable-result formatting and tests the targeted warning/action plus explicit override preservation.

PR surface: Source +14, Tests +42. Total +56 across 2 files.

Reproducibility: yes. Current main source routes missing node:sqlite manager setup failures into the generic unavailable-result helper, and the PR body includes terminal output for the missing-builtin error plus the after-fix payload.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/tools.shared.ts, unknown-data-model-change: extensions/memory-core/src/tools.test.ts, vector/embedding metadata: extensions/memory-core/src/tools.shared.ts, vector/embedding metadata: extensions/memory-core/src/tools.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #69199
Summary: This PR is the canonical active item for the immediate missing-node:sqlite unavailable-payload UX fix; fallback search and provider-error hinting are adjacent work.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Live mergeability is currently unknown, and a maintainer comment says the branch needed an update against current main for Dependency Guard, so landing should refresh the exact head against current main.

Maintainer options:

  1. Decide the mitigation before merge
    Land this narrow unavailable-payload formatter/test change, and keep degraded keyword fallback behavior in [Feature]: Add keyword fallback for memory_search when node:sqlite is unavailable #93150 or fix(memory): fall back to keyword search without sqlite #93260 as the separate capability decision.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair lane is needed because no concrete patch defect was found; maintainers should review/refresh the branch and merge if exact-head checks and mergeability pass.

Security
Cleared: The diff only changes memory-core error-message branching and tests; it does not touch dependencies, workflows, secrets, permissions, package resolution, or code-execution surfaces.

Review details

Best possible solution:

Land this narrow unavailable-payload formatter/test change, and keep degraded keyword fallback behavior in #93150 or #93260 as the separate capability decision.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main source routes missing node:sqlite manager setup failures into the generic unavailable-result helper, and the PR body includes terminal output for the missing-builtin error plus the after-fix payload.

Is this the best way to solve the issue?

Yes. The narrow maintainable fix is to classify known SQLite-runtime messages inside the existing unavailable-result helper while preserving quota, generic, and explicit override behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 25090056dc22.

Label changes

Label justifications:

  • P2: This fixes a real but bounded memory-search UX bug where missing SQLite support points users and agents toward the wrong provider remediation.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides copied live terminal output showing the missing-builtin reproduction, after-fix unavailable payload, override preservation, and targeted test/lint runs, which is sufficient for this helper-level formatter change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides copied live terminal output showing the missing-builtin reproduction, after-fix unavailable payload, override preservation, and targeted test/lint runs, which is sufficient for this helper-level formatter change.
Evidence reviewed

PR surface:

Source +14, Tests +42. Total +56 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 25 11 +14
Tests 1 43 1 +42
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 68 12 +56

What I checked:

Likely related people:

  • steipete: History shows this person introduced and refactored the memory-core helper boundary and memory host SDK path that this PR relies on. (role: feature-history contributor; confidence: high; commits: 3d0050c306ac, d0ce2d104443, bd6c7969ea9c; files: extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.ts, packages/memory-host-sdk/src/host/sqlite.ts)
  • vincentkoc: Recent path history touches the same memory-core helper and release/current-main surface around the unavailable-result behavior. (role: recent area contributor; confidence: medium; commits: c645ec4555c0; files: extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.ts, packages/memory-host-sdk/src/host/sqlite.ts)
  • Takhoffman: Recent memory diagnostics work touched adjacent memory behavior and tests, making this person a useful reviewer for memory-search UX changes. (role: adjacent memory contributor; confidence: medium; commits: c37e49f275fb; files: extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.test.ts)
  • zerone0x: Current blame on the helper and SQLite loader points to a recent broad current-main commit that carried these files, though the behavior predates that commit. (role: recent current-main touch; confidence: low; commits: 3a7cdaf32c87; files: extensions/memory-core/src/tools.shared.ts, packages/memory-host-sdk/src/host/sqlite.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 20, 2026
@RomneyDa

Copy link
Copy Markdown
Member

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 13, 2026
@rrrrrredy
rrrrrredy force-pushed the fix/memory-search-sqlite-error-clean branch from 7492cf1 to c4d7092 Compare June 15, 2026 03:28
@rrrrrredy

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 15, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 15, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 15, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 20, 2026
@vincentkoc

Copy link
Copy Markdown
Member

Thanks @rrrrrredy for the original scoped implementation in #69199.

I am closing this because the replacement PR has now landed: #95916, merged as 19627c7dd9b6f4eb25a896168f53c5528917108e. Clownfish used a replacement path to carry the narrow node:sqlite unavailable guidance fix forward while preserving attribution for your source PR.

If the landed behavior does not match the original intent or a separate reproduction remains, please reply and we can reopen or split that follow-up.

@vincentkoc vincentkoc closed this Jun 23, 2026
@vincentkoc vincentkoc added the clownfish Tracked by Clownfish automation label Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clownfish Tracked by Clownfish automation extensions: memory-core Extension: memory-core P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UX]: error response is not actionable when is missing — wrong root cause diagnosis and no resolution path

3 participants