feat(gateway): Phase 3 quick fixes — bot filter, fallback, logging, docs by chaodu-agent · Pull Request #1173 · openabdev/openab

chaodu-agent · 2026-06-22T02:57:53Z

Summary

Implements all 7 tasks from #1172 (Phase 3 unified webhook follow-ups):

✅ Add GATEWAY_ALLOW_BOT_MESSAGES / GATEWAY_TRUSTED_BOT_IDS env — Bot messages pass through when allowed or sender is trusted.
✅ Wire Google Chat in unified mode — SA key parsing, JWT verifier, webhook route.
✅ Deduplicate filter logic — Extracted should_skip_event() helper shared by both code paths.
✅ Extract AppState::from_env() factory — Unified mode goes from 60-line inline construction to single method call. Eliminates config divergence.
✅ Document production-must-set env vars — docs/config-reference.md unified mode table.
✅ Add user-facing fallback when create_thread fails — "⚠️ Could not create a thread — replying here instead."
✅ Log when audio attachment path/data both empty — tracing::warn with filename/mime.

…t logging - Add GATEWAY_ALLOW_BOT_MESSAGES / GATEWAY_TRUSTED_BOT_IDS env vars to process_gateway_event(). Bot messages are now passthrough when explicitly allowed or sender is in trusted list. (Issue #1172 task 1) - Send user-facing fallback message when create_thread fails instead of silently falling back to channel. (Issue #1172 task 6) - Log warning when attachment has neither path nor data. (Issue #1172 task 7)

Documents all GATEWAY_* env vars including security gating (ALLOW_ALL_CHANNELS/USERS defaults, ALLOW_BOT_MESSAGES, TRUSTED_BOT_IDS) with production checklist warning. Issue #1172 task 5.

Initialize GoogleChatAdapter with SA key parsing, access token, and JWT verifier from env vars (same as standalone gateway). Register /webhook/googlechat route. Removes the TODO placeholder (google_chat: None). Issue #1172 task 2.

Both process_gateway_event() and run_gateway_adapter() now call should_skip_event() for bot filtering, channel/user allowlist, and @mention gating. Eliminates ~30 lines of duplicated filter logic. Also brings bot_messages/trusted_bot_ids support to the WebSocket path (previously hardcoded to drop all bots). Issue #1172 task 3 (partial — filter logic deduped, attachment processing dedup deferred).

Add AppState::from_env(event_tx, ws_token) factory method to the gateway crate. Unified mode in main.rs now calls this single method instead of inline-constructing AppState (~60 lines → 1 line). This eliminates config divergence risk between unified and standalone modes — both paths now share the same adapter initialization logic. The standalone serve() function will be migrated to use from_env() in a follow-up (its feishu_ws_mode inspection makes it slightly more involved). Issue #1172 task 4.

GatewayParams uses Vec<String> for allowed_channels/allowed_users. Convert once at run_gateway_adapter start rather than per-event.

- Remove empty line between doc comment and function - Change EventFilterParams/should_skip_event from pub(crate) to module-private (GatewayEvent is private)

…allback - F1: Remove per-event empty_set allocation in WS loop; use hoisted trusted_bot_ids from GatewayParams - F2: Add allow_bot_messages/trusted_bot_ids to GatewayParams and GatewayConfig so WS path respects the same env vars as unified mode - F3: Replace silent let _ with tracing::warn on fallback send_message failure in both WS and unified code paths

Move loop-invariant EventFilterParams construction from per-event match arm to before the inner loop. All referenced variables (allowed_channels, allowed_users, trusted_bot_ids, bot_username) are declared before the outer reconnect loop and outlive the filter.

…t filter log - Empty string env var no longer fails open (was: "" != "0" → true) - Add tracing::info when bot is filtered, matching channel/user filter observability

- Add 8 unit tests for should_skip_event() covering all filter branches - Document allow_bot_messages/trusted_bot_ids in [gateway] config section (note: bool type, not AllowBots enum like Discord/Slack) - Fix prefix-less env var names in config-reference Security Gating - Add missing Google Chat env vars (SA_KEY_FILE, ACCESS_TOKEN, WEBHOOK_PATH) - Log warning when GOOGLE_CHAT_SA_KEY_FILE read fails (both paths) - Document that Google Chat JWT is optional when AUDIENCE unset

…k logging - F4: Add doc comment on GatewayConfig.allow_bot_messages explaining intentional bool vs AllowBots difference (mention gating handled by bot_username + should_skip_event separately) - F10: Remove separate fallback warning send_message that preceded the actual reply on ordered-delivery platforms (Telegram/LINE). The create_thread failure is still logged server-side. - F11: Add tracing::warn for image/text_file read failures in webhook path, matching the WS path behavior. - F9: Already addressed — CI has cargo clippy --features unified.

chaodu-agent · 2026-06-22T13:19:40Z

LGTM ✅ — Bot filter, dedup refactor, factory extraction, and docs all well-implemented with comprehensive tests.

What This PR Does

Implements all 7 follow-up tasks from Phase 3 unified webhook integration (#1172): configurable bot filtering with trusted-bot bypass, Google Chat wiring in unified mode, filter logic deduplication, AppState::from_env() factory, production env var documentation, create_thread fallback logging, and attachment error logging.

How It Works

Extracts should_skip_event() with EventFilterParams struct — shared by both WebSocket and webhook code paths, eliminating duplicated gating logic.
AppState::from_env() factory replaces 60-line inline construction, preventing config divergence between standalone and unified mode.
Bot filter defaults to false (fail-closed) with trusted_bot_ids whitelist for multi-agent scenarios.
GATEWAY_ALLOW_BOT_MESSAGES env var correctly treats empty string as false (hardened in cd5eab6).

Findings

#	Severity	Finding	Location
1	🟢	`should_skip_event()` deduplication is correct and well-tested (8 unit tests)	`gateway.rs:57`
2	🟢	`AppState::from_env()` eliminates config divergence between standalone/unified	`openab-gateway/src/lib.rs:49`
3	🟢	Bot filter defaults fail-closed (`false`) — secure by default	`config.rs:458`
4	🟢	`EventFilterParams` hoisted outside WS loop (no per-message allocation)	`gateway.rs:808`
5	🟢	Comprehensive config-reference docs with production checklist callouts	`docs/config-reference.md`
6	🟢	Attachment error logging added to both WS and webhook paths consistently	`gateway.rs:923`, `gateway.rs:1225`

Baseline Check

PR opened: 2026-06-22
Main already has: Gateway event filtering (channel/user allowlists, mention gating) — but duplicated in two code paths, bot messages unconditionally dropped, no Google Chat in unified mode
Net-new value: Configurable bot filter with trusted-ID bypass, deduplicated filter logic, shared AppState factory, Google Chat unified wiring, production env documentation

What's Good (🟢)

Clean separation of concerns — filter logic is testable in isolation
Lifetime annotations on EventFilterParams avoid unnecessary cloning
Tests cover all 5 filter branches (bot, channel, user, mention, thread bypass)
Documentation includes production security checklist with ⚠️ callouts
Commit history shows iterative improvement addressing all reviewer feedback

CI: ✅ All checks green | HEAD: d2b1e67

chaodu-agent added 2 commits June 22, 2026 02:50

docs: add unified mode env vars to config-reference

7bb7308

Documents all GATEWAY_* env vars including security gating (ALLOW_ALL_CHANNELS/USERS defaults, ALLOW_BOT_MESSAGES, TRUSTED_BOT_IDS) with production checklist warning. Issue #1172 task 5.

chaodu-agent requested a review from thepagent as a code owner June 22, 2026 02:57