fix(slack): normalise outbound URLs at boundary — closes #1092, partial #850

[johnkattenhorn]

Addresses #1092 in full and partially closes #850.

Two bugs, one outbound-Slack boundary

#1092 — Slack rewrites OAuth scope delimiters on click.
The LLM's response sometimes carries the URL inside a markdown link [label](url) and URL-encodes the literal + (scope-list separator) into %2B along the way. Whether the URL reaches Slack as plain text, mrkdwn <url>, or Block Kit RichTextLink, Slack's click redirector re-encodes + to %2B on click, so the final URL the IdP sees has every scope concatenated into one invalid string. Repro: Doist + taylorwilsdon/google_workspace_mcp + claude-sonnet-4-5 → click → Error 400: invalid_scope.

#850 — Standard markdown links don't render in Slack.
[text](url) shows as raw text in Slack; only Slack-native <url|text> renders as a clickable link. Aaron documented the three forms tested in the issue body.

The two bugs share the same outbound boundary (SlackReplyClient.PostThreadReplyAsync / SlackOutboundClient.PostNewThreadAsync / SlackReplyClient.UpdateThreadMessageAsync) so I'm fixing them in one place.

Fix

A new SlackTextProtector runs on the agent's response text before it's posted, plus the same helpers run inside SlackBlockConverter so the Block Kit and Text-field paths stay consistent.

Path	Safe URL (no +, no %2B)	Rewrite-prone URL
Bare URL	<url> mrkdwn (clickable)	inline code (non-clickable, copy-paste)
Markdown link [label](url)	`<url	label>` mrkdwn (clickable — addresses #850)
Block Kit	RichTextLink	RichTextText { Style.Code = true }

NormaliseScopeList(url) runs first for both paths: if the URL has a scope= parameter whose value contains two or more %2B separators, decode them back to +. Single %2B is left alone (likely a legitimate literal +).

IsRewriteProne(url) is the central is-it-safe-to-link predicate (today: contains + or %2B; extensible).

Tests

19 xUnit facts in Netclaw.Actors.Tests/Channels/SlackTextProtectorTests.cs:

• bare URL handling — safe, literal +, mis-encoded %2B, single-%2B negative case, %20 negative case
• markdown link handling — safe, mis-encoded scope-list, literal +
• already-protected URLs — <...>, `...`, <url|label> mrkdwn
• multiplicity — multiple URLs, mixed markdown + bare
• prose punctuation — (see https://...)
• realistic 22-scope Google OAuth URL — full decode + render as inline code
• direct invariants — IsRewriteProne and NormaliseScopeList parameterised theories

TDD evidence

Verified failing-then-passing locally — with NormaliseScopeList and IsRewriteProne removed (the rest of the test cases kept as-is), the rewrite-prone facts fail with the exact diff the bug shows. With the helpers in place, all 50 facts in Netclaw.Actors.Tests/Channels/ pass.

End-to-end verification

Doist hosted MCP + Anthropic claude-sonnet-4-5 + taylorwilsdon/google_workspace_mcp over Slack. Before this PR: bot DM'd an OAuth URL via markdown link; Slack rendered it as raw [Authorization URL](url-with-%2B) text and click/paste both produced invalid_scope. After this PR: rewrite-prone URL is decoded to original + form and rendered as inline code (gray, non-clickable); manual copy out of the code element delivers the URL byte-exact and Google accepts the full 22-scope list.

Known limitation — why #850 is only partially closed

Rewrite-prone URLs are intentionally non-clickable in this PR. Slack's link redirector cannot be told to skip a particular URL, so the only way to preserve a URL containing + or %2B is to render it as inline code (Slack does not click-rewrite text inside backticks). #850's "must be clickable" requirement is met for safe URLs but is intentionally not met for rewrite-prone ones — that's a tradeoff between clickable UX and a URL the IdP can actually accept.

A follow-up could introduce a local HTTP redirector (daemon-side) that hands Slack a short opaque URL which 302s to the real URL. That's the only path I see to clickable + preserved. Out of scope here.

Out-of-scope

• Discord channel adapter unchanged.
• IsRewriteProne is intentionally conservative — extend as further click-rewrite patterns are observed.
• Broader mrkdwn translation of the Text field (bold/italic/code outside URL handling) can be a follow-up.

[Aaronontheweb]

Thanks for this @johnkattenhorn — solid fix, and the writeup made the two failure modes easy to follow. I gave it a full review and pushed a follow-up commit (62c0c03) directly to the branch to clear the should-fix items rather than ping-pong on them:

1. Bare-URL regex swallowed trailing sentence punctuation. BareUrlRegex matched https://example.com. including the period. This was already latent in SlackBlockConverter, but the PR newly applies wrapping to the plain-text Text field — so it would emit <https://example.com.> and Slack makes the period part of the clickable link target. The regex now constrains only the final character to exclude .,!?;:; mid-URL punctuation is still preserved.

2. Two divergent BareUrlRegex definitions. SlackTextProtector and SlackBlockConverter each had their own bare-URL pattern with different exclusion sets, which undercut the "both surfaces stay in lockstep" goal of the PR. SlackBlockConverter now consumes the shared internal SlackTextProtector.BareUrlRegex() — one source of truth.

3. NormaliseScopeList scope= targeting. IndexOf("scope=") would also match a scope= substring inside a path segment or a longer parameter name (myscope=). It now anchors to a ?/& query-parameter boundary. Low real-world risk thanks to the >=2-%2B guard, but worth tightening since it is new code.

Regression tests added on both the Text and Block Kit surfaces. Locally: 63/63 channel tests pass, dotnet slopwatch analyze is clean, and copyright headers verify.

Two pre-existing nits I deliberately left alone — not introduced here, inherited from SlackBlockConverter, fine to track separately:

• IsRewriteProne flags any + anywhere in a URL, so a legitimately-clickable URL containing a + becomes non-clickable inline code. Over-protecting (copy-paste instead of click) is the safe failure mode, so this is acceptable as-is.
• The markdown-link regex \(([^)]+)\) truncates URLs containing ) (e.g. Wikipedia-style ..._(bar)).

The partial #850 close is well-reasoned and clearly documented — no objection to deferring the rest. LGTM once CI is green.

[Aaronontheweb]

LGTM