Conversation
📝 WalkthroughWalkthroughThis pull request introduces a modular CLI integration system that extends CLI functionality beyond built-in feature flags. It defines new types (Integration, IntegrationContext), adds an integrations registry with management utilities, implements Storybook as the first integration, and integrates the system into the main CLI flow with support for integration-specific next steps. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/index.ts (1)
621-631:⚠️ Potential issue | 🟠 MajorRun integrations before creating the initial git commit.
Lines 621-623 create the repo snapshot before Lines 625-631 apply Storybook. A default
bati --storybookwill therefore leave the freshly generated repo dirty, because.storybook/*and thepackage.jsonupdates happen after the initial commit.💡 Suggested change
- if (!args["skip-git"]) { - gitInit(args.project); - } - const enabledIntegrations = getEnabledIntegrations(args as unknown as Record<string, unknown>); const appliedIntegrations = await runEnabledIntegrations(enabledIntegrations, { project: args.project, flags, allFeatures: features, packageManagerExec: pm.exec, }); + + if (!args["skip-git"]) { + gitInit(args.project); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/index.ts` around lines 621 - 631, The initial git snapshot is created via gitInit before integrations run, which causes generated files (e.g., .storybook and package.json changes applied by getEnabledIntegrations/runEnabledIntegrations) to appear as uncommitted; move the integration execution to occur before calling gitInit so the repo includes integration changes in the initial commit, preserving the skip-git behavior by only calling gitInit(args.project) after awaiting runEnabledIntegrations when args["skip-git"] is falsy.
🧹 Nitpick comments (1)
packages/tests/src/load-test-files.ts (1)
38-38: Don’t special-case Storybook in the matrix allowlist.
packages/cli/integrations.tsis now the source of truth for CLI-only flags. Hardcoding"storybook"here means the next integration will need a second manual allowlist update, and the test matrix can drift from the real CLI surface. Prefer sourcing integration keys from the registry or from a shared helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tests/src/load-test-files.ts` at line 38, Remove the hardcoded "storybook" special-case in the validKeys Set in load-test-files.ts and instead derive CLI-only integration keys from the canonical source (packages/cli/integrations.ts or the shared helper/registry) so the test matrix stays in sync; replace the current new Set<unknown>([...flags, "storybook"]) usage by merging flags with the exported integration keys (import the relevant symbol from integrations.ts) and construct the Set from that combined list (referencing the validKeys and flags identifiers to locate where to change it).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/storybook.ts`:
- Around line 20-37: Add an explicit interactive flag to the integration context
and thread it into initStorybook so Storybook is non-interactive in CI/test
runs: extend IntegrationContext with an interactive boolean derived from
BATI_IS_CI || BATI_TEST in index.ts, ensure integration.run(...) receives that
context, and update the callsite that invokes initStorybook (the call to
initStorybook(...) around the integration.run path) to pass the interactive
value instead of relying on the default; this ensures initStorybook(interactive)
will run with interactive=false for unattended CI/test environments.
In `@packages/tests/tests/FRAMEWORK`+storybook.spec.ts:
- Around line 6-39: Replace the use of child_process.execSync("pwd", ...) with
the cross-platform Node API process.cwd() in both tests: use process.cwd() for
storybookDir in the "storybook config files" test (where storybookDir is set)
and for cwd in the "storybook scripts" test (where packageJsonPath is built);
update references to storybookDir and cwd accordingly so the tests no longer
shell out and work on Windows.
---
Outside diff comments:
In `@packages/cli/index.ts`:
- Around line 621-631: The initial git snapshot is created via gitInit before
integrations run, which causes generated files (e.g., .storybook and
package.json changes applied by getEnabledIntegrations/runEnabledIntegrations)
to appear as uncommitted; move the integration execution to occur before calling
gitInit so the repo includes integration changes in the initial commit,
preserving the skip-git behavior by only calling gitInit(args.project) after
awaiting runEnabledIntegrations when args["skip-git"] is falsy.
---
Nitpick comments:
In `@packages/tests/src/load-test-files.ts`:
- Line 38: Remove the hardcoded "storybook" special-case in the validKeys Set in
load-test-files.ts and instead derive CLI-only integration keys from the
canonical source (packages/cli/integrations.ts or the shared helper/registry) so
the test matrix stays in sync; replace the current new Set<unknown>([...flags,
"storybook"]) usage by merging flags with the exported integration keys (import
the relevant symbol from integrations.ts) and construct the Set from that
combined list (referencing the validKeys and flags identifiers to locate where
to change it).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: abc5d2e6-4c1a-4aea-ba87-cc877b2d68ae
📒 Files selected for processing (8)
AGENTS.mdpackages/cli/index.tspackages/cli/integrations.tspackages/cli/storybook.tspackages/cli/types.tspackages/features/README.mdpackages/tests/src/load-test-files.tspackages/tests/tests/FRAMEWORK+storybook.spec.ts
| export async function initStorybook( | ||
| cwd: string, | ||
| packageManagerExec: string, | ||
| interactive: boolean = true, | ||
| ): Promise<boolean> { | ||
| let shouldUseDefaultConfig = !interactive; | ||
|
|
||
| // Prompt user if they want to initialize Storybook | ||
| if (interactive) { | ||
| shouldUseDefaultConfig = await confirm({ | ||
| message: "Use default Storybook configuration?", | ||
| default: true, | ||
| }); | ||
| } | ||
|
|
||
| // Run Storybook init with interactive questionnaire | ||
| const command = `${packageManagerExec} storybook@latest init --no-dev${shouldUseDefaultConfig ? " --yes" : ""}`; | ||
| execSync(command, { cwd, stdio: "inherit" }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== initStorybook call sites ==\n'
rg -n 'initStorybook\(' packages/cli
printf '\n== prompt / CI / TTY plumbing in packages/cli ==\n'
rg -n 'confirm\(|interactive|BATI_TEST|process\.env\.CI|isTTY' packages/cliRepository: vikejs/bati
Length of output: 782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== IntegrationContext definition ==\n'
rg -B3 -A10 'interface IntegrationContext|type IntegrationContext' packages/cli
printf '\n== storybookIntegration.run() definition ==\n'
rg -B5 -A15 'storybookIntegration.*=|const storybookIntegration' packages/cli
printf '\n== How integrations are called in index.ts ==\n'
rg -B3 -A3 'storybookIntegration\.run\(|integration\.run\(' packages/cli/index.ts
printf '\n== Other integration implementations for interactive handling ==\n'
fd '\.(ts|tsx)$' packages/cli/integrations --exec rg -l 'interactive|BATI_TEST|confirm' {} \;Repository: vikejs/bati
Length of output: 2090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Integration.run() call site in index.ts (context around line 577) ==\n'
sed -n '540,620p' packages/cli/index.ts
printf '\n== Check how IntegrationContext is constructed ==\n'
rg -B10 -A5 'as IntegrationContext|IntegrationContext.*=' packages/cli/index.ts | head -60
printf '\n== Another integration implementation (e.g., tailwind) ==\n'
ls -la packages/cli/integrations/ 2>/dev/null || ls -la packages/cli/*.ts | grep -E '(tailwind|prettier|vitest)' || trueRepository: vikejs/bati
Length of output: 2526
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Direct search for integration.run invocation ==\n'
rg -n -B5 -A5 'integration\.run\(' packages/cli/index.ts
printf '\n== Search for all Integration usage in index.ts ==\n'
rg -n 'integrations|Integration' packages/cli/index.ts
printf '\n== Full storybook.ts file to see the full picture ==\n'
wc -l packages/cli/storybook.tsRepository: vikejs/bati
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Full storybook.ts content ==\n'
cat -n packages/cli/storybook.ts
printf '\n== Search for storybookIntegration usage across codebase ==\n'
rg -n 'storybookIntegration' --type ts packages/cliRepository: vikejs/bati
Length of output: 3008
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== integrations.ts full content ==\n'
cat -n packages/cli/integrations.ts
printf '\n== Search for integrations array usage ==\n'
rg -n 'integrations\[|for.*integrations|integrations\..*run\|integrations\.filter' packages/cli
printf '\n== Search for where integration features are added to boilerplates ==\n'
rg -n 'storybookIntegration\|integrations' packages/cli/index.ts | head -20Repository: vikejs/bati
Length of output: 1488
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search for runEnabledIntegrations invocation ==\n'
rg -n -B10 -A10 'runEnabledIntegrations' packages/cli/index.ts
printf '\n== Check types.ts Integration interface fully ==\n'
rg -B5 -A15 'export interface Integration' packages/cli/types.tsRepository: vikejs/bati
Length of output: 2789
Make --storybook explicitly non-interactive when the CLI is unattended.
Line 23 defaults interactive to true, and the only callsite on line 57 relies on that default without passing an explicit value. The IntegrationContext passed to integration.run() at index.ts:626 does not include CI/test mode information, even though BATI_IS_CI and BATI_TEST are already available in meta at that location. This prevents integrations from disabling prompts in CI, test, or scripted runs.
Add an interactive field to IntegrationContext (derived from BATI_IS_CI and BATI_TEST in index.ts) and pass it to initStorybook() at line 57.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/storybook.ts` around lines 20 - 37, Add an explicit interactive
flag to the integration context and thread it into initStorybook so Storybook is
non-interactive in CI/test runs: extend IntegrationContext with an interactive
boolean derived from BATI_IS_CI || BATI_TEST in index.ts, ensure
integration.run(...) receives that context, and update the callsite that invokes
initStorybook (the call to initStorybook(...) around the integration.run path)
to pass the interactive value instead of relying on the default; this ensures
initStorybook(interactive) will run with interactive=false for unattended
CI/test environments.
There was a problem hiding this comment.
We will indeed need a non-interactive mode for the CI, with sensible default values
There was a problem hiding this comment.
@magne4000 Agreed! Here are the three changes needed to wire up a proper non-interactive mode:
1. packages/cli/types.ts — extend IntegrationContext
export interface IntegrationContext {
project: string;
flags: string[];
allFeatures: ReadonlyArray<Feature>;
packageManagerExec: string;
+ interactive: boolean;
}2. packages/cli/index.ts — derive interactive from BATI_IS_CI / BATI_TEST when building the context
-const context: IntegrationContext = {
+const context: IntegrationContext = {
project,
flags,
allFeatures: features,
packageManagerExec,
+ interactive: !meta.BATI_IS_CI && !meta.BATI_TEST,
};3. packages/cli/storybook.ts — pass context.interactive to initStorybook
- const result = await initStorybook(context.project, context.packageManagerExec);
+ const result = await initStorybook(context.project, context.packageManagerExec, context.interactive);This way:
- Interactive terminal (normal developer flow): prompts are shown, defaulting to
--yesif the user accepts. - CI / test runs (
BATI_IS_CIorBATI_TEST):interactive = false, soshouldUseDefaultConfig = trueand--yesis always passed, skipping all prompts with sensible defaults.
🧠 Learnings used
Learnt from: CR
Repo: vikejs/bati PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T09:49:22.825Z
Learning: Applies to boilerplates/*/bati.config.ts : Create UI-framework-specific boilerplate variants (e.g., react-sentry, vue-sentry) when features require framework-specific code, using combined conditions in bati.config.ts
| test("storybook config files", async () => { | ||
| const fs = await import("fs/promises"); | ||
| const path = await import("path"); | ||
|
|
||
| const storybookDir = (await import("child_process")).execSync("pwd", { encoding: "utf-8" }).trim(); | ||
|
|
||
| const configExtensions = ["ts", "js", "mjs", "cjs"]; | ||
| let configFileExists = false; | ||
|
|
||
| for (const ext of configExtensions) { | ||
| const configPath = path.join(storybookDir, ".storybook", `main.${ext}`); | ||
| try { | ||
| await fs.access(configPath); | ||
| configFileExists = true; | ||
| break; | ||
| } catch { | ||
| // Continue to next extension | ||
| } | ||
| } | ||
|
|
||
| expect(configFileExists).toBe(true); | ||
| }); | ||
|
|
||
| test("storybook scripts", async () => { | ||
| const fs = await import("fs/promises"); | ||
| const path = await import("path"); | ||
|
|
||
| const cwd = (await import("child_process")).execSync("pwd", { encoding: "utf-8" }).trim(); | ||
|
|
||
| const packageJsonPath = path.join(cwd, "package.json"); | ||
| const packageJson = JSON.parse(await fs.readFile(packageJsonPath, "utf-8")); | ||
|
|
||
| expect(packageJson.scripts.storybook).toBeTruthy(); | ||
| expect(packageJson.scripts["build-storybook"]).toBeTruthy(); |
There was a problem hiding this comment.
Use process.cwd() instead of shelling out to pwd.
Line 10 and Line 33 invoke pwd, which isn’t portable to Windows shells. That will break this new E2E spec as soon as the matrix runs on Windows; process.cwd() is cheaper and cross-platform.
💡 Suggested change
test("storybook config files", async () => {
const fs = await import("fs/promises");
const path = await import("path");
- const storybookDir = (await import("child_process")).execSync("pwd", { encoding: "utf-8" }).trim();
+ const storybookDir = process.cwd();
@@
test("storybook scripts", async () => {
const fs = await import("fs/promises");
const path = await import("path");
- const cwd = (await import("child_process")).execSync("pwd", { encoding: "utf-8" }).trim();
+ const cwd = process.cwd();📝 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.
| test("storybook config files", async () => { | |
| const fs = await import("fs/promises"); | |
| const path = await import("path"); | |
| const storybookDir = (await import("child_process")).execSync("pwd", { encoding: "utf-8" }).trim(); | |
| const configExtensions = ["ts", "js", "mjs", "cjs"]; | |
| let configFileExists = false; | |
| for (const ext of configExtensions) { | |
| const configPath = path.join(storybookDir, ".storybook", `main.${ext}`); | |
| try { | |
| await fs.access(configPath); | |
| configFileExists = true; | |
| break; | |
| } catch { | |
| // Continue to next extension | |
| } | |
| } | |
| expect(configFileExists).toBe(true); | |
| }); | |
| test("storybook scripts", async () => { | |
| const fs = await import("fs/promises"); | |
| const path = await import("path"); | |
| const cwd = (await import("child_process")).execSync("pwd", { encoding: "utf-8" }).trim(); | |
| const packageJsonPath = path.join(cwd, "package.json"); | |
| const packageJson = JSON.parse(await fs.readFile(packageJsonPath, "utf-8")); | |
| expect(packageJson.scripts.storybook).toBeTruthy(); | |
| expect(packageJson.scripts["build-storybook"]).toBeTruthy(); | |
| test("storybook config files", async () => { | |
| const fs = await import("fs/promises"); | |
| const path = await import("path"); | |
| const storybookDir = process.cwd(); | |
| const configExtensions = ["ts", "js", "mjs", "cjs"]; | |
| let configFileExists = false; | |
| for (const ext of configExtensions) { | |
| const configPath = path.join(storybookDir, ".storybook", `main.${ext}`); | |
| try { | |
| await fs.access(configPath); | |
| configFileExists = true; | |
| break; | |
| } catch { | |
| // Continue to next extension | |
| } | |
| } | |
| expect(configFileExists).toBe(true); | |
| }); | |
| test("storybook scripts", async () => { | |
| const fs = await import("fs/promises"); | |
| const path = await import("path"); | |
| const cwd = process.cwd(); | |
| const packageJsonPath = path.join(cwd, "package.json"); | |
| const packageJson = JSON.parse(await fs.readFile(packageJsonPath, "utf-8")); | |
| expect(packageJson.scripts.storybook).toBeTruthy(); | |
| expect(packageJson.scripts["build-storybook"]).toBeTruthy(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/tests/tests/FRAMEWORK`+storybook.spec.ts around lines 6 - 39,
Replace the use of child_process.execSync("pwd", ...) with the cross-platform
Node API process.cwd() in both tests: use process.cwd() for storybookDir in the
"storybook config files" test (where storybookDir is set) and for cwd in the
"storybook scripts" test (where packageJsonPath is built); update references to
storybookDir and cwd accordingly so the tests no longer shell out and work on
Windows.
There was a problem hiding this comment.
We should reuse (and expand if necessary) the hooks/after.ts pattern for adding those kind of integrations.
Also, features.ts must be used for all integrations, as it is normalized and used by both the CLI, the Web App and the Web Component.
We probably need to create a new sub category (lets call it "Tooling") under "Utilities".
| for (const onafter of hooksMap.get("after") ?? []) { | ||
| await onafter(args.project, meta); | ||
| } |
There was a problem hiding this comment.
We should now sort after hooks in reverse enforce order (See
Line 544 in 08bfa12
This will ensure that the
shared after hook is executed last.
|
Replaced by #690 |
2 participants
This PR introduces support for Storybook via a new
--storybookflag.As Storybook is the first integration that brings in a CLI execution (the other integrations being boilerplates), I wasn't sure what was the recommended way to add this as integration, so I created the concept of "integrations" which will contain a flag, label, a run function (which would then trigger some validations and the actual CLI) and a nextSteps function to calculate what to show in the next steps section.
It looks like the following:
Note: This is a conversation starter, we can discuss and change the code to whatever works best for Bati. I'd be happy if we could provide Storybook as an option for folks to use with Vike!
Summary by CodeRabbit
New Features
--storybookCLI option for React, Vue, and Solid projects.Tests
Documentation