Discord ambient room events#82573
Conversation
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. from source inspection: current main classifies Discord unmentioned group/channel chatter as Real behavior proof Next step before merge Security Review detailsBest possible solution: Land the reviewed Discord delivery-correlation path after maintainer approval and appropriate Discord/test proof, preserving the related core room-event contract from #82606. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main classifies Discord unmentioned group/channel chatter as Is this the best way to solve the issue? Yes in direction: the PR follows the existing room-event delivery-correlation pattern and moves Discord history clearing behind visible message-tool/core outbound delivery. No safer narrower repair was identified, but protected maintainer review remains the landing path. What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against c2e90914b735. |
ae727cc to
1eb2d47
Compare
|
Rewrote this PR on current Verification:
Known proof gap:
Applied maintainer |
1eb2d47 to
c835158
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8351580d1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| if (adminResult !== undefined) { | ||
| if (action === "thread-reply") { | ||
| notifyVisibleOutbound(readStringParam(params, "threadId") ?? readTarget()); |
There was a problem hiding this comment.
Avoid throwing after successful thread-reply sends
When action === "thread-reply", this new notification path calls readStringParam(params, "threadId") ?? readTarget(). readTarget() only accepts to/from, but the existing admin handler explicitly supports CLI channelId input (handle-action.guild-admin.ts notes to/channelId compatibility), so valid channelId-only requests now throw after handleDiscordAction has already sent the reply. That turns successful thread replies into tool errors for legacy CLI callers and skips normal post-send flow.
Useful? React with 👍 / 👎.
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
c835158 to
0344f0b
Compare
|
Final maintainer proof for the rewritten branch:
Known proof gap: no live Discord API runtime capture; this is covered by focused Discord/core tests plus the proof override label. |
Summary
Verification
Real behavior proof
Behavior addressed: Discord room-event context stays available until the agent actually sends a visible Discord reply through the message tool or durable core send path.
Real environment tested: local OpenClaw checkout with Discord/core outbound unit coverage on the rebased PR branch; live Discord delivery was not run, so proof override is applied.
Exact steps or command run after this patch: see Verification above.
Evidence after fix: focused Discord/core tests cover pending room-event history, clearing on local visible sends, clearing on queued core sends, and metadata propagation into the Discord outbound adapter.
Observed result after fix: focused tests, test types, diff check, and Codex review all passed on rebased head 0344f0b.
What was not tested: live Discord bot runtime against the Discord API.