Skip to content

feat: add channel-relay integration for Slack#790

Merged
PierreLeGuen merged 13 commits intostagingfrom
feat/channel-relay-integration
Mar 10, 2026
Merged

feat: add channel-relay integration for Slack#790
PierreLeGuen merged 13 commits intostagingfrom
feat/channel-relay-integration

Conversation

@PierreLeGuen
Copy link
Copy Markdown
Contributor

@PierreLeGuen PierreLeGuen commented Mar 10, 2026

Summary

  • Add RelayChannel and RelayClient modules for connecting to the external channel-relay service via SSE streams, with automatic reconnection and exponential backoff
  • Add channel-relay extension lifecycle (install/auth/activate) with OAuth redirect flow through the relay service for Slack workspace connections
  • Add proxy message sending through channel-relay for chat.postMessage, with provider-appropriate body building
  • Wire relay channels into app startup for automatic reconnection from stored credentials

Test plan

  • cargo build --release — zero warnings
  • Relay integration test with mock SSE server (tests/relay_integration.rs)
  • E2E: install Slack relay extension, complete OAuth, verify SSE events arrive, send message back through proxy

henrypark133 and others added 2 commits March 9, 2026 23:58
…-check] (#787)

GitHub Actions step-level `if:` doesn't have access to `secrets` context.
Replace `if: secrets.X != ''` with `continue-on-error: true` and let
the Set token step handle the fallback.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add RelayChannel and RelayClient for connecting to channel-relay SSE streams
- Add RelayConfig with env-based configuration (CHANNEL_RELAY_URL, CHANNEL_RELAY_API_KEY)
- Add channel-relay extension lifecycle: install, OAuth auth, activate with hot-add
- Add proxy message sending through channel-relay for Slack chat.postMessage
- Add extension registry entry for Slack relay with OAuth auth hint
- Add relay integration test with mock SSE server
- Wire relay channel into app startup with reconnect on stored credentials
- Add AuthRequired extension error variant for cleaner auth flow detection

[skip-regression-check]
@github-actions github-actions Bot added scope: channel Channel infrastructure scope: channel/web Web gateway channel scope: extensions Extension management scope: ci CI/CD workflows size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: regular 2-5 merged PRs labels Mar 10, 2026
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly expands the application's communication capabilities by integrating with external messaging platforms, starting with Slack, through a dedicated channel-relay service. This architecture offloads the complexities of OAuth, credential management, and real-time event streaming to the relay service, allowing the application to focus on consuming a standardized event stream and proxying messages. The changes encompass new client and channel implementations for the relay, a robust SSE parsing and reconnection mechanism, and comprehensive updates to the extension management system to support the new ChannelRelay extension type, including its authentication and activation flows.

Highlights

  • Slack Integration via Channel-Relay: Introduced a new integration to connect with Slack workspaces through an external channel-relay service, enabling real-time communication via Server-Sent Events (SSE).
  • SSE Stream Handling with Reconnection Logic: Implemented RelayChannel and RelayClient modules to manage SSE streams from the channel-relay service, including automatic reconnection with exponential backoff and robust event parsing.
  • OAuth Flow and Credential Management: Added a lifecycle for channel-relay extensions (install, auth, activate) with an OAuth redirect flow for Slack workspace connections, securely storing stream tokens and team IDs.
  • Proxy Message Sending: Enabled proxying of chat.postMessage calls through the channel-relay service, with provider-appropriate body construction for Slack.
  • Extension Manager and Web UI Updates: Updated the extension manager to recognize and manage ChannelRelay extensions, including their installation, authentication, activation, and display in the web UI.
Changelog
  • src/app.rs
    • Modified catalog_entries to be mutable to allow appending built-in entries.
    • Appended built-in registry entries, including channel-relay integrations, to the catalog for extension discovery.
  • src/channels/mod.rs
    • Added a new public module relay for channel-relay integration.
  • src/channels/relay/channel.rs
    • Added RelayChannel struct implementing the Channel trait for SSE stream integration.
    • Implemented RelayProvider enum to distinguish between messaging providers like Slack.
    • Included logic for connecting to SSE streams, parsing ChannelEvents, and handling reconnections with exponential backoff.
    • Provided methods for building provider-specific message bodies and proxying messages via the RelayClient.
  • src/channels/relay/client.rs
    • Added RelayClient struct for HTTP communication with the channel-relay service.
    • Defined ChannelEvent for parsing incoming SSE events.
    • Implemented methods for initiating OAuth flows, connecting to SSE streams, renewing tokens, proxying API calls, and listing connections.
    • Introduced RelayError enum for structured error handling.
  • src/channels/relay/mod.rs
    • Added module documentation for channel-relay integration.
    • Re-exported DEFAULT_RELAY_NAME, RelayChannel, and RelayClient.
  • src/channels/web/handlers/extensions.rs
    • Updated extensions_list_handler to correctly display the status of ChannelRelay extensions (active, configured, installed).
    • Added ChannelRelay to the ExtensionKind mapping in extensions_install_handler.
    • Removed the old extensions_activate_handler function, as its logic was refactored and moved.
  • src/channels/web/mod.rs
    • Added oauth_rate_limiter to GatewayChannel and GatewayChannelBuilder for rate-limiting OAuth callbacks.
  • src/channels/web/server.rs
    • Imported DEFAULT_RELAY_NAME for use in Slack OAuth callback.
    • Added oauth_rate_limiter to GatewayState for controlling OAuth callback request frequency.
    • Registered a new public route /oauth/slack/callback for handling Slack relay OAuth redirects.
    • Implemented slack_relay_oauth_callback_handler to process Slack OAuth responses, store stream tokens, and activate the relay channel.
    • Refactored extensions_activate_handler to use the new ExtensionError::AuthRequired variant for clearer authentication failure handling.
  • src/channels/web/static/app.js
    • Updated renderExtensionCard to display an 'Activate' button for channel_relay extensions when they are installed but inactive.
  • src/channels/web/test_helpers.rs
    • Added oauth_rate_limiter initialization to TestGatewayBuilder.
  • src/channels/web/ws.rs
    • Added oauth_rate_limiter to GatewayState initialization in test setup.
  • src/config/mod.rs
    • Added relay module to configuration.
    • Exported RelayConfig.
    • Included an optional relay field in the main Config struct.
    • Initialized relay field from environment variables in Config::from_env.
  • src/config/relay.rs
    • Added RelayConfig struct to define settings for the channel-relay service.
    • Implemented from_env to load configuration from environment variables, making the integration opt-in.
    • Provided from_values for test setup and from_env_reader for internal testing.
    • Ensured API key is redacted in debug output.
  • src/extensions/discovery.rs
    • Added ChannelRelay to the extract_source function for proper URL extraction.
  • src/extensions/manager.rs
    • Added relay_channel_manager and installed_relay_extensions fields to ExtensionManager.
    • Implemented set_relay_channel_manager to allow hot-adding relay channels.
    • Provided a public secrets accessor for OAuth callback handlers.
    • Updated install_extension to handle ChannelRelay extensions by marking them as installed without downloading.
    • Extended auth and activate methods to dispatch to specific auth_channel_relay and activate_channel_relay functions.
    • Modified list_installed_extensions to include ChannelRelay extensions, checking for active status and stored tokens.
    • Updated remove_extension to handle ChannelRelay extensions by removing them from internal tracking, active channels, and deleting stored tokens.
    • Adjusted upgrade_extension and get_extension_info to account for ChannelRelay extensions.
    • Added auth_channel_relay to initiate OAuth for Slack via the relay service.
    • Added activate_channel_relay to load and register a RelayChannel instance with the channel manager.
    • Added activate_stored_relay for activating relay channels from persisted credentials during startup.
    • Updated determine_installed_kind to correctly identify ChannelRelay extensions based on in-memory state or stored secrets.
  • src/extensions/mod.rs
    • Added ChannelRelay variant to ExtensionKind enum.
    • Added ChannelRelay variant to ExtensionSource enum.
    • Added ChannelRelayOAuth variant to AuthHint enum.
    • Introduced AuthRequired error variant to ExtensionError for clearer authentication failures.
  • src/extensions/registry.rs
    • Modified builtin_entries to conditionally add ChannelRelay entries (e.g., Slack) only if CHANNEL_RELAY_URL is configured in the environment.
  • src/main.rs
    • Ensured the relay_channel_manager is always set in the extension manager, even if the WASM runtime is not available.
  • tests/openai_compat_integration.rs
    • Added oauth_rate_limiter to GatewayState initialization in test helper functions.
  • tests/relay_integration.rs
    • Added new integration tests for RelayClient functionality.
    • Included tests for SSE stream event reception, token expiration and renewal, proxying Slack API calls, listing connections, and API key header transmission.
  • tests/ws_gateway_integration.rs
    • Added oauth_rate_limiter to GatewayState initialization in test helper functions.
Ignored Files
  • Ignored by pattern: .github/workflows/** (1)
    • .github/workflows/staging-ci.yml
Activity
  • The pull request introduces significant new functionality for integrating with external messaging platforms via a channel-relay service.
  • A comprehensive test plan is outlined, including cargo build --release for zero warnings, a relay_integration.rs test with a mock SSE server, and an end-to-end test for Slack relay extension installation, OAuth, SSE event verification, and message proxying.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a major new feature: Slack integration via a channel-relay service. However, a significant security flaw was identified in the OAuth callback handling, specifically a CSRF vulnerability due to the lack of a state parameter. This could allow an attacker to link their own Slack workspace to a victim's IronClaw instance, granting unauthorized access. Additionally, there are other issues including a potential resource leak in the channel's background task management, leftover test code that will fail to compile, and opportunities to reduce code duplication.

I am having trouble creating individual review comments. Click here to see my feedback.

src/channels/web/server.rs (621-774)

security-high high

The slack_relay_oauth_callback_handler endpoint, a public route for OAuth callbacks, lacks a state parameter for CSRF protection. This critical vulnerability could allow an attacker to link their own Slack workspace to a victim's IronClaw instance, leading to unauthorized access and potential data exfiltration.

Remediation:

  1. In src/extensions/manager.rs, modify auth_channel_relay to generate a random nonce (state) and store it in a pending flow registry.
  2. Append the nonce to the callback_url as a query parameter.
  3. In src/channels/web/server.rs, modify slack_relay_oauth_callback_handler to accept a state parameter and verify it against the stored nonce before proceeding.

Additionally, the validation logic within this handler for stream_token, team_id, and provider contains duplicated code for generating the "Invalid callback parameters" HTML error response. Consider extracting this response generation into a helper function to improve conciseness and maintainability.

src/channels/relay/channel.rs (59)

high

The current implementation of the reconnect logic for the SSE stream can lead to a leaked parser task. The parser_handle on RelayChannel stores the initial parser task handle, but it's not updated upon reconnection. The spawned task for handling reconnections creates its own local parser_handle which is updated, but the shutdown method doesn't have access to it. This means if a reconnect occurs, the new parser task will not be aborted on shutdown.

To fix this, parser_handle in the RelayChannel struct should be an Arc<RwLock<Option<tokio::task::JoinHandle<()>>>>. This allows it to be shared with and updated by the reconnect task, ensuring shutdown can always abort the current parser task. You'll need to update the constructor new_with_provider to initialize it with Arc::new(RwLock::new(None)) and then clone it into the start method's spawned task.

    parser_handle: Arc<RwLock<Option<tokio::task::JoinHandle<()>>>>,

src/channels/relay/channel.rs (503-583)

high

These tests for Telegram (telegram_conversation_context_platform, build_send_body_telegram, build_send_body_telegram_no_thread) will fail to compile because RelayProvider::Telegram is not defined in the RelayProvider enum. Since Telegram support doesn't seem to be part of this pull request, this test code should be removed.

src/channels/relay/client.rs (252-285)

medium

The proxy_slack method appears to be unused. The more generic proxy_provider is used by RelayChannel instead. To improve maintainability and reduce dead code, consider removing the proxy_slack method.

Copy link
Copy Markdown
Collaborator

@zmanian zmanian left a comment

Choose a reason for hiding this comment

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

Review: feat: add channel-relay integration for Slack

Large PR (2604 lines, 21 files) adding full Slack relay channel support via an external relay service. Solid architecture overall with SSE streaming, reconnection with exponential backoff, and OAuth flow.

Design concerns:

  1. Overlap with #778 -- PR #778 also adds a webhook proxy for Slack messaging. These two PRs appear to solve overlapping problems (Slack integration). Need to clarify the relationship -- is #790 the replacement, or do they coexist? If they coexist, document the distinction.

  2. No tests for RelayChannel -- The relay/channel.rs file has unit tests for the client but I don't see integration-level tests for the Channel trait implementation (start, respond, reconnect flow). The reconnect logic is complex enough to warrant at least a mock-based test.

  3. Token stored in memory -- stream_token: Arc<RwLock<String>> stores the relay auth token in memory. This is fine for now but means token renewal state is lost on restart. If the relay service issues short-lived tokens, this could cause startup failures after hibernation.

  4. Hardcoded Slack-isms -- RelayProvider enum currently only has Slack. The build_send_body match and channel_name() method are good abstractions, but event_type checks like "direct_message" and "channel_message" are Slack-specific string constants scattered through the code. Consider defining these as constants on the provider.

  5. Missing event_type in metadata -- In the start() method, the IncomingMessage metadata JSON includes team_id, channel_id, sender_id, thread_id, provider but NOT event_type. However, PR #796 (approval buttons) depends on metadata.get("event_type") to determine DM vs channel context. This needs to be added.

  6. Error handling in reconnect loop -- When both reconnect and token renewal fail, the loop continues with the old (potentially expired) token. After N consecutive failures, consider stopping the channel and surfacing an error to the user rather than silently retrying forever.

Minor:

  • Good use of exponential backoff with max ceiling
  • Clean separation of RelayClient (HTTP) vs RelayChannel (Channel trait)
  • with_timeouts() builder is a nice touch for testability

The main blockers are the missing event_type in metadata (breaks #796) and lack of tests for the reconnect flow.

…uit breaker

- Fix parser handle leak on reconnect by sharing Arc<RwLock> instead of
  creating a local copy in start() (shutdown now aborts the correct task)
- Add CSRF state nonce to OAuth flow: generate in auth_channel_relay,
  validate in slack_relay_oauth_callback_handler, one-time use
- Remove dead proxy_slack method, update integration test to use
  proxy_provider
- Add reconnect circuit breaker (max_consecutive_failures, default 50)
- Fix stale docs (Telegram refs), extract event_types constants
@PierreLeGuen PierreLeGuen enabled auto-merge (squash) March 10, 2026 18:59
})?;

Ok(())
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

High Severity — Double backoff in reconnect loop

When both the SSE reconnect and the subsequent list_connections check fail in the same loop iteration, backoff_ms is doubled and slept on twice:

  1. Line ~334-335: sleep(backoff_ms) then backoff_ms = (backoff_ms * 2).min(backoff_max_ms)
  2. Line ~405-406 (in the list_connections Err branch): sleep(backoff_ms) then backoff_ms = (backoff_ms * 2).min(backoff_max_ms)

This means:

  • Two sleeps per iteration instead of one
  • Backoff grows as O(4^n) instead of O(2^n): 1s → 4s → 16s → 64s (max) instead of 1s → 2s → 4s → 8s
  • After just 3 consecutive failures, the combined sleep per iteration is already ~20s instead of ~4s

Suggested fix: Remove the second sleep+backoff in the list_connections error branch. The connection validity check failing shouldn't compound the reconnect delay — just log the warning and let the next loop iteration handle it with the normal backoff.

Err(e) => {
    tracing::warn!(
        error = %e,
        "Could not verify team connection, will retry next iteration"
    );
    // Don't sleep or increment backoff again — the main loop backoff handles it
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in previous push — removed the second sleep+backoff in the list_connections error branch. Now only the reconnect delay applies.

Comment thread src/channels/relay/client.rs Outdated
buffer.push_str(&String::from_utf8_lossy(&chunk));

// Process complete lines
while let Some(newline_pos) = buffer.find('\n') {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Medium Severity — UTF-8 corruption from chunk-boundary splitting

String::from_utf8_lossy(&chunk) can corrupt multi-byte UTF-8 characters when a network chunk boundary falls mid-character. For example, an emoji (4 bytes) split across two chunks will produce two U+FFFD replacement characters instead of the original emoji.

This will cause real data loss for Slack messages containing emoji, CJK characters, accented characters, etc. — which are very common in Slack.

Suggested fix: Buffer raw bytes instead of converting each chunk to a string immediately:

let mut buffer = Vec::<u8>::new();
// ...
buffer.extend_from_slice(&chunk);

// Process complete lines from the byte buffer
while let Some(newline_pos) = buffer.iter().position(|&b| b == b'\n') {
    let line_bytes = &buffer[..newline_pos];
    let line = String::from_utf8_lossy(line_bytes).trim_end_matches('\r').to_string();
    buffer.drain(..=newline_pos);
    // ... process line as before
}

This ensures UTF-8 decoding only happens on complete lines, avoiding mid-character splits.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — changed buffer from String to Vec<u8>, buffering raw bytes and only decoding complete newline-delimited lines via String::from_utf8_lossy. Added test parse_sse_handles_multibyte_utf8_across_chunks that splits a multi-byte emoji across chunk boundaries.

…tion

- Remove second sleep+backoff in list_connections error branch to prevent
  O(4^n) backoff growth (was sleeping and doubling twice per iteration)
- Buffer raw bytes in SSE parser instead of per-chunk String::from_utf8_lossy
  to prevent U+FFFD corruption when multi-byte chars span chunk boundaries
Copy link
Copy Markdown
Collaborator

@zmanian zmanian left a comment

Choose a reason for hiding this comment

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

Re-review: feat: add channel-relay integration for Slack

Good progress since last review. The parser handle leak fix, CSRF state nonce, circuit breaker, double-backoff fix, and UTF-8 chunk-boundary fix all look correct. Tests for CSRF are solid.

Remaining issues

1. event_type still missing from IncomingMessage metadata (from previous review, not addressed)

In src/channels/relay/channel.rs, the metadata JSON does not include event_type:

.with_metadata(serde_json::json!({
    "team_id": event.team_id(),
    "channel_id": event.channel_id,
    "sender_id": event.sender_id,
    "thread_id": event.thread_id,
    "provider": event.provider,
}))

Add "event_type": event.event_type to the metadata. Downstream code (and future PRs for approval buttons / DM-vs-channel routing) will need this to distinguish direct_message from message from mention.

2. Auto-install side-effect in determine_installed_kind is too permissive

determine_installed_kind (manager.rs) writes to installed_relay_extensions as a side-effect of merely looking up what kind an extension is. This means calling auth("slack-relay", None) or activate("slack-relay") for a name that has a ChannelRelay registry entry will silently install it without the user going through the install flow. The install step should be explicit -- determine_installed_kind should only read state, never write it.

Suggestion: remove the auto-insertion into installed_relay_extensions from determine_installed_kind. If the extension isn't installed, return NotInstalled and let the user install it through the normal flow.

3. RelayConfig::from_env() called multiple times at runtime

auth_channel_relay and activate_channel_relay both call RelayConfig::from_env() at invocation time rather than using a config stored at startup. This means:

  • If env vars change between auth and activate, the relay URL could differ
  • The config is re-parsed on every call

The relay config should be passed in at construction or stored once. The Config struct already has relay: Option<RelayConfig> -- use it rather than re-reading env vars.

4. builtin_entries() reads env var at call time

builtin_entries() in registry.rs reads CHANNEL_RELAY_URL from the environment. This function is called during app startup via app.rs, which is fine, but it's also potentially callable in other contexts. Since builtin_entries is now pub, document that it reads from the environment, or better, accept the relay URL as a parameter.

5. OAuth callback error message leaks nothing sensitive -- but could be more specific in logs

The slack_relay_oauth_callback_handler returns generic error HTML to the user (good), but the tracing::error! on activation failure includes %e which could contain relay service internals. Consider redacting the error detail or at minimum ensure the relay service doesn't return secrets in error bodies.

Minor / non-blocking

  • The stream_token is passed as a query parameter to /stream?token=X. If the relay service uses HTTPS this is fine, but query params end up in server access logs. Consider using an Authorization header instead for the stream endpoint. (Non-blocking, depends on relay service API.)

  • conversation_context checks metadata.get("sender_name") but the metadata constructed in start() doesn't include sender_name -- it uses sender_id. The sender_name field would need to be added to metadata for conversation_context to populate sender.

  • Good: CSRF nonce is one-time use, rate limiting on OAuth callback, proper input validation on team_id format, circuit breaker with configurable max failures, proper shutdown of both parser and reconnect handles.

Verdict

Two items need fixing before merge: (1) missing event_type in metadata and (2) the auto-install side-effect in determine_installed_kind. Items 3-4 are strong recommendations but could be addressed in a follow-up.

- Add event_type and sender_name to IncomingMessage metadata (blocker)
- Remove auto-install side-effect from determine_installed_kind (blocker)
- Store RelayConfig at startup instead of re-reading env vars each call
- Add builtin_entries_with_relay() for env-free registry construction
- Fix test_ext_mgr missing mcp_process_manager arg
@PierreLeGuen
Copy link
Copy Markdown
Contributor Author

Status: All re-review items addressed

Pushed bece363 addressing all items from @zmanian's re-review:

Blockers (fixed)

  1. event_type missing from metadata — Added event_type and sender_name to the IncomingMessage metadata JSON in channel.rs. This enables downstream DM-vs-channel routing and populates conversation_context().

  2. Auto-install side-effect in determine_installed_kind — Removed the two code paths that wrote to installed_relay_extensions as a side-effect of lookup. The method is now read-only. If a relay has a stored stream_token, it returns ChannelRelay without mutating state. The registry-entry auto-install path is removed entirely. Added regression test test_determine_installed_kind_does_not_auto_install_relay.

Strong recommendations (fixed)

  1. RelayConfig::from_env() called multiple times — Added relay_config: Option<RelayConfig> field to ExtensionManager, captured once at construction. auth_channel_relay and activate_channel_relay now use self.relay_config() instead of re-reading env vars.

  2. builtin_entries() reads env var — Added builtin_entries_with_relay(relay_url: Option<String>) that accepts the URL as a parameter. The existing builtin_entries() delegates to it (backward compatible). Added tests for both None and Some cases.

Also fixed

  • test_ext_mgr in server.rs was missing mcp_process_manager arg after upstream refactor

Review items 5 (log redaction) and minor items

  • Item 5 (error detail in logs): The relay service error bodies are already generic ("OAuth initiation failed", "Connection refused", etc.) — no secrets leak. Left as-is.
  • sender_name in metadata: now included (was missing, conversation_context couldn't populate sender).
  • stream_token as query param: acknowledged as non-blocking, relay service uses HTTPS.

Test results

  • cargo clippy --all --all-features — zero warnings
  • cargo clippy --no-default-features --features libsql --all --tests — zero warnings
  • 27 relay-related tests pass (26 unit + 1 integration)

Copy link
Copy Markdown
Collaborator

@zmanian zmanian left a comment

Choose a reason for hiding this comment

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

Found several blocking regressions in the relay integration.

  • src/main.rs:549-589 only replays persisted active channels inside the WASM-runtime branch. In a relay-only deployment, that branch never runs, so slack-relay does not come back after restart even though the relay channel manager is initialized later.
  • src/main.rs:571 plus src/extensions/manager.rs:578-610 / 3304-3310 use activate(name) on startup instead of activate_stored_relay(name). That can leave the relay actively consuming messages while disappearing from /api/extensions, because installed_relay_extensions is never repopulated.
  • src/channels/web/server.rs:760-765 and src/extensions/manager.rs:3233-3244 only persist/read team_id through the settings store. When no store exists, activation falls back to "", and the first reconnect/token-renew cycle in src/channels/relay/channel.rs:343-350 decides the workspace is disconnected and stops the channel.
  • src/extensions/manager.rs:3278-3281 has no already-active guard before hot_add(). Re-activating the same relay spawns another live SSE consumer/forwarder while only replacing the map entry, which can duplicate inbound events and replies.
  • src/extensions/manager.rs:765-770 only shuts a relay down through self.channel_runtime. In relay-only mode there is no WASM runtime, so Remove clears state but leaves the hot-added relay channel and its reconnect task alive until process restart.

- Relay channels now restored on startup in relay-only mode (no WASM runtime)
  via new restore_relay_channels() called unconditionally after set_relay_channel_manager
- WASM block loop now skips relay channels; relay restoration uses activate_stored_relay()
  so installed_relay_extensions is correctly populated for /api/extensions visibility
- Empty team_id (no DB store) no longer kills the channel — reconnect loop
  skips team validation when team_id is empty
- hot_add() shuts down existing channel before replacing to prevent
  duplicate SSE consumers from running in parallel
- remove() now checks relay_channel_manager for shutdown (not just channel_runtime),
  fixing relay-only mode where WASM runtime is absent
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: tool Tool infrastructure scope: tool/builtin Built-in tools scope: tool/wasm WASM tool sandbox scope: tool/mcp MCP client scope: tool/builder Dynamic tool builder scope: safety Prompt injection defense scope: llm LLM integration scope: worker Container worker scope: setup Onboarding / setup scope: docs Documentation scope: dependencies Dependency updates risk: high Safety, secrets, auth, or critical infrastructure and removed risk: medium Business logic, config, or moderate-risk modules labels Mar 10, 2026
…-integration

# Conflicts:
#	src/extensions/manager.rs
@PierreLeGuen PierreLeGuen force-pushed the feat/channel-relay-integration branch from fdb67ff to aa25f85 Compare March 10, 2026 21:55
@github-actions github-actions Bot added risk: medium Business logic, config, or moderate-risk modules and removed risk: high Safety, secrets, auth, or critical infrastructure labels Mar 10, 2026
Copy link
Copy Markdown
Collaborator

@zmanian zmanian left a comment

Choose a reason for hiding this comment

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

Re-review after new commit (formatting only). All code-level feedback from previous review addressed:

  1. PR #778 overlap -- Moot, #778 was a different PR (refactor) and is already merged
  2. Reconnect tests -- Added: 9 integration tests + unit tests covering SSE stream, token renewal, multi-byte UTF-8 chunks
  3. Token persistence -- Tokens stored in secrets store, activate_stored_relay() handles restart recovery
  4. Hardcoded Slack-isms -- Event types extracted to pub mod event_types constants with consistency test
  5. Missing event_type in metadata -- Added with metadata_shape_includes_event_type_and_sender_name regression test
  6. Infinite retry -- max_consecutive_failures (default 50) with builder and tests
  7. Double backoff bug -- Fixed (removed duplicate sleep)
  8. UTF-8 chunk corruption -- Fixed (buffer as Vec, decode on complete lines, regression test)

Solid work. Approved.

@PierreLeGuen PierreLeGuen merged commit b0214fe into staging Mar 10, 2026
14 checks passed
@PierreLeGuen PierreLeGuen deleted the feat/channel-relay-integration branch March 10, 2026 23:34
zmanian pushed a commit that referenced this pull request Mar 12, 2026
* feat: add channel-relay integration for Slack via external relay service

- Add RelayChannel and RelayClient for connecting to channel-relay SSE streams
- Add RelayConfig with env-based configuration (CHANNEL_RELAY_URL, CHANNEL_RELAY_API_KEY)
- Add channel-relay extension lifecycle: install, OAuth auth, activate with hot-add
- Add proxy message sending through channel-relay for Slack chat.postMessage
- Add extension registry entry for Slack relay with OAuth auth hint
- Add relay integration test with mock SSE server
- Wire relay channel into app startup with reconnect on stored credentials
- Add AuthRequired extension error variant for cleaner auth flow detection

[skip-regression-check]

* chore: apply cargo fmt

* fix: remove remaining Telegram test references in relay channel

* fix: address PR #790 review feedback — parser handle leak, CSRF, circuit breaker

- Fix parser handle leak on reconnect by sharing Arc<RwLock> instead of
  creating a local copy in start() (shutdown now aborts the correct task)
- Add CSRF state nonce to OAuth flow: generate in auth_channel_relay,
  validate in slack_relay_oauth_callback_handler, one-time use
- Remove dead proxy_slack method, update integration test to use
  proxy_provider
- Add reconnect circuit breaker (max_consecutive_failures, default 50)
- Fix stale docs (Telegram refs), extract event_types constants

* fix: double backoff in reconnect loop and UTF-8 chunk-boundary corruption

- Remove second sleep+backoff in list_connections error branch to prevent
  O(4^n) backoff growth (was sleeping and doubling twice per iteration)
- Buffer raw bytes in SSE parser instead of per-chunk String::from_utf8_lossy
  to prevent U+FFFD corruption when multi-byte chars span chunk boundaries

* feat: add Slack approval buttons for tool execution in DMs

Send Block Kit Approve/Deny buttons via relay when a tool requires
approval in a DM context. Auto-deny approval-requiring tools in
shared channels to prevent prompt injection and stuck threads.

* fix: address PR #796 review — use PreflightOutcome::Rejected, add tests

- Auto-deny in non-DM relay channels now uses PreflightOutcome::Rejected
  instead of manually pushing to reason_ctx.messages, so the post-flight
  handler properly records the error in the turn
- Add regression tests for relay auto-deny decision logic
- Remove test_clean.db artifact

* feat: restore Block Kit approval buttons in send_status

The send_status implementation was accidentally dropped during the
staging merge. Restores Approve/Deny Block Kit buttons for DM tool
approval, with required sender_id validation, payload size docs,
and 4 regression tests. Also removes test_clean.db.

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

* fix: apply rustfmt formatting to dispatcher test code

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
* fix(ci): secrets can't be used in step if conditions [skip-regression-check] (nearai#787)

GitHub Actions step-level `if:` doesn't have access to `secrets` context.
Replace `if: secrets.X != ''` with `continue-on-error: true` and let
the Set token step handle the fallback.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add channel-relay integration for Slack via external relay service

- Add RelayChannel and RelayClient for connecting to channel-relay SSE streams
- Add RelayConfig with env-based configuration (CHANNEL_RELAY_URL, CHANNEL_RELAY_API_KEY)
- Add channel-relay extension lifecycle: install, OAuth auth, activate with hot-add
- Add proxy message sending through channel-relay for Slack chat.postMessage
- Add extension registry entry for Slack relay with OAuth auth hint
- Add relay integration test with mock SSE server
- Wire relay channel into app startup with reconnect on stored credentials
- Add AuthRequired extension error variant for cleaner auth flow detection

[skip-regression-check]

* chore: apply cargo fmt

* fix: remove remaining Telegram test references in relay channel

* fix: address PR nearai#790 review feedback — parser handle leak, CSRF, circuit breaker

- Fix parser handle leak on reconnect by sharing Arc<RwLock> instead of
  creating a local copy in start() (shutdown now aborts the correct task)
- Add CSRF state nonce to OAuth flow: generate in auth_channel_relay,
  validate in slack_relay_oauth_callback_handler, one-time use
- Remove dead proxy_slack method, update integration test to use
  proxy_provider
- Add reconnect circuit breaker (max_consecutive_failures, default 50)
- Fix stale docs (Telegram refs), extract event_types constants

---------

Co-authored-by: Henry Park <henrypark133@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
* feat: add channel-relay integration for Slack via external relay service

- Add RelayChannel and RelayClient for connecting to channel-relay SSE streams
- Add RelayConfig with env-based configuration (CHANNEL_RELAY_URL, CHANNEL_RELAY_API_KEY)
- Add channel-relay extension lifecycle: install, OAuth auth, activate with hot-add
- Add proxy message sending through channel-relay for Slack chat.postMessage
- Add extension registry entry for Slack relay with OAuth auth hint
- Add relay integration test with mock SSE server
- Wire relay channel into app startup with reconnect on stored credentials
- Add AuthRequired extension error variant for cleaner auth flow detection

[skip-regression-check]

* chore: apply cargo fmt

* fix: remove remaining Telegram test references in relay channel

* fix: address PR nearai#790 review feedback — parser handle leak, CSRF, circuit breaker

- Fix parser handle leak on reconnect by sharing Arc<RwLock> instead of
  creating a local copy in start() (shutdown now aborts the correct task)
- Add CSRF state nonce to OAuth flow: generate in auth_channel_relay,
  validate in slack_relay_oauth_callback_handler, one-time use
- Remove dead proxy_slack method, update integration test to use
  proxy_provider
- Add reconnect circuit breaker (max_consecutive_failures, default 50)
- Fix stale docs (Telegram refs), extract event_types constants

* fix: double backoff in reconnect loop and UTF-8 chunk-boundary corruption

- Remove second sleep+backoff in list_connections error branch to prevent
  O(4^n) backoff growth (was sleeping and doubling twice per iteration)
- Buffer raw bytes in SSE parser instead of per-chunk String::from_utf8_lossy
  to prevent U+FFFD corruption when multi-byte chars span chunk boundaries

* feat: add Slack approval buttons for tool execution in DMs

Send Block Kit Approve/Deny buttons via relay when a tool requires
approval in a DM context. Auto-deny approval-requiring tools in
shared channels to prevent prompt injection and stuck threads.

* fix: address PR nearai#796 review — use PreflightOutcome::Rejected, add tests

- Auto-deny in non-DM relay channels now uses PreflightOutcome::Rejected
  instead of manually pushing to reason_ctx.messages, so the post-flight
  handler properly records the error in the turn
- Add regression tests for relay auto-deny decision logic
- Remove test_clean.db artifact

* feat: restore Block Kit approval buttons in send_status

The send_status implementation was accidentally dropped during the
staging merge. Restores Approve/Deny Block Kit buttons for DM tool
approval, with required sender_id validation, payload size docs,
and 4 regression tests. Also removes test_clean.db.

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

* fix: apply rustfmt formatting to dispatcher test code

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
* fix(ci): secrets can't be used in step if conditions [skip-regression-check] (nearai#787)

GitHub Actions step-level `if:` doesn't have access to `secrets` context.
Replace `if: secrets.X != ''` with `continue-on-error: true` and let
the Set token step handle the fallback.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add channel-relay integration for Slack via external relay service

- Add RelayChannel and RelayClient for connecting to channel-relay SSE streams
- Add RelayConfig with env-based configuration (CHANNEL_RELAY_URL, CHANNEL_RELAY_API_KEY)
- Add channel-relay extension lifecycle: install, OAuth auth, activate with hot-add
- Add proxy message sending through channel-relay for Slack chat.postMessage
- Add extension registry entry for Slack relay with OAuth auth hint
- Add relay integration test with mock SSE server
- Wire relay channel into app startup with reconnect on stored credentials
- Add AuthRequired extension error variant for cleaner auth flow detection

[skip-regression-check]

* chore: apply cargo fmt

* fix: remove remaining Telegram test references in relay channel

* fix: address PR nearai#790 review feedback — parser handle leak, CSRF, circuit breaker

- Fix parser handle leak on reconnect by sharing Arc<RwLock> instead of
  creating a local copy in start() (shutdown now aborts the correct task)
- Add CSRF state nonce to OAuth flow: generate in auth_channel_relay,
  validate in slack_relay_oauth_callback_handler, one-time use
- Remove dead proxy_slack method, update integration test to use
  proxy_provider
- Add reconnect circuit breaker (max_consecutive_failures, default 50)
- Fix stale docs (Telegram refs), extract event_types constants

---------

Co-authored-by: Henry Park <henrypark133@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
* feat: add channel-relay integration for Slack via external relay service

- Add RelayChannel and RelayClient for connecting to channel-relay SSE streams
- Add RelayConfig with env-based configuration (CHANNEL_RELAY_URL, CHANNEL_RELAY_API_KEY)
- Add channel-relay extension lifecycle: install, OAuth auth, activate with hot-add
- Add proxy message sending through channel-relay for Slack chat.postMessage
- Add extension registry entry for Slack relay with OAuth auth hint
- Add relay integration test with mock SSE server
- Wire relay channel into app startup with reconnect on stored credentials
- Add AuthRequired extension error variant for cleaner auth flow detection

[skip-regression-check]

* chore: apply cargo fmt

* fix: remove remaining Telegram test references in relay channel

* fix: address PR nearai#790 review feedback — parser handle leak, CSRF, circuit breaker

- Fix parser handle leak on reconnect by sharing Arc<RwLock> instead of
  creating a local copy in start() (shutdown now aborts the correct task)
- Add CSRF state nonce to OAuth flow: generate in auth_channel_relay,
  validate in slack_relay_oauth_callback_handler, one-time use
- Remove dead proxy_slack method, update integration test to use
  proxy_provider
- Add reconnect circuit breaker (max_consecutive_failures, default 50)
- Fix stale docs (Telegram refs), extract event_types constants

* fix: double backoff in reconnect loop and UTF-8 chunk-boundary corruption

- Remove second sleep+backoff in list_connections error branch to prevent
  O(4^n) backoff growth (was sleeping and doubling twice per iteration)
- Buffer raw bytes in SSE parser instead of per-chunk String::from_utf8_lossy
  to prevent U+FFFD corruption when multi-byte chars span chunk boundaries

* feat: add Slack approval buttons for tool execution in DMs

Send Block Kit Approve/Deny buttons via relay when a tool requires
approval in a DM context. Auto-deny approval-requiring tools in
shared channels to prevent prompt injection and stuck threads.

* fix: address PR nearai#796 review — use PreflightOutcome::Rejected, add tests

- Auto-deny in non-DM relay channels now uses PreflightOutcome::Rejected
  instead of manually pushing to reason_ctx.messages, so the post-flight
  handler properly records the error in the turn
- Add regression tests for relay auto-deny decision logic
- Remove test_clean.db artifact

* feat: restore Block Kit approval buttons in send_status

The send_status implementation was accidentally dropped during the
staging merge. Restores Approve/Deny Block Kit buttons for DM tool
approval, with required sender_id validation, payload size docs,
and 4 regression tests. Also removes test_clean.db.

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

* fix: apply rustfmt formatting to dispatcher test code

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: regular 2-5 merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: channel/web Web gateway channel scope: channel Channel infrastructure scope: ci CI/CD workflows scope: dependencies Dependency updates scope: docs Documentation scope: extensions Extension management scope: llm LLM integration scope: safety Prompt injection defense scope: setup Onboarding / setup scope: tool/builder Dynamic tool builder scope: tool/builtin Built-in tools scope: tool/mcp MCP client scope: tool/wasm WASM tool sandbox scope: tool Tool infrastructure scope: worker Container worker size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants