Skip to content

fix(extensions): channel-relay auth dead-end, observability, and URL override#1681

Merged
PierreLeGuen merged 5 commits intostagingfrom
fix/channel-relay-auth-and-observability
Mar 26, 2026
Merged

fix(extensions): channel-relay auth dead-end, observability, and URL override#1681
PierreLeGuen merged 5 commits intostagingfrom
fix/channel-relay-auth-and-observability

Conversation

@PierreLeGuen
Copy link
Copy Markdown
Contributor

Summary

  • Bug fix: Clicking Activate on the Slack relay extension produced a dead-end "Authentication required" with no OAuth URL. auth_channel_relay() was using is_relay_channel() which returns true from the in-memory installed set (before OAuth), short-circuiting the OAuth flow. Now uses has_stored_team_id() which only checks the persistent store.
  • Observability: Added debug/warn/info tracing across all channel-relay code paths — activation, auth, OAuth callback, relay client HTTP calls, and settings store reads. Previously most failures were silent (.ok(), let _, .unwrap_or_default()).
  • Per-extension relay URL override: Users can now override CHANNEL_RELAY_URL via Settings > Extensions > Reconfigure without restarting the instance.

Test plan

  • cargo clippy --all --benches --tests --examples --all-features — zero warnings
  • cargo test relay — 38 tests pass
  • New regression test: test_auth_channel_relay_installed_without_team_id_is_not_authenticated
  • Manual: Install Slack relay → Activate → verify OAuth URL is returned (not dead-end error)
  • Manual: Set a custom relay URL via Reconfigure → verify it is used in activation logs

🤖 Generated with Claude Code

…elay URL override

Fix a bug where clicking Activate on the Slack relay extension produces
a dead-end "Authentication required" error with no OAuth URL. The root
cause: `auth_channel_relay()` used `is_relay_channel()` to check auth
status, but that function returns true as soon as the extension is
*installed* (in-memory set), before OAuth completes. This short-circuits
the OAuth flow so the authorization URL is never offered.

Changes:

1. **Bug fix** — `auth_channel_relay()` now uses `has_stored_team_id()`
   which only checks the persistent settings store for an actual team_id.
   The extension list `authenticated` field uses the same check so the UI
   accurately reflects OAuth completion status.

2. **Observability** — Added debug/warn/info tracing to all channel-relay
   code paths that were previously silent on failure:
   - `activate_channel_relay`: team_id retrieval, relay config, signing
     secret fetch, hot_add, cache operations
   - `auth_channel_relay`: auth check, OAuth initiation, nonce storage
   - `extensions_activate_handler`: request entry, auth fallback flow
   - `slack_relay_oauth_callback_handler`: team_id persistence (was
     silently ignored with `let _`)
   - `RelayClient`: initiate_oauth, get_signing_secret, proxy_provider
     all log URL, status, and errors
   - `has_stored_team_id`: store read success/failure

3. **Per-extension relay URL override** — Users can now override the
   CHANNEL_RELAY_URL via Settings > Extensions > Reconfigure. Stored
   under `extensions.{name}.relay_url` in settings. Both auth and
   activate read this override before falling back to the env default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: channel/web Web gateway channel scope: extensions Extension management size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: experienced 6-19 merged PRs labels Mar 26, 2026
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 comprehensive tracing and improved error handling across the relay client and extension manager. Key changes include the ability to override the relay URL on a per-extension basis via settings, and a shift from checking in-memory state to persistent storage for verifying relay authentication status. The slack_relay_oauth_callback_handler now explicitly handles and logs errors when persisting the team_id. Additionally, a regression test was added to ensure that installed relay extensions are not incorrectly marked as authenticated if the team_id is missing from storage. I have no feedback to provide.

Comment thread src/channels/web/server.rs Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f54f15d14b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/extensions/manager.rs
PierreLeGuen and others added 2 commits March 26, 2026 17:08
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…og message

1. Allow clearing the relay_url override: when an optional setup field
   with a setting_path is submitted empty, delete the stored setting so
   the system reverts to the env/default value. Previously empty values
   were silently skipped, making it impossible to undo an override from
   the UI.

2. Improve the OAuth callback team_id persistence error log to be
   self-contained without referencing implementation details.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes the channel-relay Slack activation/auth flow so “Activate” no longer dead-ends with “Authentication required” before offering OAuth, and adds observability + configuration improvements around relay behavior.

Changes:

  • Replace relay “authenticated” detection in auth_channel_relay() to check for persisted team_id (not the in-memory installed set) to avoid short-circuiting OAuth.
  • Add tracing across channel-relay auth/activation, OAuth callback persistence, and relay HTTP client calls.
  • Add a per-extension relay_url settings override (configured via Extensions > Reconfigure) to avoid requiring restarts for URL changes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
src/extensions/manager.rs Adds persisted-team-id auth check, relay URL override support, and additional tracing in relay auth/activation + setup schema/configure plumbing.
src/channels/web/server.rs Improves relay OAuth callback and extensions activation handler tracing; logs persistence failures.
src/channels/relay/client.rs Adds request/response tracing around relay HTTP calls and improves error logging context.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/channels/web/server.rs Outdated
Comment thread src/channels/relay/client.rs
Comment thread src/extensions/manager.rs Outdated
Comment thread src/extensions/manager.rs Outdated
Comment thread src/extensions/manager.rs Outdated
Comment thread src/extensions/manager.rs
Comment thread src/extensions/manager.rs
Comment thread src/extensions/manager.rs Outdated
henrypark133
henrypark133 previously approved these changes Mar 26, 2026
… handling

1. OAuth callback team_id persistence is now fatal: if set_setting fails,
   the callback returns an error instead of proceeding to activate (which
   would re-read from the store and fail anyway).

2. effective_relay_url uses owner scope (self.user_id) for reads, matching
   configure() which writes under the same scope. Prevents multi-user
   mismatch where an override saved via Reconfigure was invisible during
   auth/activation.

3. has_stored_team_id uses owner scope for the same reason — the OAuth
   callback stores team_id under state.owner_id (= self.user_id).

4. Security: effective_relay_url validates the override URL — only
   http/https without embedded credentials (userinfo) is accepted. This
   prevents API-key exfiltration if a user points relay_url at an
   attacker-controlled host. Logs only host portion, not full URL.

5. Fixed effective_relay_url docstring to match behavior (returns Option,
   callers handle the fallback).

6. get_setup_schema for ChannelRelay now logs a warning on settings store
   errors instead of silently returning None.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size: XL 500+ changed lines and removed size: L 200-499 changed lines labels Mar 26, 2026
@PierreLeGuen PierreLeGuen enabled auto-merge (squash) March 26, 2026 20:48
@PierreLeGuen PierreLeGuen merged commit adf4e25 into staging Mar 26, 2026
14 checks passed
@PierreLeGuen PierreLeGuen deleted the fix/channel-relay-auth-and-observability branch March 26, 2026 20:49
anthhub added a commit to anthhub/ironclaw that referenced this pull request Mar 27, 2026
PR nearai#1681 introduced 23 debug-level log statements across relay client,
web server handlers, and extension manager functions. Many of these fire
on every HTTP request or in loops (e.g. has_stored_team_id called per
extension in list_installed). Downgrade them to trace level to reduce
noise at the default debug log level while preserving warn/info logs
for actionable diagnostics.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ilblackdragon pushed a commit that referenced this pull request Mar 27, 2026
)

PR #1681 introduced 23 debug-level log statements across relay client,
web server handlers, and extension manager functions. Many of these fire
on every HTTP request or in loops (e.g. has_stored_team_id called per
extension in list_installed). Downgrade them to trace level to reduce
noise at the default debug log level while preserving warn/info logs
for actionable diagnostics.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
…override (nearai#1681)

* fix(extensions): channel-relay auth dead-end, add observability and relay URL override

Fix a bug where clicking Activate on the Slack relay extension produces
a dead-end "Authentication required" error with no OAuth URL. The root
cause: `auth_channel_relay()` used `is_relay_channel()` to check auth
status, but that function returns true as soon as the extension is
*installed* (in-memory set), before OAuth completes. This short-circuits
the OAuth flow so the authorization URL is never offered.

Changes:

1. **Bug fix** — `auth_channel_relay()` now uses `has_stored_team_id()`
   which only checks the persistent settings store for an actual team_id.
   The extension list `authenticated` field uses the same check so the UI
   accurately reflects OAuth completion status.

2. **Observability** — Added debug/warn/info tracing to all channel-relay
   code paths that were previously silent on failure:
   - `activate_channel_relay`: team_id retrieval, relay config, signing
     secret fetch, hot_add, cache operations
   - `auth_channel_relay`: auth check, OAuth initiation, nonce storage
   - `extensions_activate_handler`: request entry, auth fallback flow
   - `slack_relay_oauth_callback_handler`: team_id persistence (was
     silently ignored with `let _`)
   - `RelayClient`: initiate_oauth, get_signing_secret, proxy_provider
     all log URL, status, and errors
   - `has_stored_team_id`: store read success/failure

3. **Per-extension relay URL override** — Users can now override the
   CHANNEL_RELAY_URL via Settings > Extensions > Reconfigure. Stored
   under `extensions.{name}.relay_url` in settings. Both auth and
   activate read this override before falling back to the env default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback — clear relay_url override and improve log message

1. Allow clearing the relay_url override: when an optional setup field
   with a setting_path is submitted empty, delete the stored setting so
   the system reverts to the env/default value. Previously empty values
   were silently skipped, making it impossible to undo an override from
   the UI.

2. Improve the OAuth callback team_id persistence error log to be
   self-contained without referencing implementation details.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: collapse nested if per clippy::collapsible_if

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback — security, scope consistency, and error handling

1. OAuth callback team_id persistence is now fatal: if set_setting fails,
   the callback returns an error instead of proceeding to activate (which
   would re-read from the store and fail anyway).

2. effective_relay_url uses owner scope (self.user_id) for reads, matching
   configure() which writes under the same scope. Prevents multi-user
   mismatch where an override saved via Reconfigure was invisible during
   auth/activation.

3. has_stored_team_id uses owner scope for the same reason — the OAuth
   callback stores team_id under state.owner_id (= self.user_id).

4. Security: effective_relay_url validates the override URL — only
   http/https without embedded credentials (userinfo) is accepted. This
   prevents API-key exfiltration if a user points relay_url at an
   attacker-controlled host. Logs only host portion, not full URL.

5. Fixed effective_relay_url docstring to match behavior (returns Option,
   callers handle the fallback).

6. get_setup_schema for ChannelRelay now logs a warning on settings store
   errors instead of silently returning None.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DougAnderson444 pushed a commit to DougAnderson444/ironclaw that referenced this pull request Mar 29, 2026
… (nearai#1694)

PR nearai#1681 introduced 23 debug-level log statements across relay client,
web server handlers, and extension manager functions. Many of these fire
on every HTTP request or in loops (e.g. has_stored_team_id called per
extension in list_installed). Downgrade them to trace level to reduce
noise at the default debug log level while preserving warn/info logs
for actionable diagnostics.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
…override (nearai#1681)

* fix(extensions): channel-relay auth dead-end, add observability and relay URL override

Fix a bug where clicking Activate on the Slack relay extension produces
a dead-end "Authentication required" error with no OAuth URL. The root
cause: `auth_channel_relay()` used `is_relay_channel()` to check auth
status, but that function returns true as soon as the extension is
*installed* (in-memory set), before OAuth completes. This short-circuits
the OAuth flow so the authorization URL is never offered.

Changes:

1. **Bug fix** — `auth_channel_relay()` now uses `has_stored_team_id()`
   which only checks the persistent settings store for an actual team_id.
   The extension list `authenticated` field uses the same check so the UI
   accurately reflects OAuth completion status.

2. **Observability** — Added debug/warn/info tracing to all channel-relay
   code paths that were previously silent on failure:
   - `activate_channel_relay`: team_id retrieval, relay config, signing
     secret fetch, hot_add, cache operations
   - `auth_channel_relay`: auth check, OAuth initiation, nonce storage
   - `extensions_activate_handler`: request entry, auth fallback flow
   - `slack_relay_oauth_callback_handler`: team_id persistence (was
     silently ignored with `let _`)
   - `RelayClient`: initiate_oauth, get_signing_secret, proxy_provider
     all log URL, status, and errors
   - `has_stored_team_id`: store read success/failure

3. **Per-extension relay URL override** — Users can now override the
   CHANNEL_RELAY_URL via Settings > Extensions > Reconfigure. Stored
   under `extensions.{name}.relay_url` in settings. Both auth and
   activate read this override before falling back to the env default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback — clear relay_url override and improve log message

1. Allow clearing the relay_url override: when an optional setup field
   with a setting_path is submitted empty, delete the stored setting so
   the system reverts to the env/default value. Previously empty values
   were silently skipped, making it impossible to undo an override from
   the UI.

2. Improve the OAuth callback team_id persistence error log to be
   self-contained without referencing implementation details.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: collapse nested if per clippy::collapsible_if

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback — security, scope consistency, and error handling

1. OAuth callback team_id persistence is now fatal: if set_setting fails,
   the callback returns an error instead of proceeding to activate (which
   would re-read from the store and fail anyway).

2. effective_relay_url uses owner scope (self.user_id) for reads, matching
   configure() which writes under the same scope. Prevents multi-user
   mismatch where an override saved via Reconfigure was invisible during
   auth/activation.

3. has_stored_team_id uses owner scope for the same reason — the OAuth
   callback stores team_id under state.owner_id (= self.user_id).

4. Security: effective_relay_url validates the override URL — only
   http/https without embedded credentials (userinfo) is accepted. This
   prevents API-key exfiltration if a user points relay_url at an
   attacker-controlled host. Logs only host portion, not full URL.

5. Fixed effective_relay_url docstring to match behavior (returns Option,
   callers handle the fallback).

6. get_setup_schema for ChannelRelay now logs a warning on settings store
   errors instead of silently returning None.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
… (nearai#1694)

PR nearai#1681 introduced 23 debug-level log statements across relay client,
web server handlers, and extension manager functions. Many of these fire
on every HTTP request or in loops (e.g. has_stored_team_id called per
extension in list_installed). Downgrade them to trace level to reduce
noise at the default debug log level while preserving warn/info logs
for actionable diagnostics.

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

Labels

contributor: experienced 6-19 merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/web Web gateway channel scope: extensions Extension management size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants