feat: add slack-channel-monitor skill#258
Merged
Merged
Conversation
Adds a skill that guides users through creating a cron automation to monitor up to 10 Slack channels and start OpenHands conversations when a configurable trigger phrase is detected. Key features: - Polls channels every minute via cron (* * * * *) - Trigger phrase detection with configurable default (@OpenHands) - Resolves channel names to IDs; handles permission errors gracefully - Single search.messages call for multi-channel user tokens with search:read; falls back to per-channel conversations.history - Thread replies forwarded to running conversations - 👀 reaction on trigger messages - Conversation link posted immediately in the Slack thread - Terminal/idle state detection with debounce; posts agent final response as summary; error/stuck states post a clear error notice - State persisted across runs in automation-state/slack_poller_*.json Includes: - scripts/main.py - pure stdlib automation script (no SDK needed) - references/slack-api.md - token types, scopes, endpoints, limits - references/state-schema.md - JSON state schema and lifecycle diagram Co-authored-by: openhands <openhands@all-hands.dev>
Required by tests/test_skills_have_readme.py. Co-authored-by: openhands <openhands@all-hands.dev>
- Replace get_bot_user_id with _slack_auth_test which reads the X-OAuth-Scopes response header from auth.test. Fail fast with a clear RuntimeError if the token lacks channels:history (or equivalent) or chat:write - no point running the poll loop without read/write access. - Set can_react from the resolved scopes and guard both add_reaction call sites behind it, so a missing reactions:write scope causes a one-time note rather than a warning on every trigger message. - Add workspace field to create_conversation (POST /api/conversations requires it); use WORKSPACE_BASE env var, defaulting to /workspace. Co-authored-by: openhands <openhands@all-hands.dev>
Required by tests/test_skill_plugin_loading.py: - .plugin/plugin.json (Codex and Claude Code manifest) - .claude-plugin -> .plugin (symlink) - .codex-plugin -> .plugin (symlink) Co-authored-by: openhands <openhands@all-hands.dev>
POST /api/conversations requires either 'agent' or 'agent_settings'. Add _get_agent_settings() which calls GET /api/settings and extracts the configured agent_settings block (LLM model, agent kind, etc.), then pass it through in create_conversation so the new conversation inherits the server's LLM configuration rather than failing validation. Co-authored-by: openhands <openhands@all-hands.dev>
The agent_settings code path has a double-registration bug: Pydantic
calls create_agent() during StartConversationRequest validation to
populate the agent field, then StoredConversation construction runs the
same initialisation again - both attempts try to register the LLM with
usage_id='default' in the per-conversation registry, and the second
call raises ValueError('Usage ID already exists in registry').
Fix by switching to the same approach the SDK uses:
- Fetch GET /api/settings with X-Expose-Secrets: plaintext to get the
real (unmasked) LLM api_key
- Build an 'agent' dict directly {kind: Agent, llm: <settings.llm>}
- Pass it as the 'agent' field (not 'agent_settings') so Pydantic
validates the dict via AgentBase.model_validate() without triggering
create_agent(), and StoredConversation construction only registers
the LLM once
Also drops secrets_encrypted: True (no longer needed) and removes the
now-unused uuid import.
Co-authored-by: openhands <openhands@all-hands.dev>
When initial_message is provided, conversation_service.py calls
send_message(message, run=True) which starts the agent immediately.
Our subsequent POST to /api/conversations/{id}/run therefore always
returned 409 'Conversation already running', which propagated as an
exception out of create_conversation before conv_id was returned.
The knock-on effects were:
- active_convs was never updated, so the conversation was never tracked
- The 'On it!' Slack reply and all follow-up (completion detection,
posting the verdict) were silently skipped
Fix: drop the /run call entirely - the server handles it.
Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste Rating: 🟡 Acceptable
This is a well-structured skill that solves a real problem (Slack-OpenHands integration). The implementation is functional and the documentation is comprehensive. However, there are important architectural issues that should be addressed:
- Function complexity: The
main()function is 284 lines and violates the max-3-indentation rule in multiple places - Testing gap: Complex stateful logic lacks automated tests for core functions
- Code organization: Several opportunities to extract reusable helpers
The code works (evidenced by screenshots) but needs refactoring for long-term maintainability.
[TESTING GAPS]
This PR adds 664 lines of complex automation logic with stateful behavior (message tracking, conversation lifecycle management), but provides no automated tests. The repository has test infrastructure and similar components (pr-review, qa-changes, release-notes) include tests.
Core logic functions that should be unit tested:
_is_human_message()- message filtering logic with multiple conditionsload_state()/save_state()- state persistence with schema versioning- Thread root timestamp extraction (lines 515-516)
- Bot message timestamp tracking/trimming (lines 643-647)
- Conversation key generation (
"{channel_id}:{thread_ts}")
While full integration tests would require mocking Slack and OpenHands APIs, these pure logic functions can be tested without external dependencies. Add unit tests for the core message processing and state management logic.
[RISK ASSESSMENT]
Risk factors:
- Complexity: 284-line main() function with multiple nested control flows increases bug surface area
- State management: Persistent JSON state across runs with no validation - corrupted state could cause silent failures
- External dependencies: Integrates with two external APIs (Slack + OpenHands) with error handling that sometimes silently continues
- Testing: No automated tests to catch regressions in message filtering or state transitions
Mitigating factors:
- Manual testing confirms basic functionality (screenshots show end-to-end flow)
- Stdlib-only implementation reduces dependency risk
- Fire-and-forget automation pattern (failures don't block critical paths)
- Clear documentation and reference materials
VERDICT:
✅ Worth merging - Functional implementation solves a real need, but follow-up refactoring recommended for long-term maintainability.
KEY INSIGHT:
The complexity is concentrated in a single 284-line function. Extract 4-5 focused helpers to bring indentation back under control and make the logic testable.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/extensions/actions/runs/26301032233
- Improve constant comments (INITIAL_LOOKBACK, DONE_DEBOUNCE, MAX_BOT_TS,
CONTEXT_MESSAGE_LIMIT) with fuller explanations of intent
- Add CONTEXT_LOOKBACK_SECONDS constant to name the magic 3600 value
- Change has_search_permission to accept scopes set[str] instead of
making a redundant API call (scopes already available from auth.test)
- Extract _gather_channel_context() helper to eliminate 5-level nesting
- Break main() (284 lines) into focused helper functions:
_resolve_slack_token(), _verify_token_scopes(), _poll_new_messages(),
_process_trigger_message(), _check_conversation_completion()
Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new skill that guides users through setting up a cron automation to monitor up to 10 Slack channels and start OpenHands conversations when a configurable trigger phrase is detected.
Skill:
slack-channel-monitorWhat it does
When invoked, the skill walks the user through:
@openhands)* * * * *Each polling run:
Files
SKILL.mdscripts/main.pyreferences/slack-api.mdScreenshots
Create an automation with a prompt:
This creates an automation:
Which monitors the channel...
Creates conversations...
And posts messages back to slack...
Technical notes
search:read+ >1 channel uses a singlesearch.messagescall; otherwise oneconversations.historycall per channel +conversations.repliesper active thread{WORKSPACE_BASE_ROOT}/automation-state/slack_poller_{automation_id}.jsonacross runsOPENHANDS_URLsecret{idle, finished, error, stuck}with a 15-second debounce to avoid same-run false positivesdev:automationlocal stack; a cloud/webhook variant is a separate concernThis PR was created by an AI agent (OpenHands) on behalf of @tofarr.