AP-24630: Add optional clickable URLs in Agent Chat Widget#72
AP-24630: Add optional clickable URLs in Agent Chat Widget#72
Conversation
ya-hn
force-pushed
the
enh/AP-24630-support-urls-in-markdown-of-chat-agent-widget
branch
from
d2ac437 to
d473aab
Compare
| class AgentChatWidgetContentSettings: | ||
| allow_links = knext.BoolParameter( | ||
| "Allow links", | ||
| "If checked, URLs in the chat view will be clickable and open in a new tab.", |
There was a problem hiding this comment.
I used "view" here because the UI also says "Open view" and the description for the re-execution trigger also refers to it as chat view. But we could also change it to "chat widget".
There was a problem hiding this comment.
Pull request overview
Adds an allow_links configuration flag to the Agent Chat Widget to optionally render URLs as clickable (http/https only) in the chat UI, while tightening Markdown security by disabling raw HTML and disallowing style.
Changes:
- Introduces
allow_linksin backend widget configuration (Python) and exposes it to the frontend config/store. - Updates Markdown rendering to disable raw HTML, restrict link protocols, and (optionally) allow
<a href>plus security attributes. - Extends unit tests to cover link enable/disable behavior and link sanitization expectations.
Reviewed changes
Copilot reviewed 10 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/agents/chat_app/src/utils/markdown.ts | Adds link normalization/validation, disables raw HTML, and conditionally allows anchor tags via DOMPurify. |
| src/agents/chat_app/src/utils/tests/markdown.test.ts | Adds/updates tests for link rendering, protocol blocking, and sanitization behavior. |
| src/agents/chat_app/src/types.ts | Adds allow_links to the frontend Config type. |
| src/agents/chat_app/src/test/factories/messages.ts | Extends createConfig factory to include allow_links. |
| src/agents/chat_app/src/stores/chat.ts | Exposes allowLinks getter and adds default allow_links in configuration fallback. |
| src/agents/chat_app/src/stores/tests/chat.test.ts | Adds a test for the new allowLinks getter. |
| src/agents/chat_app/src/components/chat/tests/ChatInterface.test.ts | Updates config fixtures to include allow_links. |
| src/agents/chat_app/src/components/chat/MarkdownRenderer.vue | Uses store-driven allowLinks to toggle link rendering; adds link styling. |
| src/agents/chat_app/dist/index.html | Updates built asset references. |
| src/agents/chat_app/dist/assets/index-BdMpKhWI.css | Updates built CSS output (includes link styling). |
| src/agents/base.py | Adds an advanced “Content Settings” group with allow_links parameter and passes it into the widget config. |
| src/agents/_data_service.py | Adds allow_links to widget config and includes it in get_configuration() payload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| /^(?:www\.)?[a-z0-9-]+(?:\.[a-z0-9-]+)+(?:[/?#][^\s]*)?$/i.test(trimmed) | ||
| ) { | ||
| // handles bare domains in markdown links as they are not handled by linkify | ||
| return `https://${trimmed}`; |
There was a problem hiding this comment.
normalizeLink only prepends https:// for a narrow “bare domain” regex, which won’t match valid URLs like example.com:8080/path (port) or other common host formats. With allowLinks=true those markdown links will still fail validation and won’t render as anchors. Consider normalizing more robustly (e.g., attempt new URL('https://' + trimmed) when no scheme) or extend the pattern to support ports.
| if ( | |
| /^(?:www\.)?[a-z0-9-]+(?:\.[a-z0-9-]+)+(?:[/?#][^\s]*)?$/i.test(trimmed) | |
| ) { | |
| // handles bare domains in markdown links as they are not handled by linkify | |
| return `https://${trimmed}`; | |
| try { | |
| // handles scheme-less domains in markdown links as they are not handled by linkify | |
| const normalizedUrl = new URL(`https://${trimmed}`); | |
| if (normalizedUrl.hostname) { | |
| return normalizedUrl.toString(); | |
| } | |
| } catch { | |
| // fall through and keep the original value when it cannot be normalized |
| md.validateLink = (url: string) => { | ||
| if (!/^https?:\/\//i.test(url)) { | ||
| return false; | ||
| } | ||
| try { | ||
| // block same-origin links to avoid crash | ||
| const urlObj = new URL(url); | ||
| if (urlObj.origin === window.location.origin) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
validateLink currently blocks all same-origin http(s) links. The PR description mentions restricting to http(s) protocols, but doesn’t mention also disallowing same-origin URLs; this changes behavior beyond the stated scope and may prevent linking to internal docs served from the same origin. If the crash is the real driver, consider making this restriction configurable or documenting it explicitly.
| const action = navigatorUtils.isMac() ? "Command" : "Control"; | ||
| node.setAttribute("target", "_blank"); | ||
| node.setAttribute("rel", "noopener noreferrer"); | ||
| node.setAttribute("title", `${displayUrl}\n${action} + Click to open link`); | ||
| }); |
There was a problem hiding this comment.
The sanitization config only allows the href attribute for links, but the DOMPurify hook later injects target, rel, and title anyway. This means the final HTML can contain attributes that aren’t represented in the allowlist, which makes the sanitizer policy harder to reason about. Consider explicitly adding target, rel, and title to ALLOWED_ATTR when allowLinks is enabled (or otherwise documenting that the hook intentionally bypasses the allowlist).
| () => config.value?.show_tool_calls_and_results, | ||
| ); | ||
|
|
||
| const allowLinks = computed(() => config.value?.allow_links); |
There was a problem hiding this comment.
allowLinks is computed as config.value?.allow_links, so it can be undefined until config is loaded. Since this getter is consumed as a boolean (and the config field itself is boolean), consider coercing it to a stable boolean (e.g. !!config.value?.allow_links) so callers don’t need to add fallbacks.
| const allowLinks = computed(() => config.value?.allow_links); | |
| const allowLinks = computed(() => !!config.value?.allow_links); |
Adds an optional parameter that renders all URLs as clickable links (only http(s) protocol) in the Chat Widget (not in the output column). This affects all uses of the MarkdownRenderer (AI/User messages, reasoning, tool messages).
The commit also disables raw html and the style attribute, which were previously allowed.
The wiki link is underlined because the mouse hovered above it. The screenshot does not show the tooltip, which contains the actual https link and an instruction to press control + click. The need to press control is caused by the target _blank attribute, probably because of how the view context handles it.