fix(feishu): resolve correct accountId for subagent group replies#60634
fix(feishu): resolve correct accountId for subagent group replies#60634EronFan wants to merge 4 commits into
Conversation
… flag Before writing autoLastSuccessVersion, parse afterVersion from the JSON stdout of 'openclaw update --json' and verify it matches the resolved version AND differs from the current running VERSION. This prevents a crash loop where: 1. npm install exits 0 but binary is not actually updated 2. autoLastSuccessVersion is written anyway 3. Gateway restarts, plugin version checks fail due to minGatewayVersion mismatch, all plugins skip loading fix openclaw#58041
Fixes openclaw#58107 by ensuring subagents use their own accountId when sending Feishu group messages. Previously, subagents would use the default accountId ('default') instead of their assigned accountId, causing messages to be sent from the wrong account. The fix prioritizes agentAccountId from the tool execution context (available for subagents) over the channel's accountId.
…kup removal) 1. Remove channel.ts.backup development artefact 2. Fix effectiveAccountId scope error in startAccount: declare local startEffectiveAccountId instead of referencing out-of-scope variable 3. Fix update-startup.ts JSON path: parsed.afterVersion -> parsed.after.version
|
Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit. |
Greptile SummaryThis PR fixes three bugs introduced in an earlier change: subagents in Feishu group chats now use their assigned Confidence Score: 5/5Safe to merge; all three bug fixes are logically correct and the only remaining finding is a P2 style concern. All P0/P1 bugs are resolved. The sole remaining finding — using extensions/feishu/src/channel.ts — the two Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/feishu/src/channel.ts
Line: 621
Comment:
**`any` cast to access `agentAccountId`**
`(ctx as any).agentAccountId` bypasses TypeScript's type system and will silently produce `undefined` if the field name ever changes. The project guideline explicitly avoids `no-explicit-any`. The same pattern is repeated in `startAccount` (line 1139). A narrower typed extension of the action/gateway context (e.g. `interface FeishuCtx { agentAccountId?: string }` combined with a type-guard or a typed intersection) would give the compiler visibility into this path and keep the type safety the rest of the file relies on.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: resolve 3 P1 bugs in #58228 (scope ..." | Re-trigger Greptile |
| describeMessageTool: describeFeishuMessageTool, | ||
| handleAction: async (ctx) => { | ||
| // Use agentAccountId if available (for subagents), otherwise fall back to ctx.accountId | ||
| const effectiveAccountId = (ctx as any).agentAccountId ?? ctx.accountId ?? undefined; |
There was a problem hiding this comment.
any cast to access agentAccountId
(ctx as any).agentAccountId bypasses TypeScript's type system and will silently produce undefined if the field name ever changes. The project guideline explicitly avoids no-explicit-any. The same pattern is repeated in startAccount (line 1139). A narrower typed extension of the action/gateway context (e.g. interface FeishuCtx { agentAccountId?: string } combined with a type-guard or a typed intersection) would give the compiler visibility into this path and keep the type safety the rest of the file relies on.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/channel.ts
Line: 621
Comment:
**`any` cast to access `agentAccountId`**
`(ctx as any).agentAccountId` bypasses TypeScript's type system and will silently produce `undefined` if the field name ever changes. The project guideline explicitly avoids `no-explicit-any`. The same pattern is repeated in `startAccount` (line 1139). A narrower typed extension of the action/gateway context (e.g. `interface FeishuCtx { agentAccountId?: string }` combined with a type-guard or a typed intersection) would give the compiler visibility into this path and keep the type safety the rest of the file relies on.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e9202c861
ℹ️ 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".
|
|
||
| // Parse version from output (format: "openclaw 2026.3.28") | ||
| const versionOutput = versionCheckStep.stdoutTail || ""; | ||
| const versionMatch = versionOutput.match(/openclaw\s+(\d+\.\d+\.\d+)/); |
There was a problem hiding this comment.
Parse version output using the real CLI format
The regex versionOutput.match(/openclaw\s+(\d+\.\d+\.\d+)/) does not match what --version actually prints (OpenClaw <version> with capitalized name, optional commit hash, and possible prerelease suffixes), so versionMatch can be null on healthy builds. In that case runGatewayUpdate exits with version-parse-failed before running doctor, which blocks git-based updates even when the binary and package are aligned.
Useful? React with 👍 / 👎.
| if (!trimmed.startsWith("{")) { | ||
| continue; | ||
| } | ||
| const parsed = JSON.parse(trimmed); |
There was a problem hiding this comment.
Parse full JSON stdout before checking after.version
This logic parses outcome.stdout one line at a time and JSON.parses any line starting with {, but openclaw update --json emits pretty-printed multi-line JSON, so the first matching line is usually just { and parsing fails immediately. That leaves afterVersion null and causes successful auto-update runs to be logged as skipped, so success metadata is never recorded.
Useful? React with 👍 / 👎.
Fixes #58107
Updated after review:
Original description:
When multiple Feishu group agents (xixi, ling, aoao, weiwei) send messages in a group chat, only the main agent's messages are delivered correctly. Subagents were using the wrong accountId ('default') instead of their assigned accountId.