Repository navigation
fix(mcp): Keep upstream data inside its Markdown construct in event output - #1797
Merged
Merged
Conversation
…utput Issue and event details interpolated upstream values straight into Markdown. A message or frame containing a fence could close its code block, and a title, culprit, or tag with a line break could start a new heading or list item, for example a fake `## Response Notes` section. Render these values with `@sentry/toolkit-core/markdown`: - `codeBlock` for exception values, messages, stack frames, CSP policies, N+1 query patterns, and span trees - `text` for chained exception headings, issue descriptions, locations, culprits, assignee and project names, and tag keys - `inlineCode` for tag values and the performance query pattern, which was a hand-built code span Tag values now render as code spans, so they read unchanged; `text` would have escaped every URL as `https\://`. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
BYK
approved these changes
Nov 2, 2026
BYK
added a commit
that referenced
this pull request
Nov 2, 2026
…#1795) 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". <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3A67232d03908f6d80361e0c2c --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A67232d03908f6d80361e0c2c/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>
BYK
added a commit
that referenced
this pull request
Nov 3, 2026
The `Default` ruleset on `main` (and `Release branches` on `release/**`) requires one status check: `test` from GitHub Actions. That is meant to be the aggregate `test` job in `test.yml`, which `needs` discovery, install, quality, every project matrix job, the npm runtime jobs, and the CLI asset catalog. `pr-risk-labels-test.yml` also had a job with the id `test` and no `name`, so its check run was also called `test` and came from the same app. On a PR that touches the risk label workflow or its tests, that short job could satisfy the required check before the full Test workflow finished. This PR gives that job its own name and records in `docs/operations/github-actions.md` that the `test` check name is reserved. This PR does not change the rulesets. They require only `~DEFAULT_BRANCH` and `release/**`, so PRs that target other branches, such as stacked PRs, have no required checks. That is how #1797 merged into `feat/toolkit-core-markdown` while `sentry` and `test` were still running. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3A7ce4fa19419509f244098772 --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A7ce4fa19419509f244098772/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
```closed its fixed-length fence.## Response Notessection.Changes
codeBlocktext`${value}`span)inlineCodeVisible output change: tag values are now code spans (
**level**:error``).textwould 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
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
tscand lint are clean.via @BYK.
--
View Session in Sentry