fix: strip raw OpenAI function-call wire format from user-facing text#30479
fix: strip raw OpenAI function-call wire format from user-facing text#30479kevinWangSheng wants to merge 25 commits into
Conversation
Add allowMissingTarget option to assertPathSafety for write, mkdirp, and rename operations. This fixes the regression where creating new files fails with 'cannot create directories' error, even when the workspace has rw access. The issue occurs because assertPathSafety was checking for file existence before allowing directory creation, but for new files the parent directory may not have the target file yet. Fixes openclaw#28734
Add allowMissingTarget option to assertPathSafety for write, mkdirp, and rename operations. This fixes the regression where creating new files fails with 'cannot create directories' error, even when the workspace has rw access. The issue occurs because assertPathSafety was checking for file existence before allowing directory creation, but for new files the parent directory may not have the target file yet. Fixes openclaw#28734
Export waitForAbortSignal from the plugin SDK so extensions can import it from 'openclaw/plugin-sdk' instead of using a relative path that breaks in the built project. Fixes openclaw#30057
- Add defaultAccount field to FeishuConfigSchema - Update resolveDefaultFeishuAccountId to check defaultAccount first - Update resolveFeishuAccount to use configured default when accountId is not provided - Exclude defaultAccount from base config during merge to prevent incorrect inheritance Fixes openclaw#30045
The withRemoteHttpResponse function was not passing proxy: "env" to fetchWithSsrFGuard, causing memory search to fail with ECONNREFUSED on servers that require an HTTP proxy for outbound HTTPS access. This fix ensures the Gemini embedding API call respects the system's HTTPS_PROXY and HTTP_PROXY environment variables, consistent with other code paths like Telegram channel and web_fetch tool. Fixes openclaw#30075
This fix enables providers like kimi-coding that require specific headers (e.g., User-Agent) for authentication. The kimi-coding provider already had headers defined in its config, but they were not being passed to the API calls. This change adds a wrapper function that injects the provider's custom headers into the streamFn options for every API request. Fixes openclaw#30099
…tories Docker COPY preserves original permissions from the host filesystem. When building the Docker image, extensions directories may end up with 777 permissions (world-writable), which causes OpenClaw to block all plugins with: 'blocked plugin candidate: world-writable path' This change adds chmod commands after COPY to ensure: - Directories are 755 (rwxr-xr-x) - Files are 644 (rw-r--r--) Fixes openclaw#30139
…(feishu:user:...) Fixes openclaw#30324 - Feishu target normalization fails for provider-prefixed targets like 'feishu:user:ou_xxx'. The normalizeFeishuTarget function now strips the optional 'feishu:' prefix first, then applies existing chat|user|open_id parsing. Also updates looksLikeFeishuId to accept provider-prefixed forms.
…bility Issue openclaw#30467: Control UI blank page on browsers without Array.prototype.toSorted Replace toSorted() with slice().sort() to ensure compatibility with older browsers (e.g., Windows 7 Chrome-based browsers) that don't support the ES2023 toSorted method.
…tibility Fixes openclaw#30467 - Control UI blank page on browsers without Array.prototype.toSorted Changed .slice().toSorted() to .slice().sort() for cross-browser compatibility. The toSorted() method is not available in older Chrome-based browsers (e.g., Windows 7), causing the Control UI to render a blank page with 'TypeError: Array.from(...).toSorted is not a function' error.
…user responses When 'steer' mode injects a user message into a heartbeat run, the agent produces a combined response like 'HEARTBEAT_OK Sure! Here is the answer.' The previous logic stripped HEARTBEAT_OK and then checked if the remaining text was ≤ maxAckChars (300). Since many user responses are under 300 chars, the entire response was treated as a brief heartbeat ack and suppressed. This fix adds a heuristic: after stripping HEARTBEAT_OK, if the remaining text contains multi-sentence structure (sentence-ending punctuation [.!?] followed by more content), it is recognized as a substantive response and NOT suppressed. Fixes openclaw#30449
When logging.file is set to a tilde-prefixed path (e.g. ~/.openclaw/logs/gateway.log), the gateway crashes on startup with 'ENOENT: no such file or directory, mkdir ~/.openclaw/logs'. This fix expands the tilde (~) to the user's home directory before creating the log directory and writing log files. Fixes: Issue openclaw#30401
When models like Codex fail to produce properly structured tool calls, they sometimes emit raw function-call wire format like 'assistant to=functions.xxx' mixed with hallucinated text. This change adds a sanitization function to strip these patterns from text before it's delivered to users. This addresses issue openclaw#30441 where Codex subagent output was leaking raw function-call syntax to Discord channels.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f731b69350
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| * Response behavior when an unpaired (unauthorized) user sends a message. | ||
| * @default "silent" | ||
| */ | ||
| unpairedResponse?: UnpairedResponseMode; |
There was a problem hiding this comment.
Add unpairedResponse to strict config schemas
unpairedResponse is introduced on channel defaults here, but the runtime config validators are still strict and do not accept it (src/config/zod-schema.providers.ts for channels.defaults and src/config/zod-schema.providers-core.ts for Telegram account objects). In practice, users who set channels.defaults.unpairedResponse (or Telegram overrides) will get validation errors, so the new behavior cannot be configured in production.
Useful? React with 👍 / 👎.
| code, | ||
| mode: getUnpairedResponseMode(cfg), | ||
| }) | ||
| : "Pairing already requested. Ask the bot owner to approve your code."; |
There was a problem hiding this comment.
Respect silent mode for repeated DM pairing replies
The created === false branch hardcodes a reply string, which bypasses buildUnpairedResponse and therefore bypasses unpairedResponse: "silent". In silent mode, an unpaired user who retries a Discord component interaction after their first pairing request will still get a DM response, which leaks bot behavior in the mode intended to suppress all unpaired replies.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR primarily adds Key issues found:
Confidence Score: 2/5
Last reviewed commit: f731b69 |
| const responseText = created | ||
| ? buildUnpairedResponse({ | ||
| channel: "discord", | ||
| idLine: `Your Discord user id: ${user.id}`, | ||
| code, | ||
| mode: getUnpairedResponseMode(cfg), | ||
| }) | ||
| : "Pairing already requested. Ask the bot owner to approve your code."; |
There was a problem hiding this comment.
Silent mode still sends "already requested" message
When created === false (the user has already made a pairing request), responseText is unconditionally set to "Pairing already requested. Ask the bot owner to approve your code." — a non-null string. The if (responseText !== null) guard on line 516 therefore never suppresses this branch.
In "silent" mode the intent is to reveal nothing to unauthorized users, but an attacker who sends a second message will receive this reply, leaking that their first pairing request was recorded.
The fix is to also gate the repeat-request message through the mode check:
const responseText = (() => {
if (!created) {
// Only inform the user if the mode is not silent
const mode = getUnpairedResponseMode(cfg);
return mode !== "silent"
? "Pairing already requested. Ask the bot owner to approve your code."
: null;
}
return buildUnpairedResponse({
channel: "discord",
idLine: `Your Discord user id: ${user.id}`,
code,
mode: getUnpairedResponseMode(cfg),
});
})();
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/agent-components.ts
Line: 507-514
Comment:
**Silent mode still sends "already requested" message**
When `created === false` (the user has already made a pairing request), `responseText` is unconditionally set to `"Pairing already requested. Ask the bot owner to approve your code."` — a non-null string. The `if (responseText !== null)` guard on line 516 therefore never suppresses this branch.
In "silent" mode the intent is to reveal nothing to unauthorized users, but an attacker who sends a second message will receive this reply, leaking that their first pairing request was recorded.
The fix is to also gate the repeat-request message through the mode check:
```
const responseText = (() => {
if (!created) {
// Only inform the user if the mode is not silent
const mode = getUnpairedResponseMode(cfg);
return mode !== "silent"
? "Pairing already requested. Ask the bot owner to approve your code."
: null;
}
return buildUnpairedResponse({
channel: "discord",
idLine: `Your Discord user id: ${user.id}`,
code,
mode: getUnpairedResponseMode(cfg),
});
})();
```
How can I resolve this? If you propose a fix, please make it concise.| { | ||
| name: "function call with trailing content", | ||
| text: "assistant to=functions.subagents.commentary some json here and then normal text after", | ||
| expected: "some json here and then normal text after", |
There was a problem hiding this comment.
Duplicate test case name
The name "function call with trailing content" is already used for the test case at line 562. When a test in this loop fails and the case name is printed, there will be two entries with the same label, making it hard to identify which one failed. Consider renaming this one to something more descriptive, e.g. "function call at start with trailing text".
| { | |
| name: "function call with trailing content", | |
| text: "assistant to=functions.subagents.commentary some json here and then normal text after", | |
| expected: "some json here and then normal text after", | |
| { | |
| name: "function call at start with trailing text", | |
| text: "assistant to=functions.subagents.commentary some json here and then normal text after", | |
| expected: "some json here and then normal text after", | |
| }, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-utils.test.ts
Line: 598-601
Comment:
**Duplicate test case name**
The name `"function call with trailing content"` is already used for the test case at line 562. When a test in this loop fails and the case name is printed, there will be two entries with the same label, making it hard to identify which one failed. Consider renaming this one to something more descriptive, e.g. `"function call at start with trailing text"`.
```suggestion
{
name: "function call at start with trailing text",
text: "assistant to=functions.subagents.commentary some json here and then normal text after",
expected: "some json here and then normal text after",
},
```
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
The second Consider limiting the permission change to known non-executable types, or applying Prompt To Fix With AIThis is a comment left during a code review.
Path: Dockerfile
Line: 12-13
Comment:
**`chmod 644` strips executable bit from all files**
The second `find` command applies `644` to every file in the extension and agent directories, removing the executable bit from any shell scripts or compiled binaries that live there. Node.js module files (`.js`, `.ts`) don't need the executable bit, but if any extension ships a helper script or pre-compiled binary (common in `.agent`/`.agents` directories), it will silently fail to execute at runtime.
Consider limiting the permission change to known non-executable types, or applying `chmod 755` to files that already have an executable bit set:
```
# Only strip world-write permissions, leave executable bits intact
RUN find /app/extensions /app/.agent /app/.agents -type d -exec chmod go-w {} \; 2>/dev/null || true && \
find /app/extensions /app/.agent /app/.agents -type f -exec chmod go-w {} \; 2>/dev/null || true
```
How can I resolve this? If you propose a fix, please make it concise. |
|
Closing - issue #30441 has been closed as completed. |
When models like Codex fail to produce properly structured tool calls, they sometimes emit raw function-call wire format like 'assistant to=functions.xxx' mixed with hallucinated text. This change adds a sanitization function to strip these patterns from text before it's delivered to users.
This addresses issue #30441 where Codex subagent output was leaking raw function-call syntax to Discord channels.
Changes
Test plan