Conversation
📝 WalkthroughWalkthroughDependency bumps (Node, Playwright, Astro, Biome, Vue, dev tooling), small runtime/type fixes, CI workflow updates, and minor docs styling/config tweaks. No public API changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
🧹 Nitpick comments (1)
Dockerfile.test (1)
1-27: Consider adding a non-root user for defense-in-depth.Static analysis flags that the container runs as root. While this is a test container (lower risk), running as a non-root user is a security best practice that prevents potential container escape vulnerabilities.
🛡️ Optional: Add non-root user
FROM mcr.microsoft.com/playwright:v1.58.2-jammy +# Create non-root user for running tests +RUN groupadd --gid 1000 testuser && \ + useradd --uid 1000 --gid 1000 -m testuser + # Install dependencies for Bun RUN apt-get update && apt-get install -y unzip && rm -rf /var/lib/apt/lists/* # Install Bun -RUN curl -fsSL https://bun.sh/install | bash -ENV PATH="/root/.bun/bin:${PATH}" +RUN curl -fsSL https://bun.sh/install | bash -s -- --install-dir /usr/local/bun +ENV PATH="/usr/local/bun/bin:${PATH}" WORKDIR /app # Copy all package files for proper workspace resolution COPY package.json bun.lock ./ COPY packages/core/package.json ./packages/core/ COPY apps/docs/package.json ./apps/docs/ +COPY packages/cli/package.json ./packages/cli/ # Install dependencies (skip frozen for cross-platform compatibility) RUN bun install # Copy source COPY . . +RUN chown -R testuser:testuser /app # Build RUN bun run build +USER testuser + # Default command CMD ["bun", "run", "--cwd", "packages/core", "test:visual"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.test` around lines 1 - 27, Create and switch to a non-root user in the Dockerfile to avoid running tests as root: add steps to create a user/group (e.g., "testuser"), chown the /app workspace and Bun install directory (/root/.bun or move bun to a location the non-root user can access), set HOME and PATH for that user (so ENV PATH="/root/.bun/bin:${PATH}" is updated or Bun is installed into a user-writable location), then add USER testuser before the final CMD (which runs bun run --cwd packages/core test:visual) so the container executes tests as the non-root account; ensure file ownership and permission changes happen after COPY and bun install/build so the non-root user can read/write necessary files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/package.json`:
- Around line 14-17: Replace the deprecated astro-critters dependency with the
maintained `@critters-rs/astro` (bump to >=1.2.0) in apps/docs/package.json and
update any build references to the new package name so Critters runs with Astro
6; also update the Node engine requirement in the root package.json from "node":
">=20.0.0" to "node": ">=22.0.0" to satisfy Astro 6's Node 22+ requirement.
Ensure package.json dependencies no longer include "astro-critters" and that CI
/ Docker configs honor the new engines spec.
---
Nitpick comments:
In `@Dockerfile.test`:
- Around line 1-27: Create and switch to a non-root user in the Dockerfile to
avoid running tests as root: add steps to create a user/group (e.g.,
"testuser"), chown the /app workspace and Bun install directory (/root/.bun or
move bun to a location the non-root user can access), set HOME and PATH for that
user (so ENV PATH="/root/.bun/bin:${PATH}" is updated or Bun is installed into a
user-writable location), then add USER testuser before the final CMD (which runs
bun run --cwd packages/core test:visual) so the container executes tests as the
non-root account; ensure file ownership and permission changes happen after COPY
and bun install/build so the non-root user can read/write necessary files.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7156961-94f5-4e5c-9dfa-882e171f9725
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (8)
Dockerfile.testapps/docs/package.jsonbiome.jsonpackage.jsonpackages/cli/package.jsonpackages/cli/src/core/color.tspackages/core/package.jsonpackages/core/types/react.d.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/tests/e2e/accessibility.a11y.ts (1)
27-29: Type annotations are pragmatic and correct.The inline
{ id: string }type annotation on filter callbacks works correctly for the comparison. For slightly richer type information, you could import and use theResulttype fromaxe-core:import type { Result } from 'axe-core'; // ... (v: Result) => v.id === 'color-contrast'This would give you access to other
Resultproperties (likenodes,impact) with full type support if needed in future debugging scenarios. However, the current approach is perfectly functional.Also applies to: 48-50, 64-66, 91-93, 158-160, 218-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/e2e/accessibility.a11y.ts` around lines 27 - 29, Replace the inline callback type annotations used in the filter calls (e.g., the callback that produces contrastViolations from accessibilityScanResults.violations) with the axe-core Result type: add "import type { Result } from 'axe-core'" at the top, then change the callbacks to accept (v: Result) so future access to properties like nodes or impact is fully typed; apply this same change to the other filter occurrences mentioned (the callbacks at the equivalent filter sites around lines 48-50, 64-66, 91-93, 158-160, and 218-220).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/tests/e2e/accessibility.a11y.ts`:
- Around line 27-29: Replace the inline callback type annotations used in the
filter calls (e.g., the callback that produces contrastViolations from
accessibilityScanResults.violations) with the axe-core Result type: add "import
type { Result } from 'axe-core'" at the top, then change the callbacks to accept
(v: Result) so future access to properties like nodes or impact is fully typed;
apply this same change to the other filter occurrences mentioned (the callbacks
at the equivalent filter sites around lines 48-50, 64-66, 91-93, 158-160, and
218-220).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8701ac45-6325-4776-be2c-fcd7dc98b900
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (7)
apps/docs/package.jsonapps/docs/src/components/BundleSize.astroapps/docs/src/content.config.tsapps/docs/tests/accessibility.test.tspackages/core/package.jsonpackages/core/tests/e2e/accessibility.a11y.tspackages/core/tests/e2e/contrast.contrast.ts
✅ Files skipped from review due to trivial changes (5)
- apps/docs/tests/accessibility.test.ts
- apps/docs/src/content.config.ts
- packages/core/tests/e2e/contrast.contrast.ts
- apps/docs/package.json
- packages/core/package.json
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/cli/src/ui/prompts.ts (1)
168-170: Remove unreachable branch and normalize input once.
Line 169is unreachable becauseLine 168already returns for the same condition. Also, whitespace-only input should be treated as empty before extension checks.♻️ Suggested cleanup
validate: (value) => { - if (!value) return 'Value is required'; - if (!value) return 'Please enter a path'; - if (!value.endsWith('.css')) return 'Path must end with .css'; + const trimmed = value.trim(); + if (!trimmed) return 'Value is required'; + if (!trimmed.endsWith('.css')) return 'Path must end with .css'; return undefined; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/ui/prompts.ts` around lines 168 - 170, The validation contains a duplicate unreachable branch for the same condition and doesn't normalize whitespace; update the validator that checks the input `value` so you trim it once up front (e.g., normalize `value = value.trim()`), remove the redundant `if (!value) return 'Please enter a path'` branch, keep a single empty check that returns `'Value is required'`, and then perform the extension check using `value.endsWith('.css')` to return `'Path must end with .css'`.apps/docs/src/pages/showcase/article.astro (1)
419-420: Consider aligning with the existing pattern intheme.cssfor inline code styling.The global
color-scheme: light darkis properly configured, solight-dark()will work reliably across supported browsers (~85-90% as of 2026). However,apps/docs/src/styles/theme.cssstyles the samecode:not(pre code)selector withoutlight-dark(), using onlybackground: var(--void-100). For consistency, either:
- Keep the
light-dark()approach here and updatetheme.cssto match, or- Use the simpler semantic token fallback pattern like
theme.cssdoesThis is not a breaking issue given the proper
color-schemesetup, but consistency reduces cognitive load for maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/src/pages/showcase/article.astro` around lines 419 - 420, The inline code styling in apps/docs/src/pages/showcase/article.astro uses light-dark(...) for background and color while theme.css applies the same selector (code:not(pre code)) using a simpler semantic token (background: var(--void-100)); update article.astro to match the existing pattern by replacing the light-dark(...) usage with the semantic token(s) used in theme.css (e.g., background: var(--void-100) and the corresponding color token), or alternatively update theme.css’s code:not(pre code) rule to use light-dark(...) so both files follow the same approach—modify whichever file contains the differing rule (the code:not(pre code) styles in article.astro or theme.css) so both use the same token pattern for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/docs/src/pages/showcase/article.astro`:
- Around line 419-420: The inline code styling in
apps/docs/src/pages/showcase/article.astro uses light-dark(...) for background
and color while theme.css applies the same selector (code:not(pre code)) using a
simpler semantic token (background: var(--void-100)); update article.astro to
match the existing pattern by replacing the light-dark(...) usage with the
semantic token(s) used in theme.css (e.g., background: var(--void-100) and the
corresponding color token), or alternatively update theme.css’s code:not(pre
code) rule to use light-dark(...) so both files follow the same approach—modify
whichever file contains the differing rule (the code:not(pre code) styles in
article.astro or theme.css) so both use the same token pattern for consistency.
In `@packages/cli/src/ui/prompts.ts`:
- Around line 168-170: The validation contains a duplicate unreachable branch
for the same condition and doesn't normalize whitespace; update the validator
that checks the input `value` so you trim it once up front (e.g., normalize
`value = value.trim()`), remove the redundant `if (!value) return 'Please enter
a path'` branch, keep a single empty check that returns `'Value is required'`,
and then perform the extension check using `value.endsWith('.css')` to return
`'Path must end with .css'`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19fcf816-8096-46f9-8a90-e4b314e1793c
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (9)
.github/workflows/ci.yml.github/workflows/deploy-docs.ymlapps/docs/astro.config.mjsapps/docs/package.jsonapps/docs/src/pages/showcase/article.astropackage.jsonpackages/cli/package.jsonpackages/cli/src/ui/prompts.tspackages/core/src/components/prose.css
✅ Files skipped from review due to trivial changes (4)
- .github/workflows/deploy-docs.yml
- packages/cli/package.json
- packages/core/src/components/prose.css
- apps/docs/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Update all dependencies including major version bumps:
Major version updates:
astro: 5.18.1 → 6.1.1@astrojs/mdx: 4.2.6 → 5.0.3@types/node: 22.x → 25.5.0@clack/prompts: 0.7.0 → 1.1.0Minor/patch updates:
@biomejs/biome: 2.3.11 → 2.4.9@astrojs/check: 0.9.6 → 0.9.8@playwright/test: 1.50.1 → 1.58.2vue: 3.5.27 → 3.5.31Additional changes:
Dockerfile.testto use Playwright v1.58.2-jammybiome.jsonschema versionpackages/cli/src/core/color.tsreact.d.tsRelated Issues
Closes #50, closes #51, closes #52, closes #53, closes #54, closes #55, closes #56, closes #57, closes #59, closes #60
Checklist
bun run lintpassesbun run buildsucceedsType of Change
Additional Notes
All visual tests pass in Docker with the updated Playwright version. No snapshot regeneration was needed.