Skip to content

Finish Milestone 2 (PR #35516): toolsets into OSA via registerToolset, ctx.format, review state fully in OSA, MCP-parity docs output #35656

Description

@kasperpeulen

Spec for bringing PR #35516 (Milestone 2 of #35526) to its agreed end-state. All work lands in that same PR; this is the single spec issue for it.

Problem Statement

Milestone 2 established defineToolset and internal-only OSA services, but review of PR #35516 surfaced structural drift from the agreed architecture:

  • Toolsets live outside OSA under a separately named public entry, while the review toolset sits in a third place — the filesystem contradicts the "services and toolsets are both OSA" thesis.
  • Output format is a per-method json schema flag, contradicting the agreed principle that methods never declare the format; adapters own it.
  • Review state is still split: payload and staleness live in the OSA service, but deferred updates and display state live in a manager-side store — exactly the dual-state the OSA effort exists to eliminate.
  • The docs toolset's Markdown silently diverges from @storybook/mcp's get-documentation output (no props section, no story cap, no subcomponents), which would turn the Milestone 4 adapter swap into a silent output regression.
  • The diff carries unrelated noise: an addon-vitest enum workaround empirically shown to be unnecessary, an export-reorder, and agent working documents in the public docs tree.

Solution

One PR brings Milestone 2 to the consensus state:

  • OSA hosts two sibling constructs behind its single entry: services (internal state sync) and toolsets (public agent surface). The separate public-api package export disappears.
  • Toolsets register imperatively via registerToolset, called from the existing services preset hook — no new preset property. Adapters read the registered toolsets from the OSA registry.
  • Handlers read ctx.format: 'markdown' | 'json'; the json input flag is gone from every method schema.
  • The core/review OSA service owns all review data: current payload, pending (deferred) payload, and staleness — the manager becomes a pure projection.
  • The docs toolset's Markdown is produced by a verbatim port of the @storybook/mcp manifest formatter over the same manifest shape core already builds, making Milestone 4 output-neutral by construction.
  • The diff is cleaned of everything unrelated.

User Stories

  1. As an AI agent consuming Storybook via MCP, I want documentation output to be byte-identical before and after addon-mcp migrates onto toolsets (Milestone 4), so that my prompts and parsers keep working unchanged.
  2. As an AI agent, I want component documentation to include the TypeScript props section, capped example stories, and subcomponents, so that I don't hallucinate prop names or miss variants.
  3. As a CLI or MCP consumer, I want Markdown by default and structured JSON on request via a single format switch, so that I choose the representation per call without the method schema knowing about it.
  4. As a CLI consumer, I want CLI Markdown aligned with MCP output as the standard, so that documentation reads the same everywhere unless a CLI-specific need arises.
  5. As an addon author, I want to contribute a toolset with one registerToolset call from my existing services preset hook, so that services and toolsets share one contribution surface.
  6. As a core maintainer, I want a service's registration helper to be able to register its 1:1 toolset behind the scenes, so that paired service+toolset stay together.
  7. As a future contributor, I want toolset registration to not depend on the Node preset system, so that manager- or preview-realm toolsets can use the same API later.
  8. As a core maintainer, I want services and toolsets in mirrored trees behind one OSA entry, so that the architecture is legible from the filesystem.
  9. As a Storybook user with the review feature disabled, I want no review toolset registered, so that feature gating works identically for services and toolsets.
  10. As a reviewer using the review UI, I want accepting or dismissing a review in one browser tab to propagate to all connected tabs, so that review state has one source of truth.
  11. As a reviewer, I want an incoming updated review to be held as a pending update until I accept it, so that my in-progress review isn't yanked out from under me.
  12. As a reviewer, I want the review to be marked stale when story-affecting files change (after the grace window), so that I don't review outdated state.
  13. As a reviewer, I want review mode to remain per-browser-tab, so that opening review mode in one tab doesn't force every tab or teammate into it.
  14. As an agent creating a review via the toolset, I want unknown story ids rejected with a typed error, so that reviews never reference nonexistent stories.
  15. As a maintainer, I want the legacy review channel reduced to thin event adapters, so that review domain logic lives only in the OSA service.
  16. As a maintainer reviewing the PR, I want the diff free of unrelated changes (enum workaround, export reorder, agent working docs), so that review effort goes to the architecture.
  17. As a Milestone 4 implementer, I want ported parity fixtures asserting the MCP Markdown path, so that I can swap addon-mcp onto toolsets with proof of unchanged output.
  18. As a toolset author, I want ctx.origin typed as optional, so that the type reflects that a CLI outside a running Storybook has no origin.
  19. As a maintainer, I want double application of the services hook to fail with a typed Storybook error, so that misconfiguration is diagnosable.

Implementation Decisions

  • Toolsets are part of OSA. The OSA module exposes both defineService/registerService and defineToolset/registerToolset. The dedicated public-api package export and its build entry are removed; everything exports from the existing open-service entry. All four toolsets (docs, stories, test, review) live in one toolsets tree mirroring the services tree.

  • Registration is imperative, riding the existing services preset hook (decided with Jeppe, 2026-07-29): no new preset property; the experimental_toolsets preset property is deleted. Rationale: future manager/preview-realm toolsets cannot depend on the Node preset API. Where a service and toolset map 1:1, the service's registration helper may register both. Adapters obtain toolsets via a registry accessor (e.g. getRegisteredToolsets()); nothing consumes it until Milestones 4/5. Shape:

    // addon preset (or core's own services hook)
    export const services = async (_, options) => {
      registerTestService(options);   // OSA service, as today
      registerToolset(testToolset);   // toolset joins the same hook
    };
    
    // adapter (M4/M5)
    const toolsets = getRegisteredToolsets();
  • Feature gating: the review toolset registers inside the same gated branch that registers the review service, so a disabled review feature registers neither.

  • Output format: ToolsetCtx gains a required format: 'markdown' | 'json'; the json property is removed from all eight method schemas across docs, stories, test, and review. Adapters own the mapping (CLI --json flag, MCP json tool input). ctx.consumer remains for consumer-specific rendering.

  • Format/consumer contract: MCP consumer + Markdown is verbatim-bound to the ported @storybook/mcp formatter and is the only parity-tested cell. CLI Markdown uses the same formatter as its standard and may diverge only for a concrete need. JSON output is the structured mapped shape on every method, independent of MCP.

  • Docs Markdown parity by construction: the docs toolset builds its Markdown input through the existing core pipeline that merges docgen and story-docs payloads into the component-manifest shape (the same shape @storybook/mcp consumes), and renders it with a verbatim port of the @storybook/mcp manifest formatter — including the props section, the 3-story cap with remaining-story references, subcomponents, and docs-manifest formatting. The toolset's previous divergent Markdown formatter is deleted. The ported module carries a provenance note naming its source in @storybook/mcp, since the two copies must not drift until Milestone 4 deletes the original.

  • Review state fully in OSA: the review service state becomes { current, pending }. Setting a review while a different one is current defers it to pending; a new acceptPending command promotes it; dismissal clears both; the staleness grace window stays enforced inside the service. The service subscribes to the module-graph revision at registration time, replacing the channel-level staleness subscription. The legacy channel keeps only two thin adapters: applying pushed reviews and relaying dismissal navigation. The manager review store stops holding review payloads entirely — the UI projects from the service's live queries and issues commands; deferred-accept UI reads the pending query. Review-mode, auto-enter deduplication, exit transition, and session persistence remain per-tab manager state (OSA state is global across connected managers; review mode must not be).

  • Diff hygiene: the addon-vitest enum-to-string-literal change and its typings change are removed — three local probes (the package check, the same check under dist-types resolution against production-compiled core, and the production compile) all pass without them, and CI's no-link mode cannot change check resolution since the compiler's resolution conditions come from the unchanged shared tsconfig. The addon-docs preset export reorder is reverted. The agent working documents are removed from the public docs tree; their content lives in the team's Notion pages, linked from the PR description.

  • Smaller fixes: ToolsetCtx.origin becomes optional to match its runtime check; the double-application guard for the services hook throws a typed Storybook server error; the type-redundant service-contract key-list test is removed; the leftover test file named after a deleted module is folded into the merged module's test file.

  • PR housekeeping: the PR description is updated to the final architecture (including that the toolset registry is adapter-facing and consumed only from Milestone 4 on), and all nine open review threads are answered with what changed or why not.

Testing Decisions

  • Good tests assert externally observable contracts: rendered Markdown strings, structured JSON shapes, service command/query behavior, and typed errors — never internal wiring.
  • Parity suite: test fixtures ported verbatim from the @storybook/mcp formatter tests, asserting the MCP-consumer Markdown path only. This fulfils the standing decision that contract parity is proven by ported MCP tests.
  • Review service: unit tests for set/defer/accept/dismiss/staleness incl. the grace window, following the existing OSA server-test patterns (fixture-controlled clock configured in test setup; memfs conventions where the filesystem is involved).
  • Toolset handlers: tested through their definitions with a stubbed context, following the existing toolset API test pattern; ctx.format variants covered for each method.
  • Manager review UI: behavior changes to the provider/store projection are covered by updating the existing review stories with play functions (repo rule: React behavior lives in stories, not unit tests).
  • Staleness coverage moves from the channel tests to the service tests along with the logic; the channel tests shrink to the two remaining adapters.
  • No tests for anything TypeScript already guarantees.

Out of Scope

  • Moving the test toolset (and later a test service) to addon-vitest.
  • Registering the stories toolset (its boot-time dependencies — git, index, statuses — are not wired yet).
  • Milestone 4 (addon-mcp consuming toolsets, MCP tool-name mapping) and Milestone 5 (storybook tools CLI).
  • @storybook/mcp hosted-package code sharing (open decision record owned by Jeppe).
  • Manager/preview-realm toolsets (motivates the registration mechanism; not built now).
  • Making review mode global across tabs.

Further Notes

  • Target: merged before Friday (2026-08-01) to unblock the Frameworks workstream. Review-state work and the formatter port touch disjoint files and can proceed in parallel; pushes should be batched since CI runs are costly.
  • The PR's Chromatic UI review will need re-approval after the review-state changes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions