Skip to content

Discord: guard message:received hook session keys#33038

Closed
webdevtodayjason wants to merge 1 commit intoopenclaw:mainfrom
webdevtodayjason:codex/discord-message-hook-guard-refresh
Closed

Discord: guard message:received hook session keys#33038
webdevtodayjason wants to merge 1 commit intoopenclaw:mainfrom
webdevtodayjason:codex/discord-message-hook-guard-refresh

Conversation

@webdevtodayjason
Copy link
Copy Markdown
Contributor

@webdevtodayjason webdevtodayjason commented Mar 3, 2026

Summary

  • preserve message:received internal hook delivery when SessionKey is blank by falling back to CommandTargetSessionKey
  • keep Discord monitor session-key fallback aligned by using route.sessionKey when context/session fields are missing
  • include focused regressions for both fallback paths

Testing

  • pnpm test -- src/auto-reply/reply/dispatch-from-config.test.ts
  • pnpm test -- src/discord/monitor/message-handler.process.test.ts

Supersedes #31264 (same fix rebased on current main).

@webdevtodayjason webdevtodayjason mentioned this pull request Mar 3, 2026
Closed
18 tasks
@webdevtodayjason
Copy link
Copy Markdown
Contributor Author

webdevtodayjason commented Mar 3, 2026

Maintainer-ready summary:

  • Scope: Discord/internal hook session-key guardrails.
  • Behavior: ensures message:received hook dispatch falls back to CommandTargetSessionKey when SessionKey is blank, and keeps Discord monitor fallback aligned with route-level session keys.
  • Risk: low; only session-key selection paths are touched.
  • Coverage: focused regressions in dispatch-from-config and message-handler.process.
  • CI: running.

If this shape looks right, this should be safe to merge as a targeted guardrail fix.

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: S labels Mar 3, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 3, 2026

Greptile Summary

This PR fixes two related session-key fallback gaps in the Discord/auto-reply pipeline:

  1. dispatch-from-config.ts (line 179) – the message:received internal hook was silently dropped whenever ctx.SessionKey was absent, even if ctx.CommandTargetSessionKey was available. The new hookSessionKey variable correctly falls back to CommandTargetSessionKey (trimmed, with an explicit || undefined guard against blank strings).

  2. message-handler.process.ts (lines 336-341, 384) – the SessionKey field in the finalized inbound context could end up undefined when boundSessionKey, autoThreadContext?.SessionKey, and threadKeys.sessionKey were all empty, leaving route.sessionKey unused until the separate persistedSessionKey assignment. The refactored inboundSessionKey array (using .find() to skip empty/whitespace strings) folds route.sessionKey into the cascade, making both ctxPayload.SessionKey and persistedSessionKey draw from a single, consistently ordered source.

Both changes are paired with focused regression tests that cover the specific fallback paths introduced.

Confidence Score: 5/5

  • This PR is safe to merge — changes are narrowly scoped to session-key fallback resolution with no new network I/O, state mutations, or architectural changes.
  • The logic is straightforward and well-tested. Session-key selection uses simple fallback chains with .trim() guards and .find() filtering. Risk surface is minimal — the changes only affect which string is selected from existing fields, not how those fields are populated. Both fallback paths have dedicated regression tests, and the existing test suite continues to cover non-fallback paths.
  • No files require special attention.

Last reviewed commit: 8c96472

@webdevtodayjason
Copy link
Copy Markdown
Contributor Author

webdevtodayjason commented Mar 3, 2026

Friendly ping: this PR is currently clean with required checks green. If the scope looks good, this should be ready for maintainer merge.

@thewilloftheshadow
Copy link
Copy Markdown
Member

thewilloftheshadow commented Mar 3, 2026

Superceded by existing commits on main

@ferchache ferchache mentioned this pull request Apr 28, 2026
Closed
@amknight amknight mentioned this pull request Apr 28, 2026
Merged
amknight added a commit that referenced this pull request Apr 28, 2026
@amknight
fix: Discord read/search timeout, session-key fallback, and gateway e…
e4ff7c1 
…xecution mode (#73521)

* fix: Discord read/search timeout, session-key fallback, and gateway execution mode

- Add 15s timeout to readMessagesDiscord and searchMessagesDiscord so they
  fail fast instead of hanging indefinitely (#73431)
- Fall back to CommandTargetSessionKey in dispatchReplyFromConfig when
  SessionKey is empty, so Discord inbound message:received hooks fire
  reliably (#73431, refs #33038)
- Add resolveExecutionMode to Discord channel actions routing read/search
  through gateway timeout path, matching Telegram's pattern (#73431)

* fix: move timeout to fetch layer, drop send.messages wrapper

Inject AbortSignal.timeout into the Discord proxy-request-client fetch
wrapper so every Discord REST call gets a 15s timeout at the HTTP level.
This replaces the Promise.race wrapper in send.messages.ts — cleaner,
covers all calls, and actually aborts the TCP connection.

* fix: remove unused callerController variable in proxy-request-client test

* fix: remove unnecessary mergeAbortSignal helper
bminicore pushed a commit to bminicore/openclaw-fork that referenced this pull request Apr 29, 2026
@amknight @bminicore
fix: Discord read/search timeout, session-key fallback, and gateway e…
6ad54f3 
…xecution mode (openclaw#73521)

* fix: Discord read/search timeout, session-key fallback, and gateway execution mode

- Add 15s timeout to readMessagesDiscord and searchMessagesDiscord so they
  fail fast instead of hanging indefinitely (openclaw#73431)
- Fall back to CommandTargetSessionKey in dispatchReplyFromConfig when
  SessionKey is empty, so Discord inbound message:received hooks fire
  reliably (openclaw#73431, refs openclaw#33038)
- Add resolveExecutionMode to Discord channel actions routing read/search
  through gateway timeout path, matching Telegram's pattern (openclaw#73431)

* fix: move timeout to fetch layer, drop send.messages wrapper

Inject AbortSignal.timeout into the Discord proxy-request-client fetch
wrapper so every Discord REST call gets a 15s timeout at the HTTP level.
This replaces the Promise.race wrapper in send.messages.ts — cleaner,
covers all calls, and actually aborts the TCP connection.

* fix: remove unused callerController variable in proxy-request-client test

* fix: remove unnecessary mergeAbortSignal helper
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
@amknight
fix: Discord read/search timeout, session-key fallback, and gateway e…
af58488 
…xecution mode (openclaw#73521)

* fix: Discord read/search timeout, session-key fallback, and gateway execution mode

- Add 15s timeout to readMessagesDiscord and searchMessagesDiscord so they
  fail fast instead of hanging indefinitely (openclaw#73431)
- Fall back to CommandTargetSessionKey in dispatchReplyFromConfig when
  SessionKey is empty, so Discord inbound message:received hooks fire
  reliably (openclaw#73431, refs openclaw#33038)
- Add resolveExecutionMode to Discord channel actions routing read/search
  through gateway timeout path, matching Telegram's pattern (openclaw#73431)

* fix: move timeout to fetch layer, drop send.messages wrapper

Inject AbortSignal.timeout into the Discord proxy-request-client fetch
wrapper so every Discord REST call gets a 15s timeout at the HTTP level.
This replaces the Promise.race wrapper in send.messages.ts — cleaner,
covers all calls, and actually aborts the TCP connection.

* fix: remove unused callerController variable in proxy-request-client test

* fix: remove unnecessary mergeAbortSignal helper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: discord Channel integration: discord size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@webdevtodayjason @thewilloftheshadow