Repository navigation
fix(adapters/slack): drop mrkdwn from chat.postEphemeral and correct retry docs - #137
Conversation
With a Slack Retry-After, each sleep reaches MaxDelay (1s), so two sleeps total MaxElapsed (2s), not about 600ms. A local 429 server that always sends Retry-After: 1 measured 2.01s over three attempts.
Decision 3 claimed that the deadline check keeps synchronous retry inside the ack window. It bounds only the backoff sleep. The decision now says that MaxElapsed bounds the backoff and that the caller's context and HTTP client bound each HTTP attempt, with a dated note that points to #132.
A Plain Text ephemeral message that falls back to a DM goes through chat.postMessage, which documents mrkdwn. The test fails when the fallback stops sending "mrkdwn": false.
Slack documents mrkdwn for chat.postMessage but not for chat.postEphemeral. The adapter test posts a Plain Text ephemeral message and expects no mrkdwn field. The slackapi test sets every PostEphemeralRequest field and checks that each encoded argument is one that Slack documents. Both fail until the field goes.
Slack lists no mrkdwn argument for chat.postEphemeral. Remove Mrkdwn from slackapi.PostEphemeralRequest and stop setting it in PostEphemeralMessage. The DM fallback still sends "mrkdwn": false through chat.postMessage. No caller or test relied on the field to send literal ephemeral text, so the text needs no escape.
A refactor onto the typed PostMessage would decode the native payload into []slackapi.Block and can drop fields that Block does not model. docs/reference.md says that PostNative posts opaque Block Kit blocks.
The PostNative comment now names the real limit: []slackapi.Block models only markdown, actions, and image blocks. The withDefaults comment drops a sentence that repeated the RetryPolicy doc.
|
@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: 82cff01577
ℹ️ 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 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. |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82cff01577
ℹ️ 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 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. |
Decision 4 and two Consequences also said that the default policy keeps synchronous retry inside the ack deadline. They now say that the caller's context or HTTP client bounds each HTTP attempt.
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Delightful! 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 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. |
…on, and the Linear bound PostNative skips the typed PostMessage because Payload is an any that it must post unchanged, not because Block is closed. CONTEXT.md and the interactive-components how-to now note that a native ephemeral Plain Text post keeps Slack formatting. The Linear withDefaults comment now gives its real worst-case backoff of 2s.
|
@codex review |
|
@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. |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ 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". |
Why
The operator wants every open follow-up issue in coder/chat closed by one small, reviewed PR. This PR closes the five Slack adapter and retry-doc follow-ups from the reviews of #132 and #133, as the triage comment on #136 decided them.
What changed
chat.postEphemeralno longer gets amrkdwnfield. Slack lists no such argument for that method, soslackapi.PostEphemeralRequestlosesMrkdwn. The DM fallback keeps"mrkdwn": falseonchat.postMessage, and a test now pins it.withDefaultscomment states the real worst-case backoff: 2s (MaxElapsed) with a SlackRetry-After, not about 600ms. ADR 0005 decision 3 now states only the bounds, with a dated amendment note that points to docs: fix stale example, dispatch, and retry text #132. Decision 4 and two Consequences drop the same ack-window claim.PostNativepayload says why it skips the typedPostMessage:[]slackapi.Blockmodels only three block types, and the docs call the payload opaque Block Kit.Closes #136
Scope
mrkdwn: falsefor literal ephemeral text, so the text gets no escape.mrkdwnargument onchat.postEphemeral, so the dropped field likely never turned parsing off. I did not verify this against a live workspace. CONTEXT.md and the interactive-components how-to now record this exception.adapters/linear/retry.gohad the same stale 600ms comment, and this PR corrects it too.Blast Radius
slackapi.PostEphemeralRequest.Mrkdwnis an exported field, so a caller that sets it stops compiling. The field had no documented effect on Slack.Verification
mrkdwn.Retry-After: 1made the default policy returnRateLimitedafter 2.01s and three attempts.go vet,go testfor every module,gofmt -l, andgo test -race ./adapters/slack/...pass.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high