Skip to content

fix(discord): implement messages.statusReactions timing lifecycle#81623

Closed
umeshchandra-rao wants to merge 1 commit into
openclaw:mainfrom
umeshchandra-rao:fix/discord-status-reactions
Closed

fix(discord): implement messages.statusReactions timing lifecycle#81623
umeshchandra-rao wants to merge 1 commit into
openclaw:mainfrom
umeshchandra-rao:fix/discord-status-reactions

Conversation

@umeshchandra-rao

Copy link
Copy Markdown

Resolves #78431

This PR fixes the issue where Discord status reactions did not properly respect the doneHoldMs and errorHoldMs lifecycle timings configured in cfg.messages.statusReactions.timing. It wires the logic correctly in message-handler.process.ts by utilizing the configuration values instead of hardcoding DEFAULT_TIMING.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. channel: discord Channel integration: discord size: XS labels May 14, 2026
@clawsweeper

clawsweeper Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open. Current main and v2026.6.6 still ignore configured Discord terminal status-reaction hold timing, but this PR is conflicting, lacks real Discord proof, and needs to preserve current failed-final delivery error timing before it can merge.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

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

Yes, at source level. Current main and v2026.6.6 pass configured timing into the Discord controller but still use hardcoded DEFAULT_TIMING for terminal cleanup sleeps; I did not run a live Discord repro in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. The direction matches Slack and Telegram, but the branch is stale, must preserve finalDeliveryFailed, and needs focused tests plus real Discord proof.

Security review:

Security review cleared: The diff only changes Discord runtime timing selection and does not add dependency, workflow, secret, package, or code-execution surface.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-05-14T02:56:40Z, is older than 30 days, and the latest review rated it F.
  • proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • vincentkoc: Current-main blame for the Discord finalizer and controller setup points to Vincent Koc's recent commit in this checkout; local history is shallow/grafted, so this is a routing signal rather than complete ownership proof. (role: recent area contributor; confidence: medium; commits: 2e240e772b83, 00479b12d10b; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/monitor/message-handler.process.test.ts)
  • steipete: Recent commits extracted Discord ack reaction helpers and adjusted explicit tool-only Discord status reactions on the same monitor path. (role: recent Discord status-reaction contributor; confidence: high; commits: 570ed4285ec4, 0949f4fe5151; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/monitor/message-handler.process.test.ts, extensions/discord/src/monitor/ack-reactions.ts)
  • Takhoffman: Authored the nearby Discord status-reactions toggle fix that touched the same lifecycle surface and tests. (role: adjacent status-reaction contributor; confidence: medium; commits: 30fc29c9b035; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/monitor/message-handler.process.test.ts)
  • Hsiao A: Authored the merged Slack/shared status-reaction lifecycle work used as a sibling contract for configured timing behavior. (role: shared lifecycle contributor; confidence: medium; commits: cea7162490f4; files: src/channels/status-reactions.ts, src/channels/status-reactions.slack-lifecycle.test.ts, extensions/slack/src/monitor/message-handler/dispatch.ts)

Codex review notes: model internal, reasoning high; reviewed against 04875efd28ee.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 2, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 2, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

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

Labels

channel: discord Channel integration: discord merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discord: implement messages.statusReactions lifecycle (done emoji persistence) — currently Telegram-only despite docs

2 participants