Route setup skills through a decision tree so existing Storybooks skip story generation - #364
Route setup skills through a decision tree so existing Storybooks skip story generation#364kasperpeulen wants to merge 7 commits into
Conversation
…p story generation 'Set up Storybook for Codex' (the plugin's default prompt) currently always runs 'npx storybook ai setup', which prints the full write-up-to-10-stories plan even when the project already has a working Storybook with its own stories — slow and expensive for exactly the users the plugin targets first. The setup skill is now a decision tree: init/upgrade when needed, ensure addon-mcp, and gate on user-written stories. Projects with existing non-example stories route to the stories skill (start and show Storybook) instead of story generation; only story-less projects get 'storybook ai setup', with addon-vitest ensured first since the printed plan silently assumes it.
|
✅ Deploy Preview for storybook-mcp-self-host-example canceled.
|
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #364 +/- ##
=======================================
Coverage 79.66% 79.66%
=======================================
Files 50 50
Lines 2095 2095
Branches 624 624
=======================================
Hits 1669 1669
Misses 220 220
Partials 206 206 ☔ View full report in Codecov by Harness. |
📝 WalkthroughWalkthroughStorybook skills now use explicit decision trees for initialization, upgrades, addon installation, and story generation. Agent evaluations and shared utilities verify these branches, preserve existing stories, validate addons, and detect genuine Storybook AI setup commands. Plugin tests cover Claude/Codex skill pairs. ChangesStorybook skill guidance
Agent evaluation harness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Eval
participant SetupSkill
participant StorybookAddons
participant StoryFiles
participant StorybookAI
Eval->>SetupSkill: execute setup scenario
SetupSkill->>StorybookAddons: install and register addon-mcp
SetupSkill->>StoryFiles: inspect existing story globs and files
SetupSkill->>StorybookAI: run AI setup when no user stories exist
Eval->>StoryFiles: verify files and exports
Possibly related issues
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
- Give the init skill sole ownership of the continuation (it already re-invokes setup), instead of setup claiming a resume point. - Detect user stories via the 'stories' globs in .storybook/main.* rather than assuming the *.stories.* naming convention. - Narrow the step-4 exception to bulk generation; single-component story requests route through the stories skill. - Soften hardcoded paths (custom config dirs, init boilerplate locations). - Extend plugin.test.ts so the setup skill twins share the stories skill's description-budget and cross-plugin-identical guarantees.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/claude-plugin/skills/storybook-setup/SKILL.md`:
- Line 8: Update the Storybook setup detection in
packages/claude-plugin/skills/storybook-setup/SKILL.md at line 8 and
packages/codex-plugin/plugins/storybook/skills/setup/SKILL.md at line 8 to
invoke the respective initialization command when either the Storybook
dependency or the Storybook configuration directory is missing, rather than only
when both are absent. Preserve the existing command-specific behavior and
handoff details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29fccdbf-8a1c-4abb-9636-bf59b0b88229
📒 Files selected for processing (2)
packages/claude-plugin/skills/storybook-setup/SKILL.mdpackages/codex-plugin/plugins/storybook/skills/setup/SKILL.md
| Run `npx storybook ai setup` from the project root (or the Storybook package in a monorepo). | ||
|
|
||
| **Follow the printed Markdown precisely.** Do not substitute your own plan. | ||
| 1. **Storybook missing?** No Storybook dependency in `package.json` and no Storybook config directory (usually `.storybook/`) → invoke `/storybook-init` first; it installs Storybook plus `@storybook/addon-mcp` and then hands control back to this skill. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Initialize incomplete Storybook setups, not only entirely absent ones.
packages/claude-plugin/skills/storybook-setup/SKILL.md#L8-L8: trigger/storybook-initwhen either the Storybook dependency or config is missing.packages/codex-plugin/plugins/storybook/skills/setup/SKILL.md#L8-L8: trigger$storybook:initwhen either the Storybook dependency or config is missing.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 10: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 11: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 12: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 12: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
📍 Affects 2 files
packages/claude-plugin/skills/storybook-setup/SKILL.md#L8-L8(this comment)packages/codex-plugin/plugins/storybook/skills/setup/SKILL.md#L8-L8
🤖 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 `@packages/claude-plugin/skills/storybook-setup/SKILL.md` at line 8, Update the
Storybook setup detection in
packages/claude-plugin/skills/storybook-setup/SKILL.md at line 8 and
packages/codex-plugin/plugins/storybook/skills/setup/SKILL.md at line 8 to
invoke the respective initialization command when either the Storybook
dependency or the Storybook configuration directory is missing, rather than only
when both are absent. Preserve the existing command-specific behavior and
handoff details.
New eval 824 pins the core behavior: a setup request against a current Storybook that already has a user-written story must not run story generation — the agent ensures @storybook/addon-mcp and Storybook still boots, while the story files and their exports stay exactly as seeded. Eval 823 gets the same story-gate assertions after its upgrade routing, with its seeded story moved to src/components so it cannot be mistaken for the init example stories the skill tells agents to ignore. Eval 820 now also asserts the positive branch: on a story-less fresh init the tree must ensure @storybook/addon-vitest and run `storybook ai setup`. The ai-setup assertions are token-aware (findStorybookAiSetupInvocations in shell-parse): a help call or a command that merely mentions the text does not count, and a correct agent that greps for the command does not false-fail.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@agent-eval/evals/820-init-no-storybook/EVAL.ts`:
- Around line 34-37: Annotate the `scripts` variable in the package.json
validation block with `Record<string, unknown>` so `scripts.storybook` is valid
under strict TypeScript, while preserving the existing record-check and
empty-object fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70afc782-9192-47a9-9e18-10a9cf0c5897
📒 Files selected for processing (14)
agent-eval/README.mdagent-eval/evals/820-init-no-storybook/EVAL.tsagent-eval/evals/823-setup-outdated-storybook/EVAL.tsagent-eval/evals/823-setup-outdated-storybook/src/components/Button.stories.tsxagent-eval/evals/824-setup-existing-storybook-with-stories/.storybook/main.tsagent-eval/evals/824-setup-existing-storybook-with-stories/.storybook/preview.tsagent-eval/evals/824-setup-existing-storybook-with-stories/EVAL.tsagent-eval/evals/824-setup-existing-storybook-with-stories/PROMPT.mdagent-eval/evals/824-setup-existing-storybook-with-stories/package.jsonagent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Button.stories.tsxagent-eval/lib/experiment.tsagent-eval/lib/shell-parse.tsagent-eval/lib/test-utils.test.tsagent-eval/lib/test-utils.ts
|
|
||
| const packageJson = parseJson(readFileSync('package.json', 'utf8')); | ||
| if (!isRecord(packageJson)) { | ||
| expect.fail('Expected package.json to contain a JSON object'); | ||
| } | ||
|
|
||
| const dependencies = { | ||
| ...(isRecord(packageJson.dependencies) ? packageJson.dependencies : {}), | ||
| ...(isRecord(packageJson.devDependencies) ? packageJson.devDependencies : {}), | ||
| }; | ||
| expect(dependencies.storybook, 'Expected a storybook dependency').toBeTypeOf('string'); | ||
| expect( | ||
| dependencies['@storybook/addon-mcp'], | ||
| 'Expected the @storybook/addon-mcp dependency (skill step 2: npx storybook add @storybook/addon-mcp)', | ||
| ).toBeTypeOf('string'); | ||
|
|
||
| const scripts = isRecord(packageJson.scripts) ? packageJson.scripts : {}; | ||
| const scripts = isRecord(packageJson) && isRecord(packageJson.scripts) ? packageJson.scripts : {}; | ||
| expect(scripts.storybook, 'Expected a storybook script').toBeTypeOf('string'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'agent-eval/evals/820-init-no-storybook/EVAL.ts'
echo '---'
cat -n agent-eval/evals/820-init-no-storybook/EVAL.ts | sed -n '1,120p'Repository: storybookjs/mcp
Length of output: 2586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
print("hello")
PYRepository: storybookjs/mcp
Length of output: 159
Annotate scripts so scripts.storybook typechecks. The fallback {} leaves scripts as Record<string, unknown> | {}, so the property access can fail under strict TypeScript. Add an explicit Record<string, unknown> annotation.
🛠️ Proposed fix
- const scripts = isRecord(packageJson) && isRecord(packageJson.scripts) ? packageJson.scripts : {};
+ const scripts: Record<string, unknown> = isRecord(packageJson) && isRecord(packageJson.scripts)
+ ? packageJson.scripts
+ : {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const packageJson = parseJson(readFileSync('package.json', 'utf8')); | |
| if (!isRecord(packageJson)) { | |
| expect.fail('Expected package.json to contain a JSON object'); | |
| } | |
| const dependencies = { | |
| ...(isRecord(packageJson.dependencies) ? packageJson.dependencies : {}), | |
| ...(isRecord(packageJson.devDependencies) ? packageJson.devDependencies : {}), | |
| }; | |
| expect(dependencies.storybook, 'Expected a storybook dependency').toBeTypeOf('string'); | |
| expect( | |
| dependencies['@storybook/addon-mcp'], | |
| 'Expected the @storybook/addon-mcp dependency (skill step 2: npx storybook add @storybook/addon-mcp)', | |
| ).toBeTypeOf('string'); | |
| const scripts = isRecord(packageJson.scripts) ? packageJson.scripts : {}; | |
| const scripts = isRecord(packageJson) && isRecord(packageJson.scripts) ? packageJson.scripts : {}; | |
| expect(scripts.storybook, 'Expected a storybook script').toBeTypeOf('string'); | |
| const packageJson = parseJson(readFileSync('package.json', 'utf8')); | |
| const scripts: Record<string, unknown> = isRecord(packageJson) && isRecord(packageJson.scripts) | |
| ? packageJson.scripts | |
| : {}; | |
| expect(scripts.storybook, 'Expected a storybook script').toBeTypeOf('string'); |
🤖 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 `@agent-eval/evals/820-init-no-storybook/EVAL.ts` around lines 34 - 37,
Annotate the `scripts` variable in the package.json validation block with
`Record<string, unknown>` so `scripts.storybook` is valid under strict
TypeScript, while preserving the existing record-check and empty-object fallback
behavior.
Source: Coding guidelines
Local run iteration 1 (cc-plugin-opus-high): 820 and 824 passed; 823 failed because Opus covered the then-storyless Tag component through the stories skill — a defensible reading of the shared prompt's 'preview the components in src/components', not a gate violation. Seeding stories for both components in 823 and 824 removes the ambiguity: any new story file is now a genuine violation. Iteration 2 re-run: both evals pass.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In
`@agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx`:
- Line 2: Update the local Tag imports in both Tag.stories.tsx files to include
the component’s explicit file extension, preserving the existing import and
story behavior.
- Around line 4-13: Rename the module-level meta constant to META in both
agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx
lines 4-13 and
agent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx
lines 4-13, updating the type Story reference in each file to use META while
preserving the story metadata.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e7762c2-f06b-4e9c-84cf-2d9197eeea41
📒 Files selected for processing (4)
agent-eval/evals/823-setup-outdated-storybook/EVAL.tsagent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsxagent-eval/evals/824-setup-existing-storybook-with-stories/EVAL.tsagent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- agent-eval/evals/823-setup-outdated-storybook/EVAL.ts
- agent-eval/evals/824-setup-existing-storybook-with-stories/EVAL.ts
| @@ -0,0 +1,27 @@ | |||
| import type { Meta, StoryObj } from '@storybook/react'; | |||
| import Tag from './Tag'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add explicit extensions to both local imports.
Both story files violate the ESM import-extension requirement.
agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx#L2-L2: importTagusing its explicit extension.agent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx#L2-L2: importTagusing its explicit extension.
📍 Affects 2 files
agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx#L2-L2(this comment)agent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx#L2-L2
🤖 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 `@agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx`
at line 2, Update the local Tag imports in both Tag.stories.tsx files to include
the component’s explicit file extension, preserving the existing import and
story behavior.
Source: Coding guidelines
| const meta = { | ||
| title: 'Components/Tag', | ||
| component: Tag, | ||
| args: { | ||
| label: 'New', | ||
| }, | ||
| } satisfies Meta<typeof Tag>; | ||
|
|
||
| export default meta; | ||
| type Story = StoryObj<typeof meta>; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename both module constants to SCREAMING_SNAKE_CASE.
Both files declare the module-level constant as meta, contrary to the naming guideline.
agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx#L4-L13: renamemetatoMETAand update its references.agent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx#L4-L13: renamemetatoMETAand update its references.
📍 Affects 2 files
agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx#L4-L13(this comment)agent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx#L4-L13
🤖 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 `@agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx`
around lines 4 - 13, Rename the module-level meta constant to META in both
agent-eval/evals/823-setup-outdated-storybook/src/components/Tag.stories.tsx
lines 4-13 and
agent-eval/evals/824-setup-existing-storybook-with-stories/src/components/Tag.stories.tsx
lines 4-13, updating the type Story reference in each file to use META while
preserving the story metadata.
Source: Coding guidelines
1 participant
Problem
"Set up Storybook for Codex" is the plugin's default prompt, and it currently always ends in
npx storybook ai setup— the full "configure preview, add MSW, write up to 10 stories" plan. On a project that already has a working Storybook with its own stories this is slow, expensive, and unnecessary — and that is the out-of-box experience for nearly all early plugin users.Change
The
setupskill (codex plugin) andstorybook-setupskill (claude plugin) become an explicit decision tree:initfirstupgradefirst@storybook/addon-mcpstorybook initboilerplate) → no story generation, nostorybook ai setup: route to the stories skill, which starts and shows Storybook. Explicit user requests for new stories still pass through.@storybook/addon-vitest(the printedai setupplan silently assumes itsvitest --project storybookverify loop works), then runnpx storybook ai setupas before.The gate applies after an upgrade too, so an outdated Storybook with existing stories no longer funnels into the full generation flow.
Evals
The lifecycle (82x) evals now cover the decision tree's story gate on both plugin experiments:
824-setup-existing-storybook-with-stories: current Storybook with user-written stories insrc/components, setup prompt → asserts nostorybook ai setup, story files and their exports stay exactly as seeded,@storybook/addon-mcpinstalled and registered, Storybook boots.823-setup-outdated-storybookstrengthened: same gate assertions after the existing upgrade-routing assertions. Its seeded story moved fromstories/tosrc/components/so it cannot be mistaken for the init example stories the skill tells agents to disregard.820-init-no-storybookstrengthened: now also asserts the positive branch — on a story-less fresh init the tree must ensure@storybook/addon-vitestand runstorybook ai setup.The ai-setup assertions are token-aware (
findStorybookAiSetupInvocationsinshell-parse.ts): help calls or commands that merely mention the text don't count as invocations, and a correct agent that greps for the command text doesn't false-fail the negative gate.Both story-gate fixtures seed a story for every component in
src/components: an early local run showed Opus (defensibly) covering a story-less component through the stories skill on the shared "preview the components insrc/components" prompt, so with full coverage seeded, any new story file is a genuine gate violation.Local
EVAL_ONLYvalidation, one experiment at a time: all three evals pass oncc-plugin-opus-highand oncodex-plugin-gpt-5.5-medium.Scope
PR 1 of 3 — works with currently released Storybook, no monorepo dependency. Follow-ups: a lightweight
storybook ai simple-setupcommand in storybookjs/storybook (one simple story + self-healing loop), then switching these skills' default path to it.Verification
pnpm vitest run --project=@storybook/claude-code-plugin --project=@storybook/codex-plugin— 9 tests pass (incl.claude plugin validate)pnpm --filter @storybook/codex-plugin validate:marketplace— passpnpm format:check— passpnpm --dir agent-eval typecheck+vitest run lib/(77 tests, incl. new detector and story-scan tests) — passEVAL_EXTRA_EVALS=1 pnpm --dir agent-eval eval:dry— 824 is picked up on both plugin experimentsSummary by CodeRabbit
Documentation
Tests