Skills M4 (4/9): run display-review on the shared review toolset

[valentinpalkovic]

Closes #

Part 4 of 9 of the Skills M4 migration. Stacked on #35729, replaces #35677.

What I did

The display-review tool hands a set of stories to Storybook's review UI so a human can walk through what an agent changed. Until now it did that indirectly: the MCP server emitted an event, the manager picked it up over a channel, and the manager was the one that actually created the review. It now calls the review service in the same process, and both the event and its consumer are gone.

Behavior change worth challenging: when applying the review to the review service failed, the old path swallowed it with a warning in the server log. The agent was handed a URL and told the review had been displayed, even though the page it pointed at had never received anything. Those failures now reach the agent as errors. To be precise about the scope, this is not about story IDs that do not exist - those were already validated up front and already produced an error. It is the rest: the review service being unavailable, or anything going wrong while the state is applied.

The producer and the consumer have to change in the same commit. Split apart, either reviews stop arriving in the manager or the tool emits into nothing.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

1. Build the monorepo packages with yarn nx run-many -t compile.
2. In test-storybooks/mcp, run yarn install and then yarn storybook. That Storybook has the review feature enabled and its MCP endpoint comes up at http://localhost:6006/mcp.
3. Point an MCP client at the endpoint and call display-review with a few valid story IDs. You should get back a review URL, and opening it in the browser should show exactly those stories in the review UI, as before.
4. Call it again with a story ID that does not exist. The error message naming the unknown IDs should be unchanged.
5. Confirm the tool is not offered at all in a Storybook that does not have the review feature enabled.

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

• Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

[github-actions]

	Fails
🚫	PR is not labeled with one of: ["ci:normal","ci:merged","ci:daily","ci:docs"]
🚫	PR is not labeled with one of: ["qa:needed","qa:skip","qa:success"]
🚫	PR title must be in the format of "Area: Summary", With both Area and Summary starting with a capital letter Good examples: - "Docs: Describe Canvas Doc Block" - "Svelte: Support Svelte v4" Bad examples: - "add new api docs" - "fix: Svelte 4 support" - "Vue: improve docs"
🚫	This PR needs an approving review from a Storybook Core or Developer Experience team member before it can be merged.

	Warnings
⚠️	This PR targets valentin/m4-03-stories-preview. The default branch for contributions is next. Please make sure you are targeting the correct branch.

Generated by 🚫 dangerJS against 0b4f55c

[coderabbitai]

Walkthrough

The review flow now uses the shared core reviewToolset for MCP and CLI contexts. The change adds structured results and recovery guidance, updates MCP registration, and removes the legacy display-review tool, review channel, and PUSH_REVIEW event.

Changes

Review toolset migration

Layer / File(s)	Summary
Core review toolset contract and behavior code/core/src/shared/open-service/toolsets/review/definition.ts, code/core/src/server-errors.ts, code/core/src/shared/open-service/toolsets/review/definition.test.ts	The review.create tool now uses explicit input and output schemas, returns review and story counts, builds URLs from uiRoot when available, renders consumer-specific guidance, and provides recovery messages for unknown story IDs. Tests cover CLI and MCP behavior.
MCP toolset registration and metadata code/core/src/core-server/index.ts, code/addons/mcp/src/test-support/register-core-toolsets.ts, code/addons/mcp/src/tools/tool-registry.ts, code/addons/mcp/src/tools/toolset-tools.ts	MCP registration now uses the exported reviewToolset with request-level enablement, friendly validation errors, and optional input-schema wrapping.
Legacy review adapter removal code/addons/mcp/src/constants.ts, code/core/src/core-server/presets/common-preset.ts, code/core/src/shared/review/events.ts, code/core/src/shared/review/review-state.ts	The legacy display-review tool and review channel wiring are removed. PUSH_REVIEW is removed from review events, and review-state documentation describes the core service flow.

Sequence Diagram(s)

sequenceDiagram
  participant MCPClient
  participant review.create
  participant coreReviewService
  participant StorybookUI
  MCPClient->>review.create: Submit review input
  review.create->>coreReviewService: Publish review
  coreReviewService-->>review.create: Return review state
  review.create-->>MCPClient: Return review URL and counts
  MCPClient->>StorybookUI: Open review URL

Loading

Possibly related issues

• Skills M4: addon-mcp and @storybook/mcp run on the shared core toolsets #35673 — Covers the shared core toolset migration and removal of legacy review adapters.
• Finish Milestone 2 (PR #35516): toolsets into OSA via registerToolset, ctx.format, review state fully in OSA, MCP-parity docs output #35656 — Covers migration of review functionality to core toolsets and removal of legacy review channels.

Possibly related PRs

• storybookjs/storybook#35727 — Migrates MCP review functionality to the shared core reviewToolset.
• storybookjs/storybook#35729 — Updates MCP tool registration through shared toolset infrastructure.
• storybookjs/storybook#35516 — Modifies the review toolset and transition away from legacy review handling.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

code/core/src/shared/open-service/toolsets/review/definition.test.ts (1)

41-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use vi.mocked() for setReview.

The setup accesses setReview directly and casts getService to ToolsetCtx['getService']. Type the mock as the ReviewService command and access its implementation through vi.mocked(setReview). This keeps the mock return type aligned with the service contract.

As per coding guidelines, “Use vi.mocked() to type and access the mocked functions in Vitest tests” and “Avoid direct function mocking without vi.mocked() in Vitest tests.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/core/src/shared/open-service/toolsets/review/definition.test.ts` around
lines 41 - 54, Update the beforeEach setup for setReview to use
vi.mocked(setReview) for configuring its implementation, and type the mocked
service command as the ReviewService contract. Remove the direct getService cast
by exposing the correctly typed mocked command through the existing cliCtx
setup, preserving the current serviceError behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@code/core/src/shared/open-service/toolsets/review/definition.test.ts`:
- Around line 41-54: Update the beforeEach setup for setReview to use
vi.mocked(setReview) for configuring its implementation, and type the mocked
service command as the ReviewService contract. Remove the direct getService cast
by exposing the correctly typed mocked command through the existing cliCtx
setup, preserving the current serviceError behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ff3713f5-aa46-4014-a3fd-42ab9a9a22e0

📥 Commits

Reviewing files that changed from the base of the PR and between 605c51b and 0b4f55c.

📒 Files selected for processing (15)

• code/addons/mcp/src/constants.ts
• code/addons/mcp/src/test-support/register-core-toolsets.ts
• code/addons/mcp/src/tools/display-review.test.ts
• code/addons/mcp/src/tools/display-review.ts
• code/addons/mcp/src/tools/tool-registry.ts
• code/addons/mcp/src/tools/toolset-tools.ts
• code/core/src/core-server/index.ts
• code/core/src/core-server/presets/common-preset.ts
• code/core/src/core-server/server-channel/review-channel.test.ts
• code/core/src/core-server/server-channel/review-channel.ts
• code/core/src/server-errors.ts
• code/core/src/shared/open-service/toolsets/review/definition.test.ts
• code/core/src/shared/open-service/toolsets/review/definition.ts
• code/core/src/shared/review/events.ts
• code/core/src/shared/review/review-state.ts

💤 Files with no reviewable changes (6)

• code/addons/mcp/src/constants.ts
• code/addons/mcp/src/tools/display-review.test.ts
• code/core/src/core-server/server-channel/review-channel.test.ts
• code/addons/mcp/src/tools/display-review.ts
• code/core/src/core-server/server-channel/review-channel.ts
• code/core/src/core-server/presets/common-preset.ts