fix(memory-lancedb): validate CLI search limit to prevent NaN propagation by PrincNL · Pull Request #63413 · openclaw/openclaw

PrincNL · 2026-04-08T23:00:39Z

Summary

Problem: The ltm search CLI command uses parseInt(opts.limit) without a radix parameter and without NaN validation. Passing a non-numeric value (e.g. --limit abc) causes NaN to propagate directly into the LanceDB .limit() call, leading to unpredictable query behavior.
Why it matters: Users who mistype or pass invalid --limit values get a confusing LanceDB error instead of a graceful fallback.
What changed: Added radix parameter (10) and a Number.isFinite + positive-integer guard that falls back to the default of 5 (matching the commander default on line 507).
What did NOT change (scope boundary): No other CLI commands or search logic modified. Only the limit parsing for ltm search.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Related fix: use Number.parseInt instead of global parseInt for consistency #63326 (same parseInt → Number.parseInt pattern, but for src/ files — this PR covers extensions/memory-lancedb)
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: parseInt(opts.limit) was used without a radix parameter and without validating the parsed result. JavaScript's parseInt("abc") returns NaN, which then flows unchecked into db.search(vector, NaN, 0.3).
Missing detection / guardrail: No input validation on the CLI option before passing to the database layer.
Contributing context (if known): The established pattern in src/cli/cron-cli/register.cron-simple.ts:72-73 already uses Number.parseInt with radix and Number.isFinite guard — this extension predates that pattern.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: extensions/memory-lancedb/index.test.ts
Scenario the test should lock in: ltm search "query" --limit abc should use the default limit (5) instead of propagating NaN.
Why this is the smallest reliable guardrail: The guard is a 2-line inline pattern; a unit test on the parsing logic would catch any future regression.
If no new test is added, why not: The fix uses an identical, battle-tested pattern from src/cli/cron-cli/register.cron-simple.ts. The commander default ("5") ensures the happy path always provides a valid string; only explicit invalid user input triggers the guard.

User-visible / Behavior Changes

openclaw ltm search "query" --limit <invalid> now silently falls back to the default limit of 5 instead of producing unpredictable LanceDB behavior.

Diagram (if applicable)

Before:
[user: --limit abc] -> parseInt("abc") -> NaN -> db.search(vector, NaN, 0.3) -> ❌ unpredictable

After:
[user: --limit abc] -> Number.parseInt("abc", 10) -> NaN -> !isFinite -> fallback 5 -> db.search(vector, 5, 0.3) -> ✅ correct

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: macOS
Runtime/container: Node.js 22+
Model/provider: N/A (CLI command, no model involved)
Integration/channel (if any): memory-lancedb extension
Relevant config (redacted): Memory plugin with LanceDB backend configured

Steps

Configure memory-lancedb extension
Run openclaw ltm search "test query" --limit abc
Observe behavior

Expected

Search returns up to 5 results (default limit)

Actual (before fix)

NaN propagates to LanceDB .limit() call, causing unpredictable query behavior

Evidence

Trace/log snippets

// Before: parseInt("abc") === NaN
// db.search(vector, NaN, 0.3) — LanceDB receives NaN as limit

// After: Number.parseInt("abc", 10) === NaN
// Number.isFinite(NaN) === false → fallback to 5
// db.search(vector, 5, 0.3) — LanceDB receives valid limit

Human Verification (required)

Verified scenarios: Confirmed Number.parseInt("abc", 10) returns NaN, and guard correctly falls back to 5. Confirmed valid inputs ("1", "10", "100") parse correctly.
Edge cases checked: Empty string, negative numbers ("-1" → fallback), zero ("0" → fallback), floats ("3.5" → truncated to 3, accepted).
What you did not verify: Live LanceDB instance behavior with NaN limit (no local LanceDB setup).

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Risks and Mitigations

Risk: Fallback to 5 when 0 is passed (users cannot request zero results).
- Mitigation: Requesting zero results is not a meaningful use case; commander default is already "5".

AI-assisted: yes

…tion Add radix and NaN guard to the --limit CLI option in the ltm search command. Without validation, passing a non-numeric value like --limit abc causes NaN to propagate directly to the LanceDB .limit() call. Now falls back to the default of 5 when the input is invalid. Follows the established CLI option parsing pattern from src/cli/cron-cli/register.cron-simple.ts.

greptile-apps · 2026-04-08T23:01:46Z

Greptile Summary

This PR adds input validation to the ltm search CLI command in the memory-lancedb extension, guarding against NaN propagation when a non-numeric value is passed to --limit. The fix adds a radix to Number.parseInt and a Number.isFinite && > 0 guard with a silent fallback to 5, matching the established pattern in src/cli/cron-cli/register.cron-simple.ts.

Confidence Score: 5/5

Safe to merge — the fix is correct, minimal, and consistent with the established pattern in the codebase.

All findings are P2 (style suggestion). The validation logic is correct: NaN, zero, and negative values all fall back to 5, matching the commander default, and valid positive integers pass through unchanged.

No files require special attention.

Vulnerabilities

No security concerns identified.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/memory-lancedb/index.ts
Line: 511

Comment:
**Silent fallback may surprise users**

When an invalid `--limit` value is supplied, the fallback to `5` happens with no visible feedback, so a user who runs `ltm search "query" --limit abc` will receive results without any indication that their input was ignored. A brief `console.warn` (matching how other CLI commands surface misconfiguration) would make this easier to diagnose.

```suggestion
            const limit = Number.isFinite(limitRaw) && limitRaw > 0 ? limitRaw : 5;
            if (limit !== limitRaw) {
              console.warn(`Invalid --limit value "${opts.limit}"; using default limit of ${limit}.`);
            }
```

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

_{Reviews (1): Last reviewed commit: "fix(memory-lancedb): validate CLI search..." | Re-trigger Greptile}

greptile-apps · 2026-04-08T23:01:49Z

            const vector = await embeddings.embed(query);
-            const results = await db.search(vector, parseInt(opts.limit), 0.3);
+            const limitRaw = Number.parseInt(opts.limit, 10);
+            const limit = Number.isFinite(limitRaw) && limitRaw > 0 ? limitRaw : 5;


Silent fallback may surprise users

When an invalid --limit value is supplied, the fallback to 5 happens with no visible feedback, so a user who runs ltm search "query" --limit abc will receive results without any indication that their input was ignored. A brief console.warn (matching how other CLI commands surface misconfiguration) would make this easier to diagnose.

Suggested change

const limit = Number.isFinite(limitRaw) && limitRaw > 0 ? limitRaw : 5;

const limit = Number.isFinite(limitRaw) && limitRaw > 0 ? limitRaw : 5;

if (limit !== limitRaw) {

console.warn(`Invalid --limit value "${opts.limit}"; using default limit of ${limit}.`);

}

Prompt To Fix With AI

This is a comment left during a code review. Path: extensions/memory-lancedb/index.ts Line: 511 Comment: **Silent fallback may surprise users** When an invalid `--limit` value is supplied, the fallback to `5` happens with no visible feedback, so a user who runs `ltm search "query" --limit abc` will receive results without any indication that their input was ignored. A brief `console.warn` (matching how other CLI commands surface misconfiguration) would make this easier to diagnose. ```suggestion const limit = Number.isFinite(limitRaw) && limitRaw > 0 ? limitRaw : 5; if (limit !== limitRaw) { console.warn(`Invalid --limit value "${opts.limit}"; using default limit of ${limit}.`); } ``` How can I resolve this? If you propose a fix, please make it concise.

Applied — added console.warn for invalid --limit values in 5edffa2.

Add console.warn when a non-numeric --limit value is silently replaced with the default, so users know their input was ignored.

openclaw-barnacle Bot added extensions: memory-lancedb Extension: memory-lancedb size: XS labels Apr 8, 2026

greptile-apps Bot reviewed Apr 8, 2026

View reviewed changes

PrincNL added 2 commits April 9, 2026 01:03

fix: warn on invalid --limit value per review feedback

5edffa2

Add console.warn when a non-numeric --limit value is silently replaced with the default, so users know their input was ignored.

Merge branch 'main' into fix/memory-lancedb-validate-cli-limit

876955c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(memory-lancedb): validate CLI search limit to prevent NaN propagation#63413

fix(memory-lancedb): validate CLI search limit to prevent NaN propagation#63413
PrincNL wants to merge 3 commits intoopenclaw:mainfrom
PrincNL:fix/memory-lancedb-validate-cli-limit

PrincNL commented Apr 8, 2026

Uh oh!

greptile-apps Bot commented Apr 8, 2026

Vulnerabilities

Uh oh!

greptile-apps Bot Apr 8, 2026

Uh oh!

PrincNL Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

PrincNL commented Apr 8, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual (before fix)

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 8, 2026

Greptile Summary

Confidence Score: 5/5

Vulnerabilities

Uh oh!

greptile-apps Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

PrincNL Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant