feat(toolkit-core): Add shared Markdown builders for untrusted values by sentry-junior[bot] · Pull Request #1795 · getsentry/toolkit

sentry-junior · 2026-11-02T11:57:06Z

Adds @sentry/toolkit-core/markdown, one place for the CLI and MCP to put upstream data into Markdown without hand-rolled escaping, and moves MCP's issue and event output onto it (#1797, merged into this branch). This is the first step of the plan that came out of the escaping review on #1293.

The helper is picked by what the value is, not by which characters it contains:

helper	for	how
`inlineCode`	identifiers, slugs, queries, short values	fence longer than any backtick run; padding keeps a leading/trailing backtick or space; line breaks become spaces
`codeBlock`	messages, stack traces, JSON	fence (≥3) longer than any backtick run, so no line of the value can close the block
`text`	values that should read as prose	backslash escapes only, no runtime deps; line breaks become spaces

The threat in MCP is structural: upstream text breaking out of its list item or block and adding headings or sections, for example a fake ## Response Notes. Each helper keeps the value inside the line or block it is placed in. inlineCode needs no escaping, so it is the default for MCP output; models read the value unchanged.

Same library as the CLI: toolkit-core has no Markdown runtime dependencies. text escapes CommonMark/GFM inline syntax, line-start syntax (headings, lists, quotes, rules), and the :/./@ that would start a literal autolink, so URLs, www. domains, and emails stay plain text. It never writes character references, because marked keeps them as-is in text tokens and the CLI would print w. marked moves to the workspace catalog, so the CLI and toolkit-core's tests use the same version.

Property tests parse each helper's output with both marked (the CLI's parser) and micromark (CommonMark reference, with GFM), placed mid-line and at the end of a list item, and check that any input round-trips as the same literal value with no other structure. They pass at 20,000 runs locally; CI runs 500.

Consumers: MCP's two formatInlineCode copies use inlineCode; mcp-core's formatting.ts uses codeBlock for messages, exception values, frames, and queries, text for titles and culprits, and inlineCode for tag values. The CLI's escapeMarkdownInline, escapeMarkdownCell, and safeCodeSpan are not swapped in this PR: the CLI wants URLs in escaped text to stay clickable autolinks (renderOneInline strips escapes inside them), while text blocks autolinks on purpose, and the cell helpers need pipe handling that text and inlineCode don't do. That move needs a table-cell helper and an autolink decision, so it comes next.

Dependencies: marked (catalog), mdast-util-from-markdown, mdast-util-gfm, and micromark-extension-gfm are toolkit-core dev deps for the test oracles. The lockfile also moves unist-util-visit from 5.0.0 to 5.1.0 as part of that resolution.

Docs: toolkit-core README section and a bullet in docs/contributing/tool-responses.md under "Formatting Upstream Data".

--

View Session in Sentry

Add `@sentry/toolkit-core/markdown` so the CLI and MCP render upstream data without hand-rolled escaping. Pick the helper by what the value is: - `inlineCode` for identifiers and short values. The fence is longer than any backtick run, padding keeps leading or trailing backticks and spaces, and line breaks become spaces. - `codeBlock` for multi-line data. The fence is longer than any backtick run, so no line of the value can close the block. - `text` for prose. It escapes with mdast-util-to-markdown and its GFM extension, and writes the first `w` of a `www.` domain as a character reference, since GFM reads that autolink from the raw source. Property tests parse each helper's output with micromark and check that any input round-trips as the same literal value with no other structure. They found the `www.` autolink gap. They also cover two gaps in MCP's existing code span helper: it dropped a leading and trailing space, and a line break in the value ended the line. MCP's two copies of `formatInlineCode` now use `inlineCode`. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>

vercel · 2026-11-02T11:57:12Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
sentry-local	Skipped		Nov 2, 2026 1:13pm UTC

CommonMark never treats an underscore between two letters or digits as emphasis, so escaping it only makes snake_case names harder to read. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>

BYK

Would it be possible to use the same libraries between the CLI and mcp-core to increase code-share?

@BYK

…utput (#1797) Stacked on #1795. It moves mcp-core's issue and event formatting (`internal/formatting.ts`) onto `@sentry/toolkit-core/markdown`. **Problem.** Issue and event details put upstream values straight into Markdown: - A message, exception value, or stack frame containing ```` ``` ```` closed its fixed-length fence. - A title, culprit, or chained exception value with a line break started a new heading or list item, for example a fake `## Response Notes` section. **Changes** | value | helper | |---|---| | exception values, messages, stack frames, CSP original policy, N+1 query pattern, span trees | `codeBlock` | | chained exception headings, issue description, location, culprit, assignee and project names, tag keys | `text` | | tag values, performance query pattern (was a hand-built `` `${value}` `` span) | `inlineCode` | **Visible output change: tag values are now code spans** (`**level**: `error``). `text` would have escaped every URL tag as `https\://…`, which reads worse for models and is easy to copy wrongly. Code spans keep the value unchanged. Other snapshots are unchanged; #1795 also leaves `snake_case` underscores unescaped in `text`. **Not in this PR** - The "Most Relevant Frame" section still prints context lines outside a code block. Changing that alters its layout. - Fences in other tools (`analyze-issue-with-seer`, `create-project`, `breadcrumbs`, …) and the CLI helpers will follow. **Tests:** regression tests cover a message that closes its fence and injects a section, a chained exception value with a line break, and an issue title, culprit, and tag carrying Markdown. All mcp-core tests pass locally, and `tsc` and lint are clean.  via @BYK.    -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A67232d03908f6d80361e0c2c/?project=4510944073809921)  Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 3c913b0. Configure here.}

…ith the CLI `text` now escapes with backslashes only, so toolkit-core has no Markdown runtime dependencies. Property tests parse the output with `marked`, the parser the CLI renders with, as well as micromark. `marked` moves to the workspace catalog so the CLI and toolkit-core tests use the same version. Backslash escapes also keep literal URLs, `www.` domains, and emails from linking, and avoid character references, which `marked` leaves as-is in text tokens.

sentry-junior · 2026-11-02T12:48:19Z

Addressed in 2b3652e, mostly. toolkit-core now uses the same Markdown library as the CLI: text dropped mdast-util-to-markdown/mdast-util-gfm for backslash-only escaping with no runtime deps, and its property tests parse the output with marked (now in the workspace catalog, so both packages pin the same version) as well as micromark.

I did not point the CLI's escapeMarkdownInline at text in this PR. I tried it: CLI formatter and release tests pass apart from the intraword-underscore case. But text blocks literal autolinks on purpose (http\://), and the CLI relies on escaped URLs staying clickable (renderOneInline strips escapes inside autolinks). safeCodeSpan/escapeMarkdownCell also need pipe handling that the shared helpers don't do yet. That move needs a table-cell helper and an autolink decision for the CLI, and it touches ~114 CLI call sites, so it fits better in its own PR.

A code span cannot be empty, and a null tag value printed as `null`.

A value that ends in a space and a run of `#` lost the hashes when placed in an ATX heading, as chained exception titles are. Property tests now place `text` in a heading too.

Follow the untrusted-value policy from #1795: the monitor's aggregate, query, environment, raw dataset, and the pre-filled tool calls now go through inlineCode, so a value with backticks can't break out of its span. Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com>

@BYK

…ach data (#1715) Fixes #1087. Metric issue output only showed the evaluated value and threshold. The event API camelCases the whole occurrence payload (`convert_dict_key_case(occurrence.to_dict(), snake_to_camel_case)` in sentry's `api/serializers/models/event.py`). The formatter from #1129 read snake_case keys (`data_sources`, `query_obj`, `snuba_query`, `alert_id`), so the dataset, aggregate, query, interval and environment never rendered. Only single-word keys like `value` and `conditions` matched. The #1129 tests used hand-written snake_case fixtures, so they passed anyway. As of current `main` (1b27c0f), `formatMetricAlertDetails` still reads the snake_case keys, and the sentry serializer still camelCases. Agents also had no next step. The Response Notes suggested stacktrace and trace lookups, which don't apply to metric issues: the event's trace context comes from the monitor evaluation, not from the data that crossed the threshold. **Changes** - Read `dataSources[].queryObj.snubaQuery`, `detectorId` and the legacy `alertId`. - Show the Metric Monitor ID, a readable dataset label (Errors, Logs, Spans or Application metrics, following the UI's `getDetectorDataset` mapping) and the evaluated window (`timeWindow` before the occurrence). - For metric issues, Response Notes now include: - `get_metric_monitor_details(organizationSlug, monitorId)` for the monitor configuration and connected Alerts. - A ready-to-run `search_errors`, `search_logs`, `search_traces` or `search_metrics` call, chosen by dataset. It is pre-filled with the monitor's query plus an `environment:` filter, the project, and a relative `period` that reaches back to the evaluated window. The window's start and end are shown next to it. A query that uses `OR` is wrapped in parentheses before the environment filter is added, because implicit AND binds tighter than OR. - Add an opt-in `showArguments` to `formatToolCallInstruction` so these notes print the full call. Previously it ignored `arguments`. - Render the monitor's aggregate, query, environment, raw dataset and the tool calls with `inlineCode`, following the untrusted-value policy from #1795. - Rebuild the metric tests from the real camelCased event shape, and add Response Notes coverage. **Example (logs monitor)** ``` **Monitor ID**: 1234567 **Dataset**: Logs **Aggregate**: `count(message)` **Query**: `severity:error` **Interval**: 3600 second(s) **Environment**: `prod` **Evaluated Window**: 2026-10-28T10:39:52.000Z to 2026-10-28T11:39:52.000Z **Evaluated Value**: 4988 **Threshold**: above 4000 ... - Metric Monitor configuration and connected Alerts: Use the Sentry tool `get_metric_monitor_details(organizationSlug='sentry', monitorId='1234567')` - Logs matched by the monitor query: Use the Sentry tool `search_logs(organizationSlug='sentry', projectSlug='seer', query='severity:error environment:prod', period='16h')`, then focus on 2026-10-28T10:39:52.000Z to 2026-10-28T11:39:52.000Z ``` **Notes** - The search call is left out for `transactions`, `generic_metrics` and crash-free (`metrics`/`sessions`) monitors. Their query syntax doesn't map cleanly onto the current search tools. The monitor ID and details note still render. - The search tools only accept a relative `period`, so the call can return data after the breach too. That's why the note gives the exact window to focus on. - The fixture shape comes from the sentry serializers (`EvidenceData` in `workflow_engine/handlers/detector/base.py`, `DataSourceSerializer` and `SnubaQuerySerializer`). It has not been checked against a captured production payload. **Tests** - `pnpm exec vitest run` in `packages/mcp-core`: 1913 passed, 6 skipped - `tsc --noEmit` (mcp-core) and `pnpm run lint` (oxlint, oxfmt, ast-grep, workspace dependency check): clean  via @BYK.    -- [View Session](https://junior-prod.sentry.dev/conversations/slack%3AC08J1NSPU6S%3A1793241227.769199) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC08J1NSPU6S%3A1793241227.769199/?project=4510944073809921)  --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Burak Yigit Kaya <126780+BYK@users.noreply.github.com>

@BYK

Follow-up to #1795. The CLI's three escape helpers are now the shared builders in `@sentry/toolkit-core/markdown`, so CLI and MCP escape untrusted values the same way, checked by property tests against the same `marked` version the CLI renders with. | CLI helper | now | was | |---|---|---| | `escapeMarkdownInline` | `text` | escaped `\ * _ ` [ ] < >` | | `escapeMarkdownCell` | `tableCell` (new): `text` plus escaped pipes | escaped `\ \| < >` and newlines only | | `safeCodeSpan` | `inlineCode` | single-backtick span, `` ` `` → `ˋ`, `\|` → `│` | The names stay, so the ~220 call sites don't change. (`text` would also shadow a lot of local `text` variables.) **`tableCell`** escapes each `|` after `text` has doubled every backslash, so every `\|` is an escaped pipe, never an escaped backslash followed by a cell delimiter. New property tests put it in a two-column GFM table and check that, with both `marked` and micromark, the value comes back intact and the next cell is untouched. They fail on `"|"` if the pipe escape is removed. **Behavior changes in CLI output** - Cells now escape emphasis, links, code spans, and line-start syntax, not just pipes and angle brackets. A dashboard or monitor name like `*prod*` no longer renders in italics. - Code spans keep backticks and pipes instead of swapping in look-alike characters. Pipes in key-value tables still become `│`: `mdKvTable` does that for table structure. - Underscores inside words stay unescaped (`hello_world`). - **Autolinks:** URLs in escaped text render as plain text, not clickable OSC 8 links. Untrusted text shouldn't create links. Deliberate links still use explicit link syntax, and most terminals detect plain URLs on their own. I kept `renderOneInline`'s escape-stripping for autolinks, because raw Markdown from other sources can still reach it. - `text` trims a value and puts it on one line. For the four multi-line callers there's a new `escapeMarkdownLines`, which keeps line breaks and up to three spaces of indent: blockquoted issue and log messages, `cli fix` instructions, and credential errors. **Fixes along the way** - `mdKvTable` turned an escaped `\|` into `\│`, so a backslash showed. It now drops the escape. Even runs of backslashes are kept. - Replay tags and breadcrumb labels put escaped text inside hand-written backticks, so the backslashes showed. They now use `safeCodeSpan`. - The offline-downgrade message passed `" (offline, from cache)"` through the escaper, and `text` would trim its leading space. It's a plain literal now. **Verification:** toolkit-core 372 tests, mcp-core 1897 (6 skipped), CLI 13002 passed. `tsc` and `lint` (including the workspace dependency check) are clean. Six CLI tests fail in my sandbox with or without this change: permission/readonly cases (EACCES, EPERM, readonly DB, unreadable file, lock) that need a non-privileged user. CI runs them as one.  via @BYK.    -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A67232d03908f6d80361e0c2c/?project=4510944073809921)  --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>

sentry-junior Bot assigned BYK Nov 2, 2026

sentry-junior Bot temporarily deployed to Actions November 2, 2026 11:57 Inactive

github-actions Bot added the risk: medium PR risk score: medium label Nov 2, 2026

vercel Bot deployed to Preview November 2, 2026 11:58

View deployment

feat(toolkit-core): Leave intraword underscores unescaped in text

9a010c7

CommonMark never treats an underscore between two letters or digits as emphasis, so escaping it only makes snake_case names harder to read. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>

vercel Bot temporarily deployed to Preview November 2, 2026 12:00 Inactive

sentry-junior Bot temporarily deployed to Actions November 2, 2026 12:00 Inactive

BYK requested changes Nov 2, 2026

View reviewed changes

sentry-junior Bot mentioned this pull request Nov 2, 2026

fix(mcp): Keep upstream data inside its Markdown construct in event output #1797

Merged

vercel Bot temporarily deployed to Preview November 2, 2026 12:27 Inactive

BYK temporarily deployed to Actions November 2, 2026 12:27 — with GitHub Actions Inactive

cursor Bot reviewed Nov 2, 2026

View reviewed changes

Comment thread packages/mcp-core/src/internal/formatting.ts Resolved

sentry-junior Bot temporarily deployed to Actions November 2, 2026 12:47 Inactive

vercel Bot deployed to Preview November 2, 2026 12:48

View deployment

fix(mcp): Write a placeholder for null and empty tag values

252823b

A code span cannot be empty, and a null tag value printed as `null`.

vercel Bot temporarily deployed to Preview November 2, 2026 12:50 Inactive

sentry-junior Bot temporarily deployed to Actions November 2, 2026 12:50 Inactive

sentry-warden Bot reviewed Nov 2, 2026

View reviewed changes

Comment thread packages/toolkit-core/src/markdown.ts Resolved

BYK approved these changes Nov 2, 2026

View reviewed changes

fix(toolkit-core): Escape trailing hashes that would close a heading

3f0264e

A value that ends in a space and a run of `#` lost the hashes when placed in an ATX heading, as chained exception titles are. Property tests now place `text` in a heading too.

vercel Bot temporarily deployed to Preview November 2, 2026 13:13 Inactive

sentry-junior Bot deployed to Actions November 2, 2026 13:13 Active

BYK enabled auto-merge (squash) November 2, 2026 13:16

BYK merged commit 96f5c9b into main Nov 2, 2026
39 checks passed

BYK deleted the feat/toolkit-core-markdown branch November 2, 2026 13:24

sentry-junior Bot mentioned this pull request Nov 2, 2026

ref(cli): Escape Markdown with the shared toolkit-core builders #1805

Merged

sentry-junior Bot mentioned this pull request Nov 2, 2026

fix(formatting): read camelCase metric alert evidence and link to breach data #1715

Merged

Navigation Menu

Repository navigation

feat(toolkit-core): Add shared Markdown builders for untrusted values - #1795

feat(toolkit-core): Add shared Markdown builders for untrusted values#1795
BYK merged 6 commits into
mainfrom
feat/toolkit-core-markdown

sentry-junior Bot commented Nov 2, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Nov 2, 2026 •

edited

Loading

Uh oh!

BYK left a comment

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

sentry-junior Bot commented Nov 2, 2026

Uh oh!

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sentry-junior Bot commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sentry-junior Bot commented Nov 2, 2026

Uh oh!

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sentry-junior Bot commented Nov 2, 2026 •

edited

Loading

vercel Bot commented Nov 2, 2026 •

edited

Loading