feat(slack): implement UI for Slack Bot OAuth connect/disconnect and cursor-based log pagination

[zaira-bibi]

What

As part of #246, this PR extends the Slack TA Bot with cursor-based log pagination, a RAG sources endpoint, per-request LLM model override, and a frontend plugin page with OAuth connect/disconnect.

Changes

• feat(api): add /api/v1/slack/logs cursor + since_id pagination, /rag/sources endpoint, and llm_model_override support on the Slack bot config
• feat(frontend): add Slack TA Bot plugin page — connection card with OAuth connect/disconnect, page header, branded four-color icon, and stub settings modal

How to Test

1. Enable the Slack TA Bot from My Plugins → Settings.
2. Navigate to Slack TA Bot in the sidebar — confirm the page header and connection card render.
3. Click Connect Slack — confirm redirect to Slack OAuth.
4. Complete OAuth — confirm redirect back to /dashboard/slack?connected=true, success banner appears, and workspace name shows in the card.
5. Click Disconnect — confirm confirmation dialog, workspace disconnects, card returns to "Not connected".
6. Hit GET /api/v1/slack/logs?limit=10 with a valid token — confirm paginated response with next_cursor and has_more.
7. Hit GET /api/v1/slack/rag/sources — confirm list of indexed RAG sources is returned.
8. Set llm_model_override in the bot config and trigger a Slack mention — confirm the override model is used for synthesis.

Notes

Migration required: yes — run make migrations (adds llm_model_override column to the Slack config table).

[claude]

Claude finished @zaira-bibi's task in 2m 39s —— View job

---

Review of PR #320

• Check for previously addressed review comments — none found
• Read all changed backend files (routes, config, types)
• Read all changed frontend files (plugin, components, API)
• Read changed test files
• Security review
• Correctness and logic review
• Performance review
• React anti-patterns check
• Test coverage assessment
• Post review summary with inline comments

---

Overview

This PR adds a well-structured Slack TA Bot frontend plugin page with OAuth connect/disconnect, cursor-based log pagination, a RAG sources endpoint, and per-request LLM model override support. The code is clean, follows existing project patterns (mirrors the googledrive plugin structure), and the test suite is thorough.

Previous Review Comments

No prior review threads found.

---

Issues by Priority

High — since_id pagination silently drops data

app/core_plugins/slack/routes.py:435-439 — The since_id path always returns has_more=False and next_cursor=None, even when results are capped by limit. If more than limit new entries exist since since_id, the client receives a truncated page with no indication that data was dropped. The fix is straightforward: apply the same limit + 1 pattern used in the cursor path. See inline comment with fix →

Medium — Browser alert()/confirm() instead of UI components

frontend/plugins/slack/components/SlackConnectionCard.tsx:27-29, 34 — Error and confirmation dialogs use raw browser APIs (alert(), confirm()), which is inconsistent with the styled error banner in the parent SlackPlugin.tsx and the project's Radix UI component library. See inline comment →

Low — Unused props in stub modal

frontend/plugins/slack/components/SlackConfigModal.tsx:14 — onSave and onRefresh are declared in the interface but unused. Minor for a stub, but could confuse future contributors. See inline comment →

Low — Hardcoded dashboard path

frontend/plugins/slack/SlackPlugin.tsx:47 and app/core_plugins/slack/routes.py:136 — The path /dashboard/slack is hardcoded in both frontend and backend. See inline comment →

---

What Looks Good

• Security: OAuth state uses itsdangerous for CSRF protection, all routes require authentication via require_user_id, Slack signature verification is in place, and queries use SQLModel's parameterized approach (no injection risk).
• Error handling: Catches specific exceptions throughout (no bare except Exception), all except blocks include logging — fully compliant with the project's exception handling guidelines.
• Pagination design: Cursor-based pagination avoids OFFSET performance issues. The limit + 1 technique in the cursor path is efficient. Mutual exclusivity of cursor and since_id is cleanly enforced.
• Frontend architecture: Clean component decomposition (SlackPlugin → SlackConnectionCard, SlackConfigModal). useCallback with proper deps, effect cleanup for the timer, and no React anti-patterns detected.
• Test coverage: Excellent — 14 test classes/functions covering OAuth flow, events, dispatch (6 edge cases), pagination (8 scenarios including bounds), RAG sources, and LLM provider with/without model override. Missing: a test for since_id with limit < available_rows (would catch the has_more bug).
• Plugin registration: Follows established patterns — generateStaticParams, plugin definition with sidebar config, lazy-loaded component/settings.

Overall Assessment

Solid PR. The since_id pagination bug is the only issue I'd consider blocking — it's a data correctness problem with a straightforward fix. The rest are minor UI consistency items. Test coverage is strong and the code quality is high throughout.

[claude]

Claude finished @zaira-bibi's task in 1m 51s —— View job

---

Re-review of PR #320 (post-sync)

• Fetch previous review comments and check if addressed
• Read all changed backend files
• Read all changed frontend files
• Read changed test files
• Security review
• Correctness and logic review
• Performance review
• React anti-patterns check
• Test coverage assessment
• Post review summary with inline comments

---

Previous Review Comments

All issues from the initial review have been addressed in commit 770e691:

#	Issue	Status	Details
1	High — since_id pagination silently drops data	Resolved	since_id path now uses limit + 1 pattern with correct has_more and next_cursor (routes.py:437-446). Test test_since_id_has_more_when_results_exceed_limit added.
2	Medium — Browser alert()/confirm() instead of UI components	Resolved	SlackConnectionCard.tsx now propagates errors via onError callback and uses a Radix Dialog for disconnect confirmation (lines 94-111).
3	Low — Unused props in stub modal	Acknowledged	PR author confirmed this stub will be replaced in the next PR. No action needed.
4	Low — Hardcoded dashboard path	Resolved	Backend extracts SLACK_FRONTEND_PATH constant (routes.py:54), frontend uses SLACK_PLUGIN_PATH (index.ts:4). Both are now single-source.

---

New Issues Found

None. The second commit cleanly addresses the review feedback without introducing regressions.

---

What Looks Good (unchanged from initial review + improvements)

• Security: OAuth CSRF protection via itsdangerous, all routes require require_user_id, Slack signature verification, parameterized queries, encrypted tokens.
• Error handling: Specific exception types throughout, all except blocks include logging — fully compliant with project guidelines.
• Pagination: Both since_id and cursor paths now correctly implement limit + 1 truncation with has_more signaling. Mutual exclusivity enforced.
• Frontend architecture: Clean component decomposition with proper error propagation (onError callback), Radix Dialog for confirmation, useCallback with correct deps, timer cleanup in the useEffect.
• Test coverage: Excellent — 14 test classes/functions covering OAuth, events, dispatch (6 edge cases), pagination (8 scenarios including the previously-missing since_id + limit case), RAG sources, and LLM provider with/without model override.
• React patterns: No anti-patterns detected — proper dependency arrays, no direct DOM manipulation, no state mutations, timer cleanup present, no array-index keys.

Overall Assessment

All blocking and non-blocking issues from the initial review have been addressed. The code is clean, well-tested, and follows project conventions. Ready to merge.

[claude]

Claude finished @zaira-bibi's task in 3m 31s —— View job

---

Re-review of PR #320 (post-sync — commit e43e6b8)

• Fetch previous review comments and check if addressed
• Read all changed backend files
• Read all changed frontend files
• Read changed test files
• Security review
• Correctness and logic review
• Performance review
• React anti-patterns check
• Test coverage assessment
• Post review summary with inline comments

---

Previous Review Comments

All issues from the initial review and from @hamza-56's review have been addressed in commits 770e691 and e43e6b8:

#	Issue	Status	Details
1	High — since_id pagination silently drops data	Resolved ✓	since_id path now uses limit + 1 pattern with correct has_more and next_cursor. Test test_since_id_has_more_when_results_exceed_limit added.
2	Medium — Browser alert()/confirm() instead of UI components	Resolved ✓	SlackConnectionCard.tsx now propagates errors via onError callback and uses a Radix Dialog for disconnect confirmation.
3	Low — Unused props in stub modal	Acknowledged ✓	PR author confirmed stub will be replaced in next PR.
4	Low — Hardcoded dashboard path	Resolved ✓	Backend uses SLACK_FRONTEND_PATH constant, frontend uses SLACK_PLUGIN_PATH.
5	Medium — llm_model_override lets any string through (@hamza-56)	Resolved ✓	New commit refactors adapter system and validates override against get_models_for_provider() at write time.

All 5 review threads resolved.

---

New Commit Analysis (e43e6b8)

This commit introduces a clean refactoring of the plugin adapter system to validate llm_model_override at config write time:

Architecture changes:

• PluginConfigAdapter Protocol (app/services/plugin_adapters/base.py) deleted — replaced by LLMConfigAdapter concrete base class (app/llm/adapter.py)
• app/services/plugin_adapters/registry.py → app/plugins/adapters.py — cleaner location
• ChatPluginConfigAdapter → ChatConfigAdapter, SlackConfigAdapter added
• Preprocessing moved from the API layer (user_plugins.py) into PluginService.update_user_plugin_config() so it runs on the merged config, catching cross-field issues when only one field changes

Validation logic (app/llm/adapter.py:53-77):

• If llm_model_override is set but llm_config_id is None → raises ValueError (can't determine provider)
• If both are set → looks up the LLMConfig, calls get_models_for_provider(llm.provider), rejects if override isn't in the allowlist
• LLMConfigValidationError extends ValueError, correctly caught by existing exception handlers in user_plugins.py

Key design decision (good): Preprocessing now runs on the full merged config (plugin.py:292) rather than just the incoming partial dict. This catches the scenario where a user changes llm_config_id to a different provider but the stored llm_model_override is incompatible — tested in test_update_config_revalidates_stored_model_override_against_new_provider.

---

New Issues Found

None. The refactoring is clean, well-tested, and introduces no regressions.

---

What Looks Good

• Security: OAuth CSRF via itsdangerous, all routes require require_user_id, Slack signature verification, parameterized queries, encrypted tokens, model override validated against allowlist at write time.
• Error handling: Specific exception types throughout, all except blocks include logging — fully compliant with project guidelines. No bare except Exception.
• Pagination: Both since_id and cursor paths correctly implement limit + 1 truncation with has_more signaling. Mutual exclusivity enforced.
• Frontend architecture: Clean component decomposition with proper error propagation (onError callback), Radix Dialog for confirmation, useCallback with correct deps, timer cleanup in useEffect. No React anti-patterns detected.
• Adapter refactoring: Removing the Protocol in favor of a concrete base class is the right call — the Protocol provided no value since all adapters inherit the same implementation. Moving preprocessing into PluginService is a correctness fix that prevents partial-update validation gaps.
• Test coverage: Excellent — the new commit adds 10 test cases across 3 files covering: model override validation (valid, invalid, missing config ID, none override), adapter registration, cross-field revalidation on partial updates, and Slack-specific adapter behavior. Total across the PR: 20+ test functions covering OAuth, events, dispatch, pagination, RAG sources, LLM provider, and adapter validation.
• Documentation: PLUGIN_GUIDE.md updated with comprehensive adapter docs including override examples.

Overall Assessment

All blocking and non-blocking issues from prior reviews have been addressed. The new commit cleanly resolves @hamza-56's feedback with a well-designed adapter refactoring and thorough test coverage. Ready to merge.