Greptile Summary

This PR adds actionable remediation hints to memory_search tool results and the openclaw memory status --deep CLI output when embedding errors occur (leaked key, quota exceeded, invalid auth). The motivation is solid and the implementation approach is straightforward.\n\nIssues found:\n- P1 — dead code in memory-cli.ts:63: lower.includes(\"invalid.*key\") passes a regex pattern to String.prototype.includes(), which does literal substring matching only. The condition can never match real error messages (e.g. \"invalid api key\") and should be replaced with a regex test or plain substring alternatives.\n- P2 — missing hint in memory-tool.ts catch block: The catch path returns disabled: true with an error but no hint, while the !manager path does. Errors thrown during the actual search call (which can also surface auth/quota messages) won't carry hints for agents.\n- P2 — redundant substring check: lower.includes(\"reported as leaked\") is unreachable since it's always covered by lower.includes(\"leaked\"). Appears in both changed files.

Confidence Score: 4/5

Safe to merge after fixing the includes("invalid.*key") dead-code bug; the rest of the hints work correctly.

The PR is additive and low-risk — it only appends hint/remediation fields to existing error results. The invalid.*key condition is simply dead code rather than a correctness regression (the existing 401/unauthorized checks still cover the most common auth errors). One targeted fix would make this complete.

src/cli/memory-cli.ts line 63 — the includes("invalid.*key") condition needs to be replaced with a regex test or a plain literal substring.

This is a comment left during a code review.
Path: src/cli/memory-cli.ts
Line: 63

Comment:
**Regex pattern used as literal substring**

`String.prototype.includes()` does not interpret regex syntax — it performs a literal substring search. The check `lower.includes("invalid.*key")` will only match if the error string literally contains the characters `invalid.*key` (including a period and asterisk), which will never happen in practice. This entire condition is effectively dead code.

To match error messages like `"invalid api key"` or `"invalid_api_key"`, use a regex test instead:

```suggestion
  if (lower.includes("401") || lower.includes("unauthorized") || /invalid.{0,10}key/.test(lower)) {
```

Or simply add `lower.includes("invalid key")` as a literal check if you prefer to avoid regex entirely.

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

---

This is a comment left during a code review.
Path: src/agents/tools/memory-tool.ts
Line: 87-90

Comment:
**Missing hint in `catch` error path**

The `catch` block returns a `disabled: true` result with an `error` message but no `hint`, while the `!manager` path directly above now includes `resolveMemoryErrorHint`. If the embedding call itself throws (e.g., a network error surfacing a 401/quota message), agents won't get the actionable guidance.

```suggestion
      } catch (err) {
        const message = err instanceof Error ? err.message : String(err);
        return jsonResult({ results: [], disabled: true, error: message, hint: resolveMemoryErrorHint(message) });
      }
```

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

---

This is a comment left during a code review.
Path: src/cli/memory-cli.ts
Line: 41

Comment:
**Redundant substring check**

`"reported as leaked"` always contains `"leaked"`, so the second condition is always covered by the first and can never contribute to a match. The same pattern appears in `resolveMemoryErrorHint` in `memory-tool.ts:234`.

```suggestion
  if (lower.includes("leaked")) {
```

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

_{Reviews (1): Last reviewed commit: "fix: add actionable remediation hints fo..." | Re-trigger Greptile}

Hi maintainers,

This PR is currently behind main by ~13K commits and the target files have been refactored:

Old structure (this PR targets):

• src/agents/tools/memory-tool.ts (deleted)
• src/cli/memory-cli.ts (deleted)

New structure (current main):

• extensions/memory-core/src/cli.ts
• src/agents/memory-search.ts
• Monorepo + packages architecture

The bot-reported issues (P1 regex pattern, P2 redundant check) have been fixed in commit 363b369, but the PR cannot be rebased due to the restructuring.

Options:

1. Should I close this PR and create a new one targeting the new file structure?
2. Or can this be manually merged/adapted by a maintainer who is familiar with the new architecture?

The feature (actionable error hints for leaked/quota-exceeded/invalid API keys) is still useful and the fixes are valid, just need to be applied to the new files.

Let me know how you would like to proceed. Thanks!

Closing this PR as it cannot be rebased due to the monorepo refactoring.

Created a new PR #55684 with the same feature, targeting the current file structure (extensions/memory-core/src/). The new PR:

• ✅ Based on latest main
• ✅ Implements the same remediation hints
• ✅ Addresses the bot-reported issues (no regex pattern misuse, no redundant checks)
• ✅ Mergeable: True

Please review #55684 instead. Thanks!