Repository navigation
adapters/slack: follow-ups from #132 and #133 (retry comment, ADR 0005, mrkdwn tests, PostNative) #136
Description
Follow-ups from the reviews of #132 and #133 in the Slack adapter and its retry docs. Each item is small, and none blocked those PRs.
- Retry default comment. The
withDefaultscomment inadapters/slack/slackapi/retry.go(L42-L44) says the worst-case backoff is about 600ms. With a SlackRetry-After, each delay is capped only atMaxDelay, so total backoff can reachMaxElapsed(2s). Correct the number. - ADR 0005 ack-window sentence. docs/adr/0005-rate-limit-handling.md#L29 ends with "This keeps synchronous retry inside the ack window." The deadline check and
MaxElapsedbound only the backoff sleep, not the HTTP attempts (docs: fix stale example, dispatch, and retry text #132 corrected the same claim in the package docs). ADRs record decisions, so decide whether to amend it or leave a note. - No test pins
mrkdwn: falseon two posts. No adapter test checks"mrkdwn": falseonchat.postEphemeral(slack.go#L359) or on the DM-fallback post (slack.go#L612). Removing either line still passes everyadapters/slacktest, on main before refactor(adapters/slack): move Web API calls onto typed slackapi methods #133 and after it. PostNativekeeps its own request body. After refactor(adapters/slack): move Web API calls onto typed slackapi methods #133,PostNativestill builds its ownchat.postMessagebody (interactive.go#L308), becauseslackapi.PostMessageRequest.Blocksis[]Blockand docs/reference.md promises the adapter forwards a native payload unchanged. Decide whetherPostNativecan require a JSON array of blocks and use the typed method.mrkdwnonchat.postEphemeral. The adapter sendsmrkdwntochat.postEphemeral, as it did before refactor(adapters/slack): move Web API calls onto typed slackapi methods #133. The refactor(adapters/slack): move Web API calls onto typed slackapi methods #133 author reported that Slack documentsmrkdwnonly forchat.postMessage. I did not verify this. Check Slack's method docs and drop the field if Slack ignores it.
Owner and trigger
The coder/chat maintainer triages this. Items 1 to 3 are ready for an agent. Items 4 and 5 need a decision first.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh
Activity
ThomasK33 commentedon Oct 29, 2026
Triage:
ready-for-agent. I made the two open decisions, so all five items can ship in one PR.MaxElapsed, and individual HTTP attempts are bounded by the caller's context and HTTP client. Add a dated amendment note that points to docs: fix stale example, dispatch, and retry text #132.PostNative): keep its own body.docs/reference.mdpromises to forward a native payload unchanged. Decoding it into[]slackapi.Blockand encoding it again can drop fields that the typed struct does not model. Add a one-line comment atinteractive.gothat states this reason, so the next refactor does not undo it.mrkdwnonchat.postEphemeral): I verified it. Slack's method docs listparse,markdown_textandtext, but nomrkdwnargument. DropMrkdwnfromslackapi.PostEphemeralRequestand from the adapter atslack.goL359.mrkdwnfield is absent. The DM-fallback post (chat.postMessage) keeps"mrkdwn": false, and the test must pin it.I did not verify one thing: whether any caller relies on
mrkdwn: falseto send literal text in an ephemeral message. If a test shows it does, escape the text instead.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:medium