-
Notifications
You must be signed in to change notification settings - Fork 94
feat: Add OpenAPI Slack Server #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@binchick-in @koskorya spun this up localhost after hours, can port in tomorrow am but scopes are authorized for our instance & slack-side app is installed |
…pi-servers into slack_openapi_server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks test run of the github mcp + o4 for that... pretty mid self code review 😂 didn't realize it would just write straight to it as me, the future is here

Overall, this is a solid foundation—nice work on the dynamic endpoint generation, detailed models, and Docker setup. Below are my observations and suggestions:
1. Project structure & documentation
- README: Very clear quickstart and prerequisites. Consider adding a link to where
.env
should live (e.g., root ofservers/slack
) and a note about version compatibility (Python 3.10, FastAPI X.X). - Docker Compose: The
compose.yaml
is minimal; you might want to include named volumes or environment injection for secrets (e.g.,env_file: .env
).
2. Dockerfile
- Caching pip dependencies is great. You may also pin dependencies in
requirements.txt
or use aconstraints.txt
to ensure reproducible builds. - Consider adding a healthcheck (e.g.,
HEALTHCHECK CMD curl --fail http://localhost:8000/ || exit 1
) so orchestrators know if the container is healthy.
3. Dependency management
requirements.txt
is straightforward. If this project grows, consider switching to Poetry or pinned hashes for more robust dependency locking.python-multipart
appears unused—unless you’re planning file uploads, it could be removed.
4. Configuration & security
- Environment validation: currently
ValueError
is raised at import time. It may be better to raise an HTTPException during startup or use FastAPI’slifespan
events, to yield clearer error messages without a raw traceback. allow_origins = ["*"]
: for production, this is open to all origins. Recommend restricting to known domains or configuring via env var.- Secrets in logs: the generic
print
statements may inadvertently log sensitive data. Consider using Python’slogging
module at appropriate levels and scrub sensitive fields.
5. SlackClient & error handling
- Great abstraction around
httpx.AsyncClient
. A few thoughts:- You call
response.json()
on non-2xx responses; a failing Slack API sometimes returns 200 withok:false
. Good handling, but consider capturing and returning the raw Slackerror
field in yourToolResponse
for richer agent feedback. - In
_request
, you catchException
broadly; it might mask programmer errors. You could limit tohttpx.RequestError
and JSON parse errors. - Rate limiting: Slack enforces rate limits (HTTP 429). You may want to handle 429 specifically, backoff (
Retry-After
), and propagate a clear error.
- You call
6. Endpoint generation & performance
- Dynamic registration via
TOOL_MAPPING
is clean. One nit:name=tool_name
in decorator eventually appears as operationId; ensure no collisions and consider naming conventions if you add more tools. get_channels
fetches channel history sequentially. For many channels, this will be slow—consider limiting history fetch or runningfetch_channel_with_history
concurrently (e.g., viaasyncio.gather
with a concurrency semaphore).
7. Models & typing
- Pydantic models are clear and documented. One thought: the
ToolResponse
wraps Slack’s JSON undercontent
. For client code, returningDict[str, Any]
loses typing. Future work could define response models per tool. - The top-level endpoint handlers use
args: args_model = Body(...)
. For consistency, consider usingDepends
or a common router prefix (/slack
) so auto-generated docs group them better.
8. Testing
- I didn’t see unit tests or CI integration. I recommend adding:
- Unit tests for
SlackClient
(mocking httpx). - Integration tests against a Slack workspace (can be skipped in CI).
- CI workflow for linting (flake8/black), type checking (mypy), and tests.
- Unit tests for
9. Miscellaneous
- In Dockerfile, the
CMD
quotes aroundmain:app
are single quotes; POSIX shells may interpret differently. Double quotes or none is typical:uvicorn main:app --host 0.0.0.0 --port 8000
. - The default CORS origin array can grow; consider making it configurable.
- For production, you may want to support OpenAPI auth (API key, OAuth) so agents can’t call your server without permission.
^ bunch of goodness - we've been running this for a couple days and very happy, these eek out a little more performance:
Performance:
|
Amazing! |
Add Slack OpenAPI Server
Overview
Implements a FastAPI-based OpenAPI server for Slack workspace interactions, providing standardized access to Slack API functionality through well-defined endpoints.
Changes
Testing
Documentation
Required env vars: