zalouser: fix DM/group routing, media send, mention gating, and auto-restart#33992
zalouser: fix DM/group routing, media send, mention gating, and auto-restart#33992darkamenosa wants to merge 14 commits intomainfrom
Conversation
sendZaloTypingEvent now returns after successful send and throws when typing is unsupported. Added error logging on typing start failure in monitor.
- Deduplicate mention IDs via Set in extractMentionIds
- Add fallback to getOwnId when fetchAccountInfo shape changes
- Narrow fetchAccountInfo return type to { profile: User }
- Fail closed when requireMention=true but detection unavailable
- Add test for mention-unavailable fail-closed behavior
Add explicit target parsing with group:/user: prefixes so the bot correctly routes outbound messages to groups vs DMs. Supports aliases (g:, g-, u:, dm:, zlu:, zalouser:) and passes isGroup flag to sendMessageZalouser.
openclaw-barnacle
bot
added
docs
Greptile SummaryThis PR addresses multiple compounding issues in the
Confidence Score: 4/5
Last reviewed commit: 7949003 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| onMessage: (msg) => { | ||
| if (stopped) { | ||
| return; | ||
| } | ||
| logVerbose(core, runtime, `[${account.accountId}] inbound message`); | ||
| statusSink?.({ lastInboundAt: Date.now() }); | ||
| processMessage( | ||
| msg, | ||
| account, | ||
| config, | ||
| core, | ||
| runtime, | ||
| { historyLimit, groupHistories }, | ||
| statusSink, | ||
| ).catch((err) => { | ||
| runtime.error(`[${account.accountId}] Failed to process message: ${String(err)}`); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Race condition in group history state
processMessage is dispatched without await inside the listener callback (line ~791), so two messages arriving close together will execute concurrently. The groupHistories Map is shared across all concurrent invocations.
A concrete scenario: message A (unmentioned) and message B (mentioned) arrive nearly simultaneously.
- Both calls start their async chain (
resolveZaloGroupContext,sendZalouserDeliveryAcks,resolveSenderCommandAuthorization). - If B's awaits complete first, B reaches
buildPendingHistoryContextFromMapwhile A hasn't yet reachedrecordPendingHistoryEntryIfEnabled— so B reads an empty history. - A then records its entry, but B has already dispatched its reply context without including A's message.
This means a user message immediately followed by a mention could silently drop the preceding context entry from the pending history. Consider serialising per-group message processing (e.g. a per-historyKey promise chain or mutex) to guarantee ordering.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/zalouser/src/monitor.ts
Line: 785-802
Comment:
**Race condition in group history state**
`processMessage` is dispatched without `await` inside the listener callback (line ~791), so two messages arriving close together will execute concurrently. The `groupHistories` Map is shared across all concurrent invocations.
A concrete scenario: message A (unmentioned) and message B (mentioned) arrive nearly simultaneously.
1. Both calls start their async chain (`resolveZaloGroupContext`, `sendZalouserDeliveryAcks`, `resolveSenderCommandAuthorization`).
2. If B's awaits complete first, B reaches `buildPendingHistoryContextFromMap` while A hasn't yet reached `recordPendingHistoryEntryIfEnabled` — so B reads an **empty** history.
3. A then records its entry, but B has already dispatched its reply context without including A's message.
This means a user message immediately followed by a mention could silently drop the preceding context entry from the pending history. Consider serialising per-group message processing (e.g. a per-`historyKey` promise chain or mutex) to guarantee ordering.
How can I resolve this? If you propose a fix, please make it concise.| function resolveUploadedVoiceAsset( | ||
| uploaded: Array<{ | ||
| fileType?: string; | ||
| fileUrl?: string; | ||
| fileName?: string; | ||
| }>, | ||
| ): { fileUrl: string; fileName?: string } | undefined { | ||
| for (const item of uploaded) { | ||
| if (!item || typeof item !== "object") { | ||
| continue; | ||
| } | ||
| const fileType = item.fileType?.toLowerCase(); | ||
| const fileUrl = item.fileUrl?.trim(); | ||
| if (!fileUrl) { | ||
| continue; | ||
| } | ||
| if (fileType === "others" || fileType === "video") { | ||
| return { fileUrl, fileName: item.fileName?.trim() || undefined }; | ||
| } | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
resolveUploadedVoiceAsset may silently miss audio attachments
The helper only returns an asset when fileType is "others" or "video". If zca-js ever returns fileType === "audio" or another string for an uploaded audio buffer, the function returns undefined and the call site throws:
Failed to resolve uploaded audio URL for voice message
This would surface as a silent send failure (caught by the outer try/catch and returned as { ok: false }). It would be worth at minimum logging the actual fileType values seen in the uploaded response when the helper returns undefined, so the failure is diagnosable:
if (!voiceAsset) {
const seen = uploaded.map((u) => u.fileType).join(", ");
throw new Error(`Failed to resolve uploaded audio URL for voice message (fileTypes: ${seen})`);
}Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/zalouser/src/zalo-js.ts
Line: 431-452
Comment:
**`resolveUploadedVoiceAsset` may silently miss audio attachments**
The helper only returns an asset when `fileType` is `"others"` or `"video"`. If `zca-js` ever returns `fileType === "audio"` or another string for an uploaded audio buffer, the function returns `undefined` and the call site throws:
```
Failed to resolve uploaded audio URL for voice message
```
This would surface as a silent send failure (caught by the outer `try/catch` and returned as `{ ok: false }`). It would be worth at minimum logging the actual `fileType` values seen in the uploaded response when the helper returns `undefined`, so the failure is diagnosable:
```ts
if (!voiceAsset) {
const seen = uploaded.map((u) => u.fileType).join(", ");
throw new Error(`Failed to resolve uploaded audio URL for voice message (fileTypes: ${seen})`);
}
```
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
The non-null assertion Prompt To Fix With AIThis is a comment left during a code review.
Path: extensions/zalouser/src/channel.ts
Line: 643-650
Comment:
**`lastResult` may be read before assignment**
`lastResult` is declared but never initialised. If `chunks` is empty (theoretically possible if a custom `chunker` returns `[]` for a whitespace-only text that passed the `!text` guard), the loop body never runs and `return lastResult!` returns `undefined`, which will break callers expecting a `ZaloSendResult`.
The non-null assertion `!` silences TypeScript's uninitialized-variable check here. A minimal safeguard:
```suggestion
const outbound = zalouserPlugin.outbound!;
const limit = outbound.textChunkLimit;
const chunks = limit && outbound.chunker ? outbound.chunker(text, limit) : [text];
if (chunks.length === 0) {
return { channel: "zalouser", messageId: "" };
}
let lastResult: Awaited<ReturnType<NonNullable<typeof outbound.sendText>>>;
for (const chunk of chunks) {
lastResult = await outbound.sendText!({ ...ctx, text: chunk });
}
return lastResult!;
```
How can I resolve this? If you propose a fix, please make it concise. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 794900379a
ℹ️ 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".
| const peer = isGroup | ||
| ? { kind: "group" as const, id: chatId } | ||
| : { kind: "group" as const, id: senderId }; | ||
| : { kind: "direct" as const, id: senderId }; |
There was a problem hiding this comment.
Changing DM peers to kind: "direct" makes Zalo DM routing depend on session.dmScope; with the default dmScope="main", resolveAgentRoute collapses all direct chats into the main session key, so different DM senders can share context/history. The previous group peer workaround avoided this, so this change reintroduces cross-user context bleed for any deployment still on main-scope DMs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is for making the behavior align with other plugin.
| const rows = groups.map((group) => | ||
| mapGroup({ | ||
| id: String(group.groupId), | ||
| id: `group:${String(group.groupId)}`, |
There was a problem hiding this comment.
Prefixing directory group IDs as group:<id> here breaks the directory groups members flow, because listGroupMembers forwards groupId unchanged into listZaloGroupMembers, which calls api.getGroupInfo(groupId) with the raw value. Copying an ID from listGroups now passes group:<id> into a path that expects the native Zalo group ID, causing failed or empty member lookups.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fed1f260a7
ℹ️ 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".
extensions/zalouser/src/zalo-js.ts
Outdated
| const info = await api.fetchAccountInfo(); | ||
| const user = | ||
| info && typeof info === "object" && "profile" in info ? (info.profile as User) : (info as User); | ||
| const user = info.profile as User; |
There was a problem hiding this comment.
getZaloUserInfo now assumes fetchAccountInfo() always returns { profile: User }, but this same commit still treats account-info shape as variable in resolveOwnUserId (falling back when profile is missing). If zca-js returns the legacy raw User object, info.profile is undefined and this function returns null, which drops account identity in status/probe/tooling flows despite a valid session. Keep the prior { profile } | User normalization (or share a helper) so account info remains available across response shapes.
Useful? React with 👍 / 👎.
|
Working to resolve these check. Here is a demo of the PR: https://youtu.be/Kuplh9AD3U8 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5db7c8b9
ℹ️ 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".
| const { senderAllowedForCommands, commandAuthorized } = await resolveSenderCommandAuthorization({ | ||
| cfg: config, | ||
| rawBody, | ||
| const configGroupAllowFrom = (account.config.groupAllowFrom ?? []).map((v) => String(v)); |
There was a problem hiding this comment.
groupAllowFrom is consumed as raw strings here, but the startup normalization path in monitorZalouserProvider only resolves allowFrom names to numeric friend IDs. When operators configure groupAllowFrom with display names (which the new docs/config shape allow), resolveDmGroupAccessWithLists ends up checking numeric senderId values against unresolved names and can block all group messages under groupPolicy="allowlist". groupAllowFrom should go through the same name→ID resolution path as allowFrom before access checks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 474bc798b9
ℹ️ 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".
| if (stopped || params.abortSignal.aborted) { | ||
| return; | ||
| } | ||
| cleanup(); |
There was a problem hiding this comment.
If api.listener emits error/closed immediately during startup, failListener now calls cleanup() before the listener is recorded in activeListeners; later in the same startup path, activeListeners.set(...) still executes, but subsequent cleanup() calls are short-circuited by stopped, so that stale entry is never removed. In that state, restart attempts hit the early "already running" guard and the account can no longer recover automatically after the initial startup fault.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This review is not correct for the current zca-js behavior
|
awesome 🚀 |
|
@shakkernerd If you have time, pls help me to have a look on this to see if I'm on right direction. So I can move on. |
Co-authored-by: Tom <[email protected]>
|
Landed manually on top of current Landed commit: Source PR head: What shipped:
Verified before landing:
Thanks @darkamenosa. |
Co-authored-by: Tom <[email protected]>
Co-authored-by: Tom <[email protected]>
Co-authored-by: Tom <[email protected]>
Co-authored-by: Tom <[email protected]>
Co-authored-by: Tom <[email protected]>
Co-authored-by: Tom <[email protected]>
Co-authored-by: Tom <[email protected]> (cherry picked from commit fcdc1a1)
… CI fixes (#1794) * fix: honor explicit Synology Chat rate-limit env values Landed from contributor PR openclaw#39197 by @scoootscooob. Co-authored-by: scoootscooob <[email protected]> (cherry picked from commit af9d76b) * fix: honor zero-valued voice-call STT settings Landed from contributor PR openclaw#39196 by @scoootscooob. Co-authored-by: scoootscooob <[email protected]> (cherry picked from commit 28b72e5) * fix: honor explicit OpenAI TTS speed values Landed from contributor PR openclaw#39318 by @ql-wade. Co-authored-by: ql-wade <[email protected]> (cherry picked from commit 442f2c3) * Voice Call: allowlist realtime STT api key fixtures (cherry picked from commit b8b6569) * Voice Call: read TTS internals in tests (cherry picked from commit b1f7cf4) * Voice Call: read realtime STT internals in tests (cherry picked from commit 244aabb) * test: fix gate regressions (cherry picked from commit 56cd008) * refactor: normalize voice-call runtime defaults (cherry picked from commit 3087893) * refactor: preserve explicit mock voice-call values (cherry picked from commit f6c7ff3) * fix(ci): resolve type regressions on main (cherry picked from commit f721141) * refactor(voice-call): share tts deep merge (cherry picked from commit ed43743) * fix: land openclaw#33992 from @darkamenosa Co-authored-by: Tom <[email protected]> (cherry picked from commit fcdc1a1) * fix(ci): repair zalouser CI failures (cherry picked from commit 06ffef8) * fix: resolve cherry-pick type errors in zalouser extension Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: resolve cherry-pick type error in voice-call config test — adapt SecretRef to string apiKey Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: scoootscooob <[email protected]> Co-authored-by: ql-wade <[email protected]> Co-authored-by: Vincent Koc <[email protected]> Co-authored-by: Tom <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
3 participants
Summary
Tasks
requireMentionin group chat — mention detection was broken, bot responded to all messages regardless of mention gating configuser:<id>andgroup:<id>prefixes for routing; added explicit prefix-based resolver to prevent messages from being sent to the wrong target (DM vs group)historyLimitfor group chat — group chats previously had no message history context, causing the bot to lose memory between messages. New optionalhistoryLimitconfig retains recent messages for contextDemo
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
requireMentioncorrectly gates group messages (bot only responds when mentioned)historyLimitconfig to retain message context across turnsSecurity Impact (required)
Repro + Verification
Environment
Steps
requireMentionenabled andhistoryLimitsetExpected
Actual
Evidence
Human Verification (required)
Compatibility / Migration
historyLimitfield in zalouser configFailure Recovery (if this breaks)
extensions/zalouser/Risks and Mitigations