Skip to content

fix(slack): respect replyToMode=off for inline directive reply tags#23513

Closed
dorukardahan wants to merge 2 commits intoopenclaw:mainfrom
dorukardahan:codex/pr16113-port-replytomode-off
Closed

fix(slack): respect replyToMode=off for inline directive reply tags#23513
dorukardahan wants to merge 2 commits intoopenclaw:mainfrom
dorukardahan:codex/pr16113-port-replytomode-off

Conversation

@dorukardahan
Copy link
Contributor

@dorukardahan dorukardahan commented Feb 22, 2026 •
edited by greptile-apps bot
Loading

Summary

This ports the fix from #16113 onto current main and resolves the Slack reply-threading edge case where inline reply directive tags (e.g. [[reply_to_current]]) can still force threading even when replyToMode: "off".

Why a new PR?

#16113 is currently marked CONFLICTING / DIRTY, but the root problem appears to be older branch history divergence (unrelated histories against current main), not a small line-level merge conflict. This PR re-applies the same intent on top of current main with updated file paths/tests.

Behavior

  • replyToMode: "off": ignore inline directive replyToId so top-level replies stay in the channel root
  • replyToMode: "all": preserve existing behavior and allow directive-driven threading
  • incoming thread replies remain threaded (unchanged)

Changes

Tests

Ran locally:

  • pnpm exec vitest run --config vitest.unit.config.ts src/slack/monitor.tool-result.test.ts ✅
  • pnpm exec oxfmt --check src/slack/monitor/replies.ts src/slack/monitor/message-handler/dispatch.ts src/slack/monitor.tool-result.test.ts CHANGELOG.md ✅

Credit

Original fix intent and test coverage direction from @zerone0x in #16113. This PR is a current-main port to get it mergeable.

AI-assisted

  • AI-assisted PR (Codex)
  • Lightly tested (targeted unit test + formatting)
  • I understand what the code does
  • Included prompts/session logs

Supersedes #16113

Greptile Summary

Ports the fix from #16113 to respect replyToMode: "off" for inline directive reply tags like [[reply_to_current]]. The implementation correctly ignores payload.replyToId when replyToMode === "off" in deliverReplies(), and the test coverage properly validates both the off and all modes.

Changes:

  • Modified deliverReplies() in src/slack/monitor/replies.ts to check replyToMode before using directive-based replyToId
  • Passed replyToMode through dispatchPreparedSlackMessage() call chain
  • Split the existing test into two cases: one for replyToMode: "off" (ignores directive) and one for replyToMode: "all" (respects directive)
  • Added changelog entry

Issues found:

  • Documentation at docs/channels/slack.md:244 contradicts the new behavior and needs updating

Confidence Score: 4/5

  • Safe to merge with one documentation fix needed
  • The code changes are clean and correct, test coverage is appropriate, and the fix addresses a real bug. Score is 4 (not 5) because the documentation contradicts the new behavior and must be updated before merge to avoid user confusion.
  • docs/channels/slack.md requires a documentation update to match the new behavior

Last reviewed commit: 590d75b

@dorukardahan dorukardahan mentioned this pull request Feb 22, 2026
Closed
6 tasks
@openclaw-barnacle openclaw-barnacle bot added channel: slack Channel integration: slack size: XS labels Feb 22, 2026
greptile-apps[bot]
greptile-apps bot reviewed Feb 22, 2026
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 22, 2026

Additional Comments (1)

docs/channels/slack.md
docs now contradict the new behavior — with this PR, replyToMode="off" ignores explicit [[reply_to_*]] tags (see src/slack/monitor/replies.ts:25)

Note: `replyToMode="off"` disables all reply threading, including explicit `[[reply_to_*]]` tags. To allow directive-driven threading, use `replyToMode="all"`.
Prompt To Fix With AI 
This is a comment left during a code review.
Path: docs/channels/slack.md
Line: 244

Comment:
docs now contradict the new behavior — with this PR, `replyToMode="off"` **ignores** explicit `[[reply_to_*]]` tags (see `src/slack/monitor/replies.ts:25`)

```suggestion
Note: `replyToMode="off"` disables all reply threading, including explicit `[[reply_to_*]]` tags. To allow directive-driven threading, use `replyToMode="all"`.
```

How can I resolve this? If you propose a fix, please make it concise.

arosstale
arosstale approved these changes Feb 22, 2026
View reviewed changes
Copy link
Contributor

@arosstale arosstale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct fix. With replyToMode: "off" and allowExplicitReplyTagsWhenOff: false on the Slack dock (set by #22944), inline [[reply_to_current]] tags should be stripped — but the tool-result dispatch path was still honouring them because it bypassed the applyReplyThreading gate. Tests updated to assert threadTs: undefined when mode is off, and a separate all test preserves the threading behaviour for that mode.

@dorukardahan
Copy link
Contributor Author

dorukardahan commented Feb 22, 2026

@steipete quick clarification (now that #16113 has been closed in favor of this PR):

This PR is not trying to remove agent overrides globally.

The narrower issue (also called out in the approved review here) is that the Slack tool-result dispatch path was still honoring inline [[reply_to_current]] / replyToId tags even when replyToMode: "off" + allowExplicitReplyTagsWhenOff: false (from #22944), because that path bypassed the applyReplyThreading gate.

So #23513 is intended to restore that Slack-specific invariant on the tool-result path, while preserving the existing directive-threading behavior for modes that allow it (e.g. replyToMode: "all", covered by the added test).

If the intended design is that inline directive tags should always override replyToMode even in that Slack path, happy to follow your direction and rework/close accordingly.

@dorukardahan dorukardahan force-pushed the codex/pr16113-port-replytomode-off branch from 590d75b to ac73932 Compare February 22, 2026 11:47
@openclaw-barnacle openclaw-barnacle bot added the docs Improvements or additions to documentation label Feb 22, 2026
@vincentkoc vincentkoc mentioned this pull request Feb 22, 2026
Merged
@vincentkoc
Copy link
Member

vincentkoc commented Feb 22, 2026

Consolidating this into #23839 so we ship one scoped Slack threading fix:

  • honor replyToMode for Slack auto-created top-level thread_ts
  • ignore inline replyToId directive tags when replyToMode=off
  • keep inline directives working when reply mode allows threading

Closing this as superseded.
EOF && gh pr close 23513 --repo openclaw/openclaw --comment "Superseded by #23839."

@vincentkoc
Copy link
Member

vincentkoc commented Feb 22, 2026

Superseded by #23839.

@vincentkoc vincentkoc closed this Feb 22, 2026
This was referenced Feb 23, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@arosstale arosstale arosstale approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

channel: slack Channel integration: slack docs Improvements or additions to documentation size: XS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dorukardahan @vincentkoc @arosstale