Repository navigation
feat(adapters/slack): add slackapi, a low-level Slack API package - #97
Conversation
…ntion helpers, and manifest types
…k, and slackapitest
…rects and check upload redirects DownloadFile sets the bearer token again on each redirect that passes the FileOrigins check, because http.Client drops it on a redirect to another host. UploadToURL checks the origin of every redirect with a per-call copy of the HTTP client and never adds a token. Origins drop the default port of the scheme, New warns about each dropped FileOrigins entry, and an empty file URL returns a clear error.
GetUploadURLExternal sends its form through callForm. The response type is now GetUploadURLExternalResponse, which matches CompleteUploadExternalResponse and does not stutter with its UploadURL field. UploadFile wraps step errors without a second slack: prefix.
… encode empty action elements SplitMarkdown tracks the backtick count of the opening fence: only a line of at least as many backticks and optional whitespace closes it, and the added closing fence repeats that count. A chunk no longer ends inside or right after an opening fence line, which left an empty code block. ActionsBlock encodes nil Elements as an empty array.
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cc5463fbd
ℹ️ 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".
|
@codex review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cc5463fbd
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4d7d5e183
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e887367dee
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Maintainer review. I accept the direction. Below are a few targeted requests before this leaves draft. Direction. Putting the Slack protocol code into a low-level Requests:
After that, it's the normal gate: green CI, then one Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e887367dee
ℹ️ 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".
… and the app home manifest section User gets IsRestricted and IsUltraRestricted, and Message gets UserTeam, SourceTeam, Username, and BotProfile, so a bot can drop guests and users from another organization and name the app behind a bot message. ManifestFeatures gets AppHome, so a manifest can turn the Messages tab off.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Status check from the maintainer side. Main still has the #98 history bug, and this PR is the planned fix, so I'd like to land it soon. Codex's code review is clean on
If you won't have time this week, say so. I can then land a small standalone #98 fix on main and leave this PR to go at its own pace. Generated with |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
2 participants
Fixes #98
The Slack adapter already does all the Slack protocol work, but you can only get at it through
chat.New. An application that runs its own chat engine, with its own state, dispatch, and routing (say, a Slack bot built into another server), can't use it. It also needs things the portable surface leaves out on purpose, like edits, deletes, reactions, and files.This PR adds
adapters/slack/slackapi, a low-level package for that: a typed Web API client with bounded retry, the request signature check, Events API parsing, file upload and host-restricted download, Block Kit types, and a Markdown splitter for Slack'smarkdownblock limit. There's alsoslackapitest, a fake Slack server for callers to test against. Both are marked experimental, so the exported API may change before they're promoted. ADR 0016 has the reasoning.The caller I'm building this for is the Coder Slack bot (coder/coder,
enterprise/coderd/x/slack, pinned to1e0e473). That's whyManifestandSetAssistantThreadStatusare exported. The bot builds its app manifest fromslackapi.Manifestfor its admin API's manifest endpoint and install link, and1e0e473addedManifestAppHomebecause the bot turns the Messages tab off to block DMs. It usesSetAssistantThreadStatusto show and clear the thread status, which is the only sign that the bot accepted a message.The adapter now uses
slackapifor its calls, retry, signature check, and history reads, so there's only one copy of each.RetryPolicyandRateLimitedare now aliases, so nothing breaks. There are three behaviour changes:conversations.historyandconversations.repliesare now sent as form posts rather than JSON, which fixes bug(adapters/slack): HistoryReader fails on live Slack — conversations.history/replies sent as JSON #98 (more on that below).HistoryReadernow comes back newest-first, across pages too.conversations.repliesreturns each page oldest-first, with the root at the start of every page, so the adapter now normalises that.rate_limitederror in an HTTP 200 response now retries, the same as a 429.The adapter still builds its own request bodies for
auth.test,chat.postMessage,chat.postEphemeralandconversations.open, and I'll move those onto the typed methods in a follow-up.There's also an env-gated
TestLive, which I ran against my test workspace, and it passed. It showed that the 12,000 character limit applies to all themarkdownblocks in a message together, not to each block. It also showed that the adapter'sHistoryReaderwas broken on real Slack, since it sentconversations.historyandconversations.repliesas JSON:slackapisends those as form posts, which works, so the adapter now just reads history throughslackapi. The adapter tests' fake Slack server now rejects JSON for those methods too, like real Slack does, and the tests assert the exact form parameters sent, including thelatestvalue used for each page.Most of the diff is tests. The first few commits split it up by area, and the rest are fixes on top.