Repository navigation
feat(cli): finalize zero-friction beta setup - #18
aetosdios27 merged 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthrough
ChangesCLI workflow and Studio runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Inspector
participant PackageManager
participant Project
User->>CLI: run scribe integrate
CLI->>Inspector: inspect project
Inspector-->>CLI: return stack and style plan
CLI->>PackageManager: install missing packages
CLI->>Project: apply stylesheet and MDX changes
CLI-->>User: report integration result
sequenceDiagram
participant User
participant Studio
participant Server
participant Browser
User->>Studio: start with optional port
Studio->>Server: bind preferred or available port
Server-->>Studio: return serving origin
Studio->>Browser: open preview URL
Browser-->>User: display Studio preview
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
packages/cli/README.md (1)
59-62: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake every npm example fail closed or pin the CLI.
All three README copies use unpinned
npx scribecommands. If the local CLI is unavailable, npx may fetch a remote, unscoped package instead of the reviewed@scribe-sdk/cli@0.1.0-alpha.7. Apply one consistent policy to all examples: use a no-install form for local workflows or pin the scoped package and exact version for remote execution. (docs.npmjs.com)
packages/cli/README.md#L59-L62: update the npm dry-run command.packages/cli/README.md#L139-L140: update the npm integration commands.packages/mdx/README.md#L59-L62: apply the same npm dry-run change.packages/mdx/README.md#L139-L140: apply the same npm integration change.packages/react/README.md#L59-L62: apply the same npm dry-run change.packages/react/README.md#L139-L140: apply the same npm integration change.🤖 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/cli/README.md` around lines 59 - 62, Replace every unpinned npm `npx scribe` example with one consistent safe form: use no-install execution for local workflows or pin `@scribe-sdk/cli@0.1.0-alpha.7` for remote execution. Apply the same policy to the dry-run and integration examples in packages/cli/README.md (lines 59-62 and 139-140), packages/mdx/README.md (lines 59-62 and 139-140), and packages/react/README.md (lines 59-62 and 139-140).Sources: MCP tools, Linters/SAST tools
packages/cli/src/integrate.ts (2)
420-425: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider unlinking the temp file when the write or rename fails.
A failed
writeFile/renameleaves.<name>.scribe-*.tmpbeside the user's source file.🤖 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/cli/src/integrate.ts` around lines 420 - 425, Update atomicWrite to clean up the temporary file when writeFile or rename fails. Ensure the generated path in temporary is unlinked during error handling, while preserving the existing successful atomic rename behavior and propagating the original failure.
88-93: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUnbounded parallel reads and a loose Tailwind version match.
Two small robustness points in
inspectProject:
- Line 89 opens up to 1,000 files concurrently; a small concurrency limit (or sequential read) avoids
EMFILEon restrictive systems and costs nothing here since this runs once per command.- Line 93's regex matches any
3/4preceded by a non-digit, so a future10.3.xrange would be reported as Tailwind 3. Matching only the leading major (e.g. after stripping range prefixes) is more precise.🤖 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/cli/src/integrate.ts` around lines 88 - 93, Update inspectProject around the entries construction and Tailwind version parsing: limit readFile concurrency when mapping over files to avoid opening all files simultaneously, and derive tailwindMajor from the leading major version after removing supported range prefixes so versions such as 10.3.x are not classified as Tailwind 3.packages/cli/src/index.test.ts (1)
39-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSource-text regexes will break on harmless reformatting.
Lines 50-52 encode exact brace/newline layout of
main(). ThetoContain('await import("./init.js")')assertions already prove the lazy boundary; a cheaper and more durable guard for "no static import of heavy modules" would be asserting on the module graph (e.g. that importing./index.jsdoes not pull./studio.js) rather than pattern-matching formatting.🤖 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/cli/src/index.test.ts` around lines 39 - 53, Replace the formatting-sensitive regex assertions in the `keeps heavyweight commands behind dynamic import boundaries` test with module-graph checks verifying that importing `./index.js` does not statically load `./studio.js`, `./init.js`, `./integrate.js`, or `@scribe-sdk/mdx`. Retain the dynamic-import assertions as the direct lazy-boundary validation, and remove the exact `main()` layout regexes.packages/cli/src/init.test.ts (1)
77-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertion depends on POSIX separators.
existingAbsoluteDirectoriesthrows with the raw absolute path (packages/cli/src/init.tsline 210), so thistoContain("content/blog exists…")only matches on POSIX. Formatting that message withdisplayPath(root, path)would make both the error and this assertion platform-independent.🤖 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/cli/src/init.test.ts` around lines 77 - 85, Update existingAbsoluteDirectories to format the conflicting path with displayPath(root, path) before constructing its error message, so the “exists but is not a directory” output uses platform-independent display paths and the test assertion remains portable.
🤖 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/cli/src/init.test.ts`:
- Around line 62-64: Sort each fs.readdir result before comparing it in the
initialization test, including the top-level content directory and its
assets/blog subdirectories, while preserving the expected entries and
empty-directory assertions.
In `@packages/cli/src/integrate.ts`:
- Around line 273-278: Update the change-application loop in the try/catch
around atomicWrite to track each successfully written change path, then include
that list in the catch error reported by stderr when a later write fails.
Preserve the existing return code and error-detail formatting while clearly
identifying the files already written.
In `@packages/cli/src/studio.ts`:
- Line 970: Update studioRuntimePaths() to define the sonnerStyle entry via
studioImportPath("sonner/dist/styles.css"), then change the corresponding Vite
alias to reference the sonnerStyle variable instead of an undefined alias.
Preserve the existing alias key and other runtime path mappings.
In `@scripts/test-portable-cli.mjs`:
- Around line 57-69: Strengthen the portable CLI test around the init and
integrate flows by snapshotting the relevant fixture tree before each operation
and comparing it afterward, rather than checking only globalStyle. Verify init
--dry-run and integrate --dry-run leave the entire fixture unchanged, and
confirm init --yes leaves host files unchanged while still producing an empty
content/blog launchpad.
In `@tests/release/changesets.test.ts`:
- Line 36: Update the changeset fragment assertion in the changesets test to
require exact set equality with pre.changesets rather than allowing additional
files. Also update scripts/check-release-alignment.mjs to reject unexpected
prerelease fragments, or explicitly constrain permitted extras to a defined
allowlist.
In `@tests/studio-browser/studio.spec.ts`:
- Around line 125-126: Make the reveal assertion in the synchronizedCode test
deterministic instead of relying on the 1.1-second data-scribe-reveal-active
window. Trigger the reveal immediately before asserting, or assert a
monotonically increasing reveal counter attribute if available, while preserving
the subsequent check that the active attribute is eventually cleared.
---
Nitpick comments:
In `@packages/cli/README.md`:
- Around line 59-62: Replace every unpinned npm `npx scribe` example with one
consistent safe form: use no-install execution for local workflows or pin
`@scribe-sdk/cli@0.1.0-alpha.7` for remote execution. Apply the same policy to
the dry-run and integration examples in packages/cli/README.md (lines 59-62 and
139-140), packages/mdx/README.md (lines 59-62 and 139-140), and
packages/react/README.md (lines 59-62 and 139-140).
In `@packages/cli/src/index.test.ts`:
- Around line 39-53: Replace the formatting-sensitive regex assertions in the
`keeps heavyweight commands behind dynamic import boundaries` test with
module-graph checks verifying that importing `./index.js` does not statically
load `./studio.js`, `./init.js`, `./integrate.js`, or `@scribe-sdk/mdx`. Retain
the dynamic-import assertions as the direct lazy-boundary validation, and remove
the exact `main()` layout regexes.
In `@packages/cli/src/init.test.ts`:
- Around line 77-85: Update existingAbsoluteDirectories to format the
conflicting path with displayPath(root, path) before constructing its error
message, so the “exists but is not a directory” output uses platform-independent
display paths and the test assertion remains portable.
In `@packages/cli/src/integrate.ts`:
- Around line 420-425: Update atomicWrite to clean up the temporary file when
writeFile or rename fails. Ensure the generated path in temporary is unlinked
during error handling, while preserving the existing successful atomic rename
behavior and propagating the original failure.
- Around line 88-93: Update inspectProject around the entries construction and
Tailwind version parsing: limit readFile concurrency when mapping over files to
avoid opening all files simultaneously, and derive tailwindMajor from the
leading major version after removing supported range prefixes so versions such
as 10.3.x are not classified as Tailwind 3.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d54aa21c-aff9-4599-b90f-cc72b470b0a9
📒 Files selected for processing (36)
.changeset/empty-writers-integrate.md.changeset/friendly-studios-follow.md.changeset/pre.jsonREADME.mdSKILL.mdpackages/cli/CHANGELOG.mdpackages/cli/README.mdpackages/cli/SKILL.mdpackages/cli/package.jsonpackages/cli/src/command-help.tspackages/cli/src/index.test.tspackages/cli/src/index.tspackages/cli/src/init.test.tspackages/cli/src/init.tspackages/cli/src/integrate.test.tspackages/cli/src/integrate.tspackages/cli/src/studio-ui.test.tspackages/cli/src/studio-ui.tspackages/cli/src/studio.test.tspackages/cli/src/studio.tspackages/mdx/CHANGELOG.mdpackages/mdx/README.mdpackages/mdx/SKILL.mdpackages/mdx/package.jsonpackages/react/CHANGELOG.mdpackages/react/README.mdpackages/react/SKILL.mdpackages/react/package.jsonpackages/styles/CHANGELOG.mdpackages/styles/README.mdpackages/styles/SKILL.mdpackages/styles/package.jsonscripts/test-portable-cli.mjstests/release/changesets.test.tstests/release/documentation.test.tstests/studio-browser/studio.spec.ts
Linear
Fixes AET-75
Summary
scribe initand existing-site integration intoscribe integrate0.1.0-alpha.7with synchronized documentation and changelogsVerification
Additional release gates passed locally:
next-mdx-remote/rscVisual evidence
The Studio changes preserve the approved shell and add a brief cobalt target pulse when the preview follows sustained Markdown edits. No visual baseline was regenerated.
Summary by CodeRabbit
New Features
scribe integratefor project inspection, package setup, MDX wiring, stylesheet selection, and safe integration plans.scribe initto create an empty content launchpad only.Bug Fixes
Documentation