Skip to content

fix(slack): use thread_ts key for multibot_threads cache lookup in handle_message#3

Open
antigenius0910 wants to merge 2 commits into
test-branchfrom
bugfix/556-multibot-cache-key
Open

fix(slack): use thread_ts key for multibot_threads cache lookup in handle_message#3
antigenius0910 wants to merge 2 commits into
test-branchfrom
bugfix/556-multibot-cache-key

Conversation

@antigenius0910
Copy link
Copy Markdown
Owner

@antigenius0910 antigenius0910 commented Apr 24, 2026 •
edited
Loading

Summary

Fixes the wrong cache key used in handle_message when computing other_bot_present, which caused streaming to never be disabled in multi-bot threads.

  • multibot_threads cache is written with thread_ts as key (note_other_bot_in_thread, line 91)
  • bot_participated_in_thread (line 228) reads it correctly with thread_ts
  • handle_message (line 1120) was reading with channel_id — a key that never exists — so other_bot_present was always false and use_streaming() always returned true

Fix: use thread_channel.thread_id with the same TTL-aware pattern as line 228.

Closes openabdev#556

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1496961096182006023/1497313281000603819

Test plan

  • Deploy to fork and run E2E bot test
  • Step 1: @mention bot-1 in a channel — confirm reply with edited: true (expected, only bot-1 in thread)
  • Step 2: In same thread, @mention bot-2 — get a reply (populates cache with thread_ts)
  • Step 3: In same thread, @mention bot-1 again
  • Step 4: Confirm bot-1's reply has no edited field via conversations.replies — streaming must be OFF

🤖 Generated with Claude Code

@antigenius0910 @claude
fix(slack): use thread_ts key for multibot_threads cache lookup in ha…
d2919bc 
…ndle_message

cache is written with thread_ts (note_other_bot_in_thread:91) but was
read with channel_id at line 1120, causing other_bot_present to always
be false and streaming to never be disabled in multi-bot threads.

Fixes openabdev#556

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added pending-screening PR awaiting automated screening closing-soon and removed closing-soon labels Apr 24, 2026
@antigenius0910 @claude
chore: remove test.txt
06ce9ed 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
antigenius0910 pushed a commit that referenced this pull request May 8, 2026
@wangyuyan-agent
feat(gateway): feishu rich text, streaming, slash commands, and bot-t…
40310f3 
…o-bot readiness (openabdev#706)

* feat(gateway): feishu slash commands, rich text, and bot-to-bot readiness

- /reset and /cancel interception in OAB core gateway adapter (src/gateway.rs)
- Rich text (post) messaging: markdown_to_post() converter + send_post_message()
- Bot-to-bot: AllowBots enum, trusted_bot_ids, max_bot_turns, bot detection heuristic
- Updated docs/feishu.md with new sections and env vars

* feat(gateway): streaming (typewriter) support for Feishu adapter

- OAB core: send_message request-response for Feishu (real message_id)
- OAB core: edit_message sends command via WebSocket
- OAB core: use_streaming returns true for Feishu only
- GatewayResponse: add message_id field (backward compatible)
- Gateway service: edit_feishu_message via PUT API
- Gateway service: handle_reply returns message_id in response

* docs: add streaming section to feishu.md

* fix: address PR openabdev#706 review — blockers, suggested changes, and nits

- Remove dead variables in markdown_to_post() (blocker #2)
- Fix allowlist bypass: check allowlist before bot classification (suggested #3)
- Fix thread key mismatch: use thread_id.unwrap_or(channel_id) (suggested #4)
- Add /models and /agents slash commands
- NITs: let mut→let, cancel format, DM comment, bot_turns TODO,
  timeout 5s, dead_code allow, warn on parse error

* docs: fix bot-to-bot description after allowlist bypass fix

* docs: add bot-to-bot env vars to gateway README

* fix: use split_once per clippy

* refactor: remove /models /agents (follow-up PR), replace platform hardcode with supports_streaming()

- Remove /models and /agents slash command interception (will be a separate PR
  with exact matching, ambiguity handling, get_or_create, category aliasing)
- Replace channel.platform == "feishu" with supports_streaming() method
- supports_streaming() uses matches!(platform_name, "feishu" | "lark")

* refactor: remove /models /agents (follow-up PR), config-driven streaming

- Remove /models and /agents slash commands (separate PR with design review)
- Replace platform-specific checks with config-driven streaming flag
- send_message: all platforms use request-response + 5s timeout fallback
- use_streaming: reads self.streaming from GatewayConfig
- Add gateway.streaming config field (default false)
- Update docs/feishu.md: remove /models /agents, streaming via config

* fix: streaming-only request-response, proper inline code parsing

- send_message: request-response only when self.streaming is true,
  fire-and-forget otherwise (no Telegram regression)
- parse_inline: paired backtick parsing preserves literal content
  inside code spans, no longer strips markers inside inline code

* fix(gateway): slash command responses use fire-and-forget, not request-response

- Extract SharedWsTx type alias (Arc<Mutex<SplitSink>>) so ws_tx can be
  shared between GatewayAdapter and the event loop
- Add send_fire_and_forget() helper for slash command responses that
  don't need message_id back (no 5s timeout penalty)
- send_message() still uses request-response when streaming=true
  (needed for placeholder message_id in streaming edit flow)

Addresses review feedback: slash command responses (/reset, /cancel)
no longer go through the request-response path, avoiding unnecessary
latency when gateway.streaming is enabled.

* docs(gateway): add TODO for bot-to-bot core guard gap (擺渡法師 finding)

Gateway core unconditionally drops is_bot events, but feishu adapter's
AllowBots filtering happens before events reach core. When Feishu lifts
the bot-to-bot delivery restriction, this guard needs to become
adapter-aware. Telegram adapter does not filter bots, so we cannot
simply remove the guard without introducing regression.

* docs(feishu): narrow bot-to-bot claim to gateway-side scaffolding only

Per 擺渡法師 review: PR claimed 'bot-to-bot readiness' but OAB core
unconditionally drops is_bot events (src/gateway.rs:430). The gateway
adapter's AllowBots filtering works, but events never reach the router.

Updated docs/feishu.md to explicitly state both blockers:
1. Feishu platform doesn't deliver bot messages to other bots
2. OAB core drops is_bot events before router

Claim narrowed from 'readiness' to 'gateway-side scaffolding only'.

* fix(feishu): paired-only marker stripping + send failure response (普渡法師 findings)

1. parse_inline(): only strip *paired* markdown markers (**, *, ~~).
   Unpaired markers kept as literal text. Fixes: ~/.ssh → /.ssh,
   *.rs → .rs, 3 * 4 → 3  4

2. handle_reply(): send GatewayResponse { success: false } when
   send_post_message() fails. Prevents core from waiting 5s timeout
   then sending edit_message to a non-existent message_id.

---------

Co-authored-by: wangyuyan-agent <265828726+wangyuyan-agent@users.noreply.github.com>
Co-authored-by: chaodu-agent <chaodu-agent@openab.dev>
chaodu-agent pushed a commit that referenced this pull request May 23, 2026
@canyugs @claude
feat(gateway): Google Chat attachment support (image / file / audio +…
0079352 
… STT) (openabdev#762)

* feat(gateway): inbound attachment support for Google Chat

Implements image / text file / audio download from Google Chat via
Media API + service account token, following the PR openabdev#731 base64 pattern.

Changes:
- GoogleChatMessage: parse attachment[] array (Attachment / AttachmentDataRef / DriveDataRef structs)
- GoogleChatMediaRef enum: Image / File / Audio variants for typed dispatch
- parse_attachments(): branches on contentType prefix, skips DRIVE_FILE source
- download_googlechat_image(): resize → 1200px JPEG q75, max 10MB, GIF preserved
- download_googlechat_file(): text extension whitelist (.txt/.md/.py/...), max 512KB
- download_googlechat_audio(): forwarded as-is for core STT pipeline, max 25MB
- media_url(): percent-encode resource_name as path segment
- webhook handler: parses attachments, async-downloads via adapter token, populates Content.attachments
- Empty-text events with attachments are now forwarded (previously dropped)
- Tests: 11 new (parse, download success/skip/oversized, URL encoding)

Refs: openabdev#731 (Feishu pattern)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(core): STT for Custom Gateway audio attachments

Extends src/gateway.rs attachment handling to transcribe audio attachments
via the existing STT pipeline (previously only Discord/Slack adapters
went through download_and_transcribe; Custom Gateway adapters got no
audio path even though stt::transcribe was available).

When a gateway adapter (Feishu, Google Chat, etc.) sends an Attachment
with attachment_type = "audio", core now:
1. Decodes base64 → audio bytes
2. Calls stt::transcribe with the configured SttConfig
3. Wraps the transcript as a ContentBlock::Text:
   "[Voice message transcript]: ..."

The audio branch is gated on stt_config.enabled — if STT is disabled in
config, audio attachments fall through unchanged (same as before).

Threads stt_config through GatewayParams and run_gateway_adapter.

This closes the audio attachment gap left by the (now-closed) PR openabdev#726
without re-introducing the HTTP MediaStore proxy approach. Pairs with
the Google Chat adapter audio download (separate PR) — once both land,
Google Chat voice/audio attachments work end-to-end.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): address googlechat attachment review feedback

Addresses canyugs#4 must-fix items:

#1+#2 Webhook timeout safety:
- Spawn background tokio task for attachment downloads so the webhook
  returns 200 within Google Chat's 30s deadline regardless of how long
  downloads take.
- Add 30s per-request timeout to all Media API GET calls — a single
  hung connection can no longer stall the download task indefinitely.
- Refactor event emission into send_googlechat_event helper to share
  between sync (no-attachment) and async (background download) paths.

#4 Text file caps (matches Discord/Slack):
- TEXT_FILE_COUNT_CAP = 5: skip text files past the 5th with a warning.
- TEXT_TOTAL_CAP = 1 MB: skip text files that would push the running
  aggregate past 1 MB with a warning.
- Image and audio attachments are not capped (same as Discord/Slack).

openabdev#6 STT silent failure:
- When stt::transcribe returns None, push a fallback ContentBlock::Text
  ("[Voice message — transcription failed for ...]") so the agent
  knows a voice message was attempted and can ask the user to re-send.
  Previously the failure was silent and confusing.

Skipped from issue #4: #3 (streaming download), openabdev#5 (cross-adapter
refactor — adapters stay independent per design), openabdev#7-openabdev#10 (cosmetic).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): correct media_url encoding, remove lossy UTF-8 round-trip, add spawn panic logging

- media_url: preserve `/` as literal path separators per Google Chat
  Media API's RFC 6570 reserved expansion (`{+resourceName}`). Previously
  all `/` were encoded as `%2F` which is fragile.
- download_googlechat_file: base64-encode raw bytes directly instead of
  round-tripping through String::from_utf8_lossy which silently replaces
  invalid bytes with U+FFFD.
- Spawned attachment download task: log an error if the task panics so
  silent message drops are diagnosable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): address review — remove .env from whitelist, add audio decode fallback

- Remove `"env"` from TEXT_EXTS whitelist to prevent credential exposure
  if a user accidentally uploads a .env file.
- Audio base64 decode failure now produces a fallback text block
  ("[Voice message — decode failed for X]") instead of silently dropping.
- Audio attachments when STT is disabled now log at debug level instead
  of being silently discarded.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor(gateway): simplify text file cap logic, defer text allocation to spawn path

- Flatten nested if/else in File download cap check using early
  continue, improving readability.
- Defer text .to_string() allocation to the tokio::spawn path so the
  no-attachment fast path avoids a heap allocation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor(gateway): address remaining review nits

- Replace double-spawn panic logging with single spawn +
  catch_unwind — more idiomatic, same observability.
- Remove unused content_type from Image/File variants of
  GoogleChatMediaRef; only Audio needs it. Drops #[allow(dead_code)]
  on the enum.
- Pass remaining aggregate budget to download_googlechat_file so
  Content-Length is checked against the budget before downloading,
  avoiding wasted bandwidth on files that would exceed the cap.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): enforce aggregate budget on post-download check, log skipped video attachments

- download_googlechat_file: post-download size check now uses max_size
  (min of FILE_MAX_DOWNLOAD and remaining_budget) instead of only
  FILE_MAX_DOWNLOAD, ensuring TEXT_TOTAL_CAP is respected even when
  Content-Length header is absent.
- parse_attachments: video/ MIME type now gets an explicit info! log
  and is skipped early, instead of silently failing the text extension
  whitelist downstream.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: chaodu-agent <chaodu-agent@openab.dev>
chaodu-agent added a commit that referenced this pull request May 23, 2026
@chaodu-agent
feat(operator): oabctl Phase 1 — apply, get, delete (openabdev#851)
4968732 
* feat(operator): add oabctl Phase 1 — apply, get, delete for ECS

Implements the CLI provisioner from ADR docs/adr/ecs-control-plane.md:
- oabctl apply -f <file|dir>: validate manifest, render config to S3,
  register task def, create/update ECS service
- oabctl get oabservice: list services via ECS DescribeServices
- oabctl delete oabservice <name>: teardown ECS service + S3 cleanup
- entrypoint.sh: wrapper script for ECS tasks (bootstrap + config download)

Schema: oab.dev/v1 OABService with capacityProvider, cpu, memory,
bootstrapFrom, networking, config, secrets fields.

* ci: add operator/oabctl build job to CI workflow

* fix(operator): handle AWS SDK builder Result types

* fix(operator): capacity_provider() returns &str not Option

* fix(operator): resolve clippy warnings (is_some_and, useless format)

* fix(operator): inline literal in format string (clippy::print_literal)

* fix(operator): address blocking review feedback

- #1: Read generation from S3 manifest, increment on each apply (immutable config path)
- #2: Remove launch_type (conflicts with capacity_provider_strategy)
- #3: Add generation field to Metadata struct

---------

Co-authored-by: Kiro <kiro@openab.dev>
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

pending-maintainer pending-screening PR awaiting automated screening

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@antigenius0910