Skip to content

fix(googlechat): preserve thread targets through reply delivery#89790

Closed
clawsweeper[bot] wants to merge 2 commits into
mainfrom
clawsweeper/gitcrawl-4952-google-chat-dm-threading-broken-each-response-creates-new-thread
Closed

fix(googlechat): preserve thread targets through reply delivery#89790
clawsweeper[bot] wants to merge 2 commits into
mainfrom
clawsweeper/gitcrawl-4952-google-chat-dm-threading-broken-each-response-creates-new-thread

Conversation

@clawsweeper

@clawsweeper clawsweeper Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Verification

  • node scripts/run-vitest.mjs extensions/googlechat/src/monitor.reply-delivery.test.ts extensions/googlechat/src/monitor-durable.test.ts
  • pnpm check:changed

Refs: #42510, #69422, #70041, #74235.

ClawSweeper 🐠 replacement reef notes:

Inherited issue-closing references from the source PR:
Fixes #52475
Closes #42510

Co-author credit kept:

fish notes: model gpt-5.5, reasoning high; reviewed against 9336240.

@clawsweeper clawsweeper Bot added channel: googlechat Channel integration: googlechat size: XL proof: supplied External PR includes structured after-fix real behavior proof. proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 3, 2026
@clawsweeper clawsweeper Bot added P1 High-priority user-facing bug, regression, or broken workflow. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. size: L size: XS clawsweeper Tracked by ClawSweeper automation labels Jun 3, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: supplied External PR includes structured after-fix real behavior proof. label Jun 3, 2026
@clawsweeper

clawsweeper Bot commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 2:11 AM ET / 06:11 UTC.

Summary
The PR updates Google Chat monitor delivery so normalized current thread targets are used for text, media, typing-placeholder cleanup, and durable final replies, with focused regression tests.

PR surface: Source +25, Tests +80. Total +105 across 5 files.

Reproducibility: yes. Current-main source shows typing messages and final delivery can still carry stale Google Chat thread metadata after replyToMode removes the intended final target, and the PR adds focused source-level regression coverage for those paths.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: vector/embedding metadata: extensions/googlechat/src/monitor-durable.test.ts. Migration or upgrade compatibility proof is recorded; maintainers should verify it before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #42510
Summary: This PR is the focused writable candidate fix for the canonical Google Chat replyToMode/off thread-routing issue; broader Google Chat thread/session work overlaps but is not a cleaner superseding landing target today.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Attach redacted live Google Chat proof for ca92a51 or apply an explicit maintainer proof override.
  • Choose whether this PR or the broader Google Chat thread-preservation PR is the canonical landing path.

Proof path suggestion
A real Google Chat visual smoke would materially verify the user-visible thread versus top-level delivery behavior that unit tests cannot fully prove. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P1] Changing Google Chat reply targeting and typing-placeholder cleanup can visibly send replies to a stale thread, the wrong thread, or the space root if the merge is wrong.
  • [P1] The exact PR head has source, focused tests, and CI signal, but no attached live Google Chat smoke or explicit maintainer proof override for the current branch.
  • [P1] The broader overlapping Google Chat thread-preservation PR remains open and conflicting, so maintainers should choose one canonical landing path before closing the cluster.

Maintainer options:

  1. Capture exact-head Google Chat proof (recommended)
    Run a real Google Chat smoke for ca92a51 that shows replyToMode off posts top-level and threaded group replies stay in the intended thread before merge.
  2. Accept source and CI proof
    Maintainers can intentionally merge with the remaining live-service uncertainty because the code path is focused and the branch is clean.
  3. Pause for the broader thread branch
    If maintainers prefer the broader thread-preservation branch as canonical, pause this replacement until that branch is repaired or chosen.

Next step before merge

  • No automated repair is indicated; maintainers need to decide the exact-head live-proof stance and canonical Google Chat thread-routing landing path.

Security
Cleared: No concrete security or supply-chain concern was found; the diff is limited to Google Chat delivery logic and focused tests.

Review details

Best possible solution:

Land one canonical Google Chat thread-routing fix after exact-head live proof or a maintainer proof override, then close the linked issue/source-PR cluster with the merged commit.

Do we have a high-confidence way to reproduce the issue?

Yes. Current-main source shows typing messages and final delivery can still carry stale Google Chat thread metadata after replyToMode removes the intended final target, and the PR adds focused source-level regression coverage for those paths.

Is this the best way to solve the issue?

Yes. This is the best focused fix I found because it stays inside the Google Chat plugin delivery boundary and uses the existing explicit-null durable reply contract instead of adding new config or broad core reply API surface.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 56c2d637d940.

Label changes

Label changes:

  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a ClawSweeper-authored replacement PR, so the external-contributor proof gate does not apply; live Google Chat proof remains a maintainer merge-risk decision.
  • remove status: needs maintainer proof decision: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P1: The PR targets a real Google Chat channel workflow where replies can be visibly delivered to the wrong thread or top-level space.
  • merge-risk: 🚨 compatibility: The patch changes Google Chat reply targeting in off-mode, typing-placeholder, and durable final-delivery paths that existing users may rely on.
  • merge-risk: 🚨 message-delivery: The touched monitor and delivery paths directly decide whether Google Chat replies land in the intended thread or top-level conversation.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a ClawSweeper-authored replacement PR, so the external-contributor proof gate does not apply; live Google Chat proof remains a maintainer merge-risk decision.
Evidence reviewed

PR surface:

Source +25, Tests +80. Total +105 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 29 4 +25
Tests 2 80 0 +80
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 109 4 +105

What I checked:

  • Repository policy read: Root AGENTS.md and scoped extensions/AGENTS.md were read fully; their plugin-boundary and compatibility-sensitive channel-delivery guidance apply to this review. (AGENTS.md:1, 56c2d637d940)
  • Current main typing indicator still uses inbound thread before replyToMode filtering: Current main derives replyThreadName from the inbound Google Chat thread and sends the typing message with that thread, before final payload delivery can reflect replyToMode filtering. (extensions/googlechat/src/monitor.ts:302, 56c2d637d940)
  • Current main text/media delivery uses raw payload replyToId: Current main sends Google Chat text and media using payload.replyToId directly, so stale, untrimmed, or absent payload thread metadata controls the Chat API thread target. (extensions/googlechat/src/monitor-reply-delivery.ts:74, 56c2d637d940)
  • Current main durable helper omits explicit null suppression: Current main omits reply metadata when the final payload has no replyToId, while the durable delivery resolver falls back to ctxPayload ReplyToId unless replyToId is explicitly null. (extensions/googlechat/src/monitor-durable.ts:19, 56c2d637d940)
  • PR head normalizes thread target and clears stale typing placeholder: The PR head trims payload.replyToId once, deletes a typing placeholder when its thread differs from the final payload thread, and uses the normalized value for text and media sends. (extensions/googlechat/src/monitor-reply-delivery.ts:36, ca92a51d7ccb)
  • PR head passes the typing thread into delivery: The monitor passes typingMessageThreadName: replyThreadName into deliverGoogleChatReply so delivery can distinguish a current-thread placeholder from stale threaded typing metadata. (extensions/googlechat/src/monitor.ts:423, ca92a51d7ccb)

Likely related people:

  • steipete: Recent commits extracted Google Chat reply delivery and migrated the monitor to the message lifecycle/durable helper path touched by this PR. (role: recent area contributor; confidence: high; commits: 4f91d81e1dbc, 05eda57b3c72, cdbed3c9b1c4; files: extensions/googlechat/src/monitor-reply-delivery.ts, extensions/googlechat/src/monitor-durable.ts, extensions/googlechat/src/monitor.ts)
  • jailbirt: Recent Google Chat monitor work introduced the group-only replyThreadName guard for DM thread suppression, which this PR carries into typing-message delivery. (role: recent adjacent contributor; confidence: high; commits: da822dd28bf8; files: extensions/googlechat/src/monitor.ts, extensions/googlechat/src/monitor.test.ts)
  • franco-viotti: Recent Google Chat API work changed send results to return thread metadata, adjacent to the outbound thread-target behavior under review. (role: recent adjacent contributor; confidence: medium; commits: 7305d9bacaec; files: extensions/googlechat/src/api.ts, extensions/googlechat/src/targets.test.ts)
  • novan: The earlier Google Chat messageReplyOption groundwork was landed from contributor work and is part of the thread-routing behavior this PR is repairing. (role: prior thread-reply groundwork contributor; confidence: medium; commits: cdbed3c9b1c4; files: extensions/googlechat/src/api.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-06-27T15:58:32.945Z sha ca92a51 :: needs maintainer review before merge. :: none

This was referenced Jun 10, 2026
Closed
Open
Open
This was referenced Jun 18, 2026
Open
Open
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 21, 2026
@clawsweeper

clawsweeper Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

🦞🔧
ClawSweeper applied a repair to this PR branch.

Repair: kept the fix on this contributor branch instead of opening a replacement PR.
Validation: pnpm check:changed; pnpm lint; pnpm check:test-types
Updated head: ca92a51d7ccb
Run: https://github.com/openclaw/clawsweeper/actions/runs/28065194534

Current state: exact-head review queued immediately; GitHub checks and the review verdict gate final merge.

Automerge progress:

  • 2026-06-24 00:09:59 UTC review queued ca92a51d7ccb (after repair)

@clawsweeper clawsweeper Bot removed the status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. label Jun 23, 2026
@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 24, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

ClawSweeper status: this ClawSweeper-authored replacement PR is blocked on real behavior proof.

Reviewed head: ca92a51d7ccb8e1acb2b9f49d5880f9af1f43d30
Proof status: not_applicable
Updated: 2026-07-01T11:06:20.306Z

Maintainer decision needed:

  • capture real behavior proof for this head
  • apply proof: override if the proof requirement should be waived
  • pause or close the replacement if proof should not be pursued

Possible manual Mantis/desktop proof suggestion:

@openclaw-mantis visual task: verify in Google Chat that replyToMode off sends final replies top-level and threaded group replies keep the intended thread on ca92a51d7ccb.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 25, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Open
@clawsweeper clawsweeper Bot added status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision. labels Jun 29, 2026
@steipete steipete self-assigned this Jul 11, 2026
@steipete steipete mentioned this pull request Jul 11, 2026
Merged
@steipete

steipete commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Landed the current-main replacement in #104402 as 8756cc4a3b4c693f3e314cc3a637135aab69f371.

The replacement keeps the useful direction from this branch, but refactors the current Google Chat monitor/durable-delivery paths so typing placeholder identity and location stay together, final targets are normalized once, mismatched placeholders are replaced rather than edited across surfaces, and explicit null suppresses stale inbound reply fallback. Focused Google Chat tests passed (26), the full plugin suite passed (199), changed checks and fresh autoreview were clean, and exact-head hosted CI passed: https://github.com/openclaw/openclaw/actions/runs/29149958216

This branch had drifted from current main and was not maintainer-editable, so it could not safely receive the required refactor. Thanks to @jai, @neeravmakwana, @banddude, and @novan for the related source work carried forward and credited in the replacement. Closing as superseded by the landed PR.

@steipete steipete closed this Jul 11, 2026
@jai

jai commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Awesome sauce thanks @steipete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

channel: googlechat Channel integration: googlechat clawsweeper Tracked by ClawSweeper automation merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Google Chat: add option to disable automatic threading on replies Google Chat: replyToMode: "off" does not suppress thread replies

2 participants

@steipete @jai