Google Chat: respect replyToMode: "off" for DM threading#67055
Google Chat: respect replyToMode: "off" for DM threading#67055banddude wants to merge 2 commits into
Conversation
The Google Chat monitor unconditionally copies message.thread.name from inbound webhooks into ReplyToId and passes it through to all outbound sends (typing indicator, text replies, media replies). This causes all agent responses to land inside a thread, even when replyToMode is set to "off" in the channel config. This change checks account.config.replyToMode at all five thread propagation points and suppresses threading when the config is "off": - Inbound context: ReplyToId and ReplyToIdFull - Typing indicator: sendGoogleChatMessage thread param - Text reply: sendGoogleChatMessage thread param - Media reply: sendGoogleChatMessage thread param Fixes openclaw#52475 Related: openclaw#39554
openclaw-barnacle
Bot
added
channel: googlechat
Greptile SummaryFixes the Google Chat monitor unconditionally propagating The fix is correctly scoped — other Confidence Score: 5/5Safe to merge — the change is a focused, correct guard at all threading propagation points with no functional regressions for other replyToMode values. All findings are P2 (a redundant defensive check worth a clarifying comment). The core logic is correct, covers every send site, and does not regress other modes. Confidence is 5. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/googlechat/src/monitor.ts
Line: 437
Comment:
**Delivery-time check is redundant but not harmful**
`payload.replyToId` flows directly from `ctxPayload.ReplyToId`, which is already set to `undefined` when `replyToMode === "off"` (line 258). The extra guard here — and the matching one on line 466 — won't fire in practice under normal pipeline flow. They're safe to keep as a defensive layer if `replyToId` could ever be injected by a pipeline stage downstream of the inbound context build, but worth a comment to clarify the intent:
```suggestion
thread: account.config.replyToMode === "off" ? undefined : payload.replyToId, // defensive: payload.replyToId is already undefined when replyToMode is "off"
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Google Chat: respect replyToMode: "off" ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| space: spaceId, | ||
| text: chunk, | ||
| thread: payload.replyToId, | ||
| thread: account.config.replyToMode === "off" ? undefined : payload.replyToId, |
There was a problem hiding this comment.
payload.replyToId flows directly from ctxPayload.ReplyToId, which is already set to undefined when replyToMode === "off" (line 258). The extra guard here — and the matching one on line 466 — won't fire in practice under normal pipeline flow. They're safe to keep as a defensive layer if replyToId could ever be injected by a pipeline stage downstream of the inbound context build, but worth a comment to clarify the intent:
| thread: account.config.replyToMode === "off" ? undefined : payload.replyToId, | |
| thread: account.config.replyToMode === "off" ? undefined : payload.replyToId, // defensive: payload.replyToId is already undefined when replyToMode is "off" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/googlechat/src/monitor.ts
Line: 437
Comment:
**Delivery-time check is redundant but not harmful**
`payload.replyToId` flows directly from `ctxPayload.ReplyToId`, which is already set to `undefined` when `replyToMode === "off"` (line 258). The extra guard here — and the matching one on line 466 — won't fire in practice under normal pipeline flow. They're safe to keep as a defensive layer if `replyToId` could ever be injected by a pipeline stage downstream of the inbound context build, but worth a comment to clarify the intent:
```suggestion
thread: account.config.replyToMode === "off" ? undefined : payload.replyToId, // defensive: payload.replyToId is already undefined when replyToMode is "off"
```
How can I resolve this? If you propose a fix, please make it concise.Verify that replyToMode: 'off' resolves correctly at both the channel level and as a per-account override through the threading adapter. Documents the config contract that monitor.ts now honors.
![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: 909a4a6982
ℹ️ 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".
| space: spaceId, | ||
| text: chunk, | ||
| thread: payload.replyToId, | ||
| thread: account.config.replyToMode === "off" ? undefined : payload.replyToId, |
There was a problem hiding this comment.
payload.replyToId here has already been processed by the shared reply-threading flow, which preserves explicit [[reply_to:*]] / [[reply_to_current]] directives even when the mode is off. Replacing it with account.config.replyToMode === "off" ? undefined : ... strips those explicit overrides, so intentionally threaded Google Chat replies are forced flat whenever replyToMode is off (and the same guard appears in the media send path).
Useful? React with 👍 / 👎.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: the branch has a concrete correctness blocker and no inspectable real-behavior proof, while the remaining Google Chat off-mode/threading work is tracked by #42510 and the newer active implementation path at #74235. Canonical path: Keep the canonical issue open and land #74235 or an equivalent Google Chat-owned fix that suppresses implicit off-mode threads while preserving explicit reply targets. So I’m closing this here and keeping the remaining discussion on the canonical linked item. Review detailsBest possible solution: Keep the canonical issue open and land #74235 or an equivalent Google Chat-owned fix that suppresses implicit off-mode threads while preserving explicit reply targets. Do we have a high-confidence way to reproduce the issue? Yes, source-level. Configure a Google Chat account with Is this the best way to solve the issue? No. The ingress and typing changes point at the right bug, but the final-send guards bypass the shared explicit-reply contract; the safer path is the canonical Google Chat threading fix that filters implicit sources before delivery. Security review: Security review cleared: The diff only changes Google Chat threading conditionals and tests, with no dependency, workflow, permission, secret-handling, or code-execution changes. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 2eee70e0a64b. |
|
Codex review: needs maintainer review before merge. What this changes: The PR adds Maintainer follow-up before merge: This is a real current-main bug and the PR is useful, but the remaining action is maintainer-guided revision or consolidation with nearby Google Chat threading PRs rather than an automated replacement branch. Review detailsBest possible solution: Revise or fold the PR into the current Google Chat threading work: filter only implicit inbound Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against acae48b790fa. |
1 participant
Summary
The Google Chat monitor unconditionally copies
message.thread.namefrom inbound webhooks intoReplyToIdand passes it through to all outbound sends (typing indicator, text replies, media replies). This causes all agent responses to land inside a thread, even whenreplyToModeis set to"off"in the channel config.Changes
This checks
account.config.replyToModeat all five thread propagation points inextensions/googlechat/src/monitor.tsand suppresses threading when the value is"off":ReplyToIdandReplyToIdFull(lines 258-259)sendGoogleChatMessagethread param (line 303)sendGoogleChatMessagethread param (line 437)sendGoogleChatMessagethread param (line 466)Testing
Tested on a live OpenClaw 2026.4.14 instance with Google Chat DMs. Before the patch, all replies landed in threads despite
replyToMode: "off". After the patch, replies appear flat in the DM conversation as expected.Fixes #52475
Related: #39554