Repository navigation
CLI: Prefer agent-matched Storybook instances - #35235
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds agent provenance tracking to Storybook's MCP runtime instance records. A new ChangesAgent-aware MCP instance resolution
Sequence Diagram(s)sequenceDiagram
participant AI_Agent as AI Agent (e.g. Claude CLI)
participant runAiTool
participant resolveInstance
participant selectCompetingBucket
participant StorybookInstance
AI_Agent->>runAiTool: invoke MCP tool
runAiTool->>runAiTool: detectAgent() → {name: 'claude'}
runAiTool->>resolveInstance: records, cwd, port?, currentAgent='claude'
resolveInstance->>selectCompetingBucket: matches, targetPort, 'claude'
selectCompetingBucket->>selectCompetingBucket: filter claude-preview bucket
alt claude-preview records found
selectCompetingBucket-->>resolveInstance: claude-preview bucket sorted
else no claude-preview, fallback
selectCompetingBucket-->>resolveInstance: claude bucket sorted
end
resolveInstance-->>runAiTool: {instance, matches, interceptReason?}
runAiTool->>StorybookInstance: call MCP on resolved instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/core/src/cli/ai/mcp/run-tool.test.ts (1)
503-503: ⚡ Quick winMove this scenario-specific mock behavior into a scoped
beforeEach.
vi.mocked(readRegistry).mockResolvedValue(...)is configured inline inside the test. To keep this file aligned with the Vitest mocking rules, set this behavior in a nesteddescribebeforeEachfor this scenario.As per coding guidelines, "Implement mock behaviors in
beforeEachblocks in Vitest tests" and "Avoid inline mock implementations within test cases 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/cli/ai/mcp/run-tool.test.ts` at line 503, The vi.mocked(readRegistry).mockResolvedValue() call is currently configured inline within the test case. Move this mock setup out of the test body and into a scoped beforeEach block. Create or use an existing nested describe block for this test scenario, then establish a beforeEach block within that describe where you set up the vi.mocked(readRegistry).mockResolvedValue() with the test data (olderPreview, selectedPreview, newerCodex), then remove the inline mock setup from the test case itself.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/cli/ai/mcp/run-tool.test.ts`:
- Line 503: The vi.mocked(readRegistry).mockResolvedValue() call is currently
configured inline within the test case. Move this mock setup out of the test
body and into a scoped beforeEach block. Create or use an existing nested
describe block for this test scenario, then establish a beforeEach block within
that describe where you set up the vi.mocked(readRegistry).mockResolvedValue()
with the test data (olderPreview, selectedPreview, newerCodex), then remove the
inline mock setup from the test case itself.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2aa317a-2c59-4012-bb16-84e33216b673
📒 Files selected for processing (9)
code/core/src/cli/ai/mcp/registry.test.tscode/core/src/cli/ai/mcp/resolve-instance.test.tscode/core/src/cli/ai/mcp/resolve-instance.tscode/core/src/cli/ai/mcp/run-tool.test.tscode/core/src/cli/ai/mcp/run-tool.tscode/core/src/cli/ai/mcp/types.tscode/core/src/core-server/utils/runtime-instance-registry.test.tscode/core/src/core-server/utils/runtime-instance-registry.tscode/core/src/shared/constants/agent-provenance.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
LogDetails |
Closes #35234
What I did
Adds optional
agentprovenance to Storybook runtime instance records and uses it whenstorybook airesolves a running Storybook instance without an explicit--port.agent: 'claude-preview'whenCLAUDE_AGENT_SDK_VERSIONis set andAI_AGENTis not set.detectAgent()?.name.claude-previewrecords for Claude CLI calls, then genericclauderecords; other agents prefer their matching records.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
No browser story QA is necessary for this PR; the behavior is CLI runtime instance selection. To manually verify the intended surface, run:
yarn vitest run code/core/src/cli/ai/mcp/resolve-instance.test.ts code/core/src/cli/ai/mcp/run-tool.test.ts code/core/src/cli/ai/mcp/registry.test.ts code/core/src/core-server/utils/runtime-instance-registry.test.tsonly warns about instances in the selected agent bucketpasses. That test simulates a Claude CLI call with twoclaude-previewrecords and a newercodexrecord at the same cwd; the expected behavior is that the command routes to the latest Claude preview record and the warning excludes the Codex record.Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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 pull request has been released as version
0.0.0-pr-35235-sha-7c9fa4b5. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-35235-sha-7c9fa4b5 sandboxor in an existing project withnpx storybook@0.0.0-pr-35235-sha-7c9fa4b5 upgrade.More information
0.0.0-pr-35235-sha-7c9fa4b5kasper/ai-agent-instance-selection7c9fa4b51781879849)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=35235Summary by CodeRabbit
New Features
Bug Fixes
Tests