Skip to content

fix(gateway): keep Feishu topic replies in thread - #16442

Closed
Kayphoon wants to merge 1 commit into
NousResearch:mainfrom
Kayphoon:fix/feishu-topic-replies
Closed

fix(gateway): keep Feishu topic replies in thread#16442
Kayphoon wants to merge 1 commit into
NousResearch:mainfrom
Kayphoon:fix/feishu-topic-replies

Conversation

@Kayphoon

Copy link
Copy Markdown

What does this PR do?

Fixes Feishu topic-mode replies so Hermes responses stay inside the originating topic instead of falling back to the main group. Feishu topic events carry the topic root as root_id, and Feishu only places messages in a topic when the reply API has a message-id target plus reply_in_thread=true.

This PR keeps the fix narrow:

  • Prefer message.root_id when deriving SessionSource.thread_id for Feishu inbound messages.
  • Pass the inbound message id into gateway streaming so the first streamed reply can use the reply API.
  • Add a Feishu adapter-level fallback for metadata-only sends (progress/status/approval/background paths): when metadata["thread_id"] is an om_... root message id, use it as the implicit reply target; reject omt_... topic resource ids as reply targets.

Related context: #16131, #9118, #9760. This is not just a duplicate of #16131 because it also fixes streamed replies and metadata-only Feishu sends; it is narrower than #9118 and does not add auto-threading.

Related Issue

Related to #6969

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/platforms/feishu.py
    • maps Feishu root_id before thread_id for topic/session routing
    • uses om_... thread metadata as a safe implicit reply target for metadata-only topic sends
    • avoids treating omt_... topic resource ids as message reply targets
  • gateway/stream_consumer.py
    • adds optional reply_to support and carries it through first-send/fallback/fresh/commentary stream sends
  • gateway/run.py
    • passes the inbound event_message_id to stream consumers only when source.thread_id exists
  • tests/gateway/test_feishu.py
    • adds root_id mapping, implicit metadata reply target, and omt_... rejection coverage
  • tests/gateway/test_stream_consumer.py
    • adds streaming first-send reply_to coverage

How to Test

  1. In a Feishu topic-mode group, send a message inside an existing topic.
  2. Verify Hermes processes it and replies inside the same topic, not the main group.
  3. Run regression tests:
scripts/run_tests.sh tests/gateway/test_feishu.py tests/gateway/test_stream_consumer.py tests/gateway/test_run_progress_topics.py::test_run_agent_progress_stays_in_originating_topic -q
.venv/bin/python -m py_compile gateway/platforms/feishu.py gateway/stream_consumer.py gateway/run.py tests/gateway/test_feishu.py tests/gateway/test_stream_consumer.py

Observed locally after rebasing onto latest origin/main:

  • Targeted tests: 232 passed, 37 skipped, 4 warnings
  • py_compile: passed

Full-suite note:

  • scripts/run_tests.sh with no args currently exits before pytest with ARGS[@]: unbound variable.
  • scripts/run_tests.sh tests/ was attempted and reached pytest, but the local environment/mainline suite failed on unrelated existing/optional-dependency issues (for example missing faster_whisper, ACP/TTS optional imports, Slack progress default test, and other broad current-main failures). The Feishu/streaming targeted suite above passes cleanly.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS / Darwin

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

Targeted regression output:

232 passed, 37 skipped, 4 warnings

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter comp/gateway Gateway runner, session dispatch, delivery labels Apr 27, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Feishu topic-routing work. An automated hermes-sweeper review found that current main already provides this behavior through later fixes.

  • plugins/platforms/feishu/adapter.py:3252 preserves root_id as topic/session scope, while :4609-4621 routes threaded metadata sends through the reply API.
  • gateway/run.py:16654-16662 passes the inbound message ID to streaming, and gateway/stream_consumer.py:1756-1765 uses it for the first streamed reply.
  • Current regression coverage includes tests/gateway/test_stream_consumer_thread_routing.py:35-52, tests/gateway/test_feishu.py:2080-2116, and tests/gateway/test_run_progress_topics.py:451-491.
  • The mainline implementation landed across a79b0ec46157efc91537e634a3dcc44a76f6dc7e, 441ef75d157d6308a9f14d42a7b0ec8566866ef8, ff14666cdc02ebad18a15de57ded4e9ec7d9f563, and e164a9c1ed781ab5e6e597ec74e9a933b81b7acd.

Closing as implemented on main; the current implementation also covers overflow and fallback thread-routing paths beyond this pre-plugin diff.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants