fix(agents): avoid blank tool names from stream trim#30769
Closed
kevinWangSheng wants to merge 4 commits intoopenclaw:mainfrom
Closed
fix(agents): avoid blank tool names from stream trim#30769kevinWangSheng wants to merge 4 commits intoopenclaw:mainfrom
kevinWangSheng wants to merge 4 commits intoopenclaw:mainfrom
Conversation
added 3 commits
March 1, 2026 05:36
Fixes issue openclaw#30669 - The Edit tool was not expanding ~ (tilde) in file_path/path parameters, causing 'File not found' errors when targeting files like ~/.codex/config.toml. This fix adds tilde expansion in the normalizeToolParams function, which is called before passing parameters to the underlying edit tool.
- Problem: Stream-time tool-call name normalization trimmed whitespace-only names to an empty string, which could propagate as toolName="" and surface repeated "Tool not found" failures in active sessions. - Why it matters: Once empty tool names enter the live run path, tool execution becomes unstable and users can lose tool I/O in channels like Telegram DM mid-session. - What changed: Adjusted stream tool-name trimming to only rewrite when the trimmed name is non-empty, preserving whitespace-only placeholders instead of collapsing them to empty names. Fixes openclaw#30723
🔒 Aisle Security Analysis✅ We scanned this PR and did not find any security vulnerabilities. Aisle supplements but does not replace security review. Analyzed PR: #30769 at commit |
openclaw-barnacle
Bot
added
agents
Contributor
Greptile SummaryThis PR fixes a stream normalization bug where whitespace-only tool-call names were being trimmed to empty strings ( Key changes:
Confidence Score: 4/5
Last reviewed commit: 5b043f6 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
437
to
+443
| if ("file_path" in normalized && !("path" in normalized)) { | ||
| normalized.path = normalized.file_path; | ||
| normalized.path = expandHomeDir(normalized.file_path); | ||
| delete normalized.file_path; | ||
| } | ||
| // Expand ~ in path for read, write, and edit tools | ||
| if ("path" in normalized) { | ||
| normalized.path = expandHomeDir(normalized.path); |
Contributor
There was a problem hiding this comment.
Redundant expandHomeDir call when converting file_path → path
When file_path is present and path is absent, the code at line 438 already calls expandHomeDir on file_path and assigns the result to normalized.path. Then the second block (lines 442–443) runs immediately after and calls expandHomeDir again on the already-expanded value.
expandHomeDir is idempotent so this is not a bug, but the second call is wasted work in this path. You can skip the redundant call with an else:
Suggested change
| if ("file_path" in normalized && !("path" in normalized)) { | |
| normalized.path = normalized.file_path; | |
| normalized.path = expandHomeDir(normalized.file_path); | |
| delete normalized.file_path; | |
| } | |
| // Expand ~ in path for read, write, and edit tools | |
| if ("path" in normalized) { | |
| normalized.path = expandHomeDir(normalized.path); | |
| // file_path → path (read, write, edit) | |
| if ("file_path" in normalized && !("path" in normalized)) { | |
| normalized.path = expandHomeDir(normalized.file_path); | |
| delete normalized.file_path; | |
| } else if ("path" in normalized) { | |
| // Expand ~ in path for read, write, and edit tools | |
| normalized.path = expandHomeDir(normalized.path); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-tools.read.ts
Line: 437-443
Comment:
**Redundant `expandHomeDir` call when converting `file_path` → `path`**
When `file_path` is present and `path` is absent, the code at line 438 already calls `expandHomeDir` on `file_path` and assigns the result to `normalized.path`. Then the second block (lines 442–443) runs immediately after and calls `expandHomeDir` again on the already-expanded value.
`expandHomeDir` is idempotent so this is not a bug, but the second call is wasted work in this path. You can skip the redundant call with an `else`:
```suggestion
// file_path → path (read, write, edit)
if ("file_path" in normalized && !("path" in normalized)) {
normalized.path = expandHomeDir(normalized.file_path);
delete normalized.file_path;
} else if ("path" in normalized) {
// Expand ~ in path for read, write, and edit tools
normalized.path = expandHomeDir(normalized.path);
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+424
to
+426
| if (filePath.startsWith("~/")) { | ||
| return os.homedir() + filePath.slice(1); | ||
| } |
Contributor
There was a problem hiding this comment.
Prefer path.join over string concatenation for cross-platform safety
os.homedir() + filePath.slice(1) produces a mixed-separator path on Windows (e.g., C:\Users\user/rest/of/path). Since node:path is already imported at the top of this file, using path.join is safer and idiomatic:
Suggested change
| if (filePath.startsWith("~/")) { | |
| return os.homedir() + filePath.slice(1); | |
| } | |
| if (filePath.startsWith("~/")) { | |
| return path.join(os.homedir(), filePath.slice(2)); | |
| } |
Note: filePath.slice(2) is correct here because path.join handles the separator itself — slicing from index 2 strips the ~/ prefix entirely (whereas slice(1) keeps the leading /, which is redundant when passed to path.join).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-tools.read.ts
Line: 424-426
Comment:
**Prefer `path.join` over string concatenation for cross-platform safety**
`os.homedir() + filePath.slice(1)` produces a mixed-separator path on Windows (e.g., `C:\Users\user/rest/of/path`). Since `node:path` is already imported at the top of this file, using `path.join` is safer and idiomatic:
```suggestion
if (filePath.startsWith("~/")) {
return path.join(os.homedir(), filePath.slice(2));
}
```
Note: `filePath.slice(2)` is correct here because `path.join` handles the separator itself — slicing from index 2 strips the `~/` prefix entirely (whereas `slice(1)` keeps the leading `/`, which is redundant when passed to `path.join`).
How can I resolve this? If you propose a fix, please make it concise.
…30723-session-io-tools-lost
Contributor
|
Superseded duplicate; closing. Thank you @kevinWangSheng. Why closing:
Notes:
Thanks again for the contribution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
toolName=""and surface repeatedTool not foundfailures in active sessions.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
toolName="") after malformed/partial stream chunks.Security Impact (required)
NoNoNoNoNoRepro + Verification
Environment
Steps
wrapStreamFnTrimToolCallNames).Expected
"".Actual
Evidence
pnpm exec vitest run src/agents/pi-embedded-runner/run/attempt.test.tsHuman Verification (required)
Compatibility / Migration
YesNoNoFailure Recovery (if this breaks)
src/agents/pi-embedded-runner/run/attempt.ts, related test file.Risks and Mitigations