fix(doctor): handle SecretRef objects in memory search apiKey check#35464
fix(doctor): handle SecretRef objects in memory search apiKey check#35464jmzlx wants to merge 2 commits intoopenclaw:mainfrom
Conversation
resolveMemorySearchConfig() may return an unresolved SecretRef object for apiKey (e.g. when configured via env var reference). Calling .trim() on a non-string value throws TypeError. Guard with typeof check before trimming. Fixes openclaw#35444.
Greptile SummaryThis PR fixes a crash in Fix quality: The fix is correct and minimal. Type safety gap: The Confidence Score: 5/5
Last reviewed commit: 666e261 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const rawApiKey = resolved?.remote?.apiKey; | ||
| const hasRemoteApiKey = Boolean(typeof rawApiKey === "string" ? rawApiKey.trim() : rawApiKey); |
There was a problem hiding this comment.
The fix correctly guards against calling .trim() on SecretRef objects at runtime. However, the apiKey field in src/agents/memory-search.ts (line 15) is still typed as apiKey?: string, which doesn't reflect that it can carry an unresolved SecretRef object.
Updating the type to apiKey?: string | SecretRef (or similar) would let TypeScript catch future call sites that unsafely assume apiKey is always a plain string, preventing recurrence of this class of crash elsewhere in the codebase.
This is a pre-existing type annotation gap that this PR works around. Closing it would strengthen type safety for memory search config handling.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/doctor-memory-search.ts
Line: 29-30
Comment:
The fix correctly guards against calling `.trim()` on `SecretRef` objects at runtime. However, the `apiKey` field in `src/agents/memory-search.ts` (line 15) is still typed as `apiKey?: string`, which doesn't reflect that it can carry an unresolved `SecretRef` object.
Updating the type to `apiKey?: string | SecretRef` (or similar) would let TypeScript catch future call sites that unsafely assume `apiKey` is always a plain string, preventing recurrence of this class of crash elsewhere in the codebase.
This is a pre-existing type annotation gap that this PR works around. Closing it would strengthen type safety for memory search config handling.
How can I resolve this? If you propose a fix, please make it concise.…undary The apiKey field in ResolvedMemorySearchConfig can carry an unresolved SecretRef object at runtime, but was typed as string | undefined. - Update type annotation to string | SecretRef in memory-search.ts - Resolve SecretRef → string at the boundary in manager.ts and manager-sync-ops.ts before passing to createEmbeddingProvider - EmbeddingProviderOptions.remote.apiKey stays as string (expects resolved values) Addresses review feedback from greptile-apps on the typeof guard.
|
Addressed in 5157062 — widened TypeScript now catches any future call site that unsafely assumes
|
openclaw-barnacle
bot
added
commands
|
Thanks for the contribution here, and for helping narrow down the SecretRef doctor crash. We merged #36835, which supersedes this PR and fixes the broader doctor/memory SecretRef surface in one place, including the related runtime embedding paths. I’m closing this as superseded by #36835, but I appreciate the fix and the investigation that went into it. |
2 participants
Problem
openclaw doctorcrashes at the end of its run:resolveMemorySearchConfig()may return an unresolved SecretRef object forapiKeywhen configured via environment variable reference. The doctor command does not resolve secrets before calling.trim().Fix
Guard with
typeofcheck before calling.trim():Verified
openclaw doctorcrashes reproducibly atdoctor-memory-search.ts:29Fixes #35444.