Skip to content

feat: update#14

Merged
AndrewHanasiro merged 6 commits intomainfrom
feature/update
Dec 11, 2025
Merged

feat: update#14
AndrewHanasiro merged 6 commits intomainfrom
feature/update

Conversation

@AndrewHanasiro
Copy link
Member

@AndrewHanasiro AndrewHanasiro commented Dec 11, 2025

Proposal

  • update node version
  • update packages

Summary by CodeRabbit

  • New Features

    • SonarCloud scanning added to CI
  • Tests

    • End-to-end tests added; test scripts split into unit and e2e; Vitest and Playwright configs and test setup updated
  • Style

    • New global stylesheet; favicon switched to SVG; Prettier ignore list reorganized
  • Chores

    • Node runtime bumped to v24; Docker base image updated; CI workflows upgraded; dev tooling overhauled; .gitignore expanded; ESLint configuration replaced/removed; LICENSE file removed

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Walkthrough

Deletes legacy .eslintrc.cjs, adds a defineConfig-based ESLint config, upgrades CI workflows (adds SonarCloud), updates Node/Docker base images, overhauls devDependencies and test configs (Vitest/Playwright), adds global CSS and an E2E test, modifies miscellaneous project files, and empties LICENSE.

Changes

Cohort / File(s) Summary
ESLint & Linting
\.eslintrc.cjs, eslint.config.js
Removed legacy FlatConfig file \.eslintrc.cjs. Added eslint.config.js exporting defineConfig(...) with gitignore integration, JS/TS/Svelte presets, languageOptions and per-file Svelte parser overrides.
CI / Scanning
.github/workflows/continuous_integration.yml, .github/workflows/manual_deploy.yml
Upgraded actions (actions/checkout, actions/setup-node) to v6, replaced set-output with GITHUB_OUTPUT, and added SonarCloud scan step in CI.
Ignore & Node version
\.gitignore, \.prettierignore, \.nvmrc
Added test-results, .netlify, .wrangler to .gitignore; restructured .prettierignore entries (package manager/build artifacts, added /static/); bumped Node in \.nvmrc 22.12→24.10.
Container
Dockerfile
Updated base images to node:24.11.1-trixie-slim; added EXPOSE 3000 and ENTRYPOINT ["node","./build/index.js"].
Package Metadata & DevDeps
package.json
Added "private": true and "type": "module"; reworked scripts (added prepare, test:unit, test:e2e, updated test), and substantially updated devDependencies (ESLint/Prettier/Svelte/Vitest/Playwright/TS tooling).
Testing / Test Setup
playwright.config.ts, vite.config.ts, vitest-setup-client.ts, e2e/demo.test.ts
Simplified Playwright config (inline defineConfig, testDire2e, added JUnit reporter); Vitest changed to multi-project (client/browser + server/node) with Playwright integration; added browser matcher/provider refs and an E2E Playwright test.
Svelte / Tooling Config
svelte.config.js, tsconfig.json
Set preprocess: vitePreprocess() and kit.adapter() directly; removed "target": "es2021" from tsconfig.json and updated inline docs.
App Assets & Styles
src/app.css, src/app.html, src/app.d.ts, static/robots.txt
Added global stylesheet src/app.css; switched favicon to favicon.svg; tightened viewport meta; minor doc update in app.d.ts; adjusted robots.txt comment.
Sonar / Analysis
sonar-project.properties
Added SonarQube configuration (project key/org, sources, junit report path, encoding).
Licensing
LICENSE
Cleared LICENSE file (content removed).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • eslint.config.js — large, intricate configuration and per-file parser settings.
    • package.json — many devDependency upgrades and script changes affecting local/CI flows.
    • vite.config.ts & Vitest multi-project setup — include/exclude patterns and Playwright wiring.
    • Dockerfile — confirm runtime path (./build/index.js) and exposed port align with build output.
    • LICENSE — verify intentional removal.

Possibly related PRs

Poem

🐰 I hopped through configs, tidy and spry,
ESLint found a nook to peacefully lie,
Node stretched to twenty‑four and twitched its nose,
CI brought scanners where the green light glows,
I nibble code crumbs and twitch my whiskered sigh.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is minimal and incomplete; it lists two bullet points but lacks detail on what was updated, version numbers, rationale, or testing information required by the template. Expand the description to include specific version upgrades, rationale for changes, testing performed, and any breaking changes or migration notes.
Title check ❓ Inconclusive The title 'feat: update' is vague and non-descriptive, failing to convey meaningful information about the changeset's primary updates. Improve the title to be more specific, such as 'feat: upgrade Node.js to 24.10 and update dependencies' or similar, clearly indicating the main changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/update

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Dockerfile (1)

1-12: Deploy image is missing runtime dependencies; likely to crash at startup.

In the deploy stage you only copy ./build and package.json, then run node ./build/index.js, but you never install or copy node_modules into this image. For a typical SvelteKit/Node build, the generated build/index.js will still import runtime dependencies, so the container will fail with Cannot find module ... at startup.

Recommend installing production deps (or copying them from the build stage), for example:

 FROM node:24.11.1-trixie-slim AS dependency
 WORKDIR /app
 COPY . .
 RUN npm ci
 RUN npm run build

 FROM node:24.11.1-trixie-slim AS deploy
 WORKDIR /app
 COPY --from=dependency /app/build ./build
-COPY package.json package.json
+COPY package*.json ./
+RUN npm ci --omit=dev
 EXPOSE 3000
 ENTRYPOINT [ "node", "./build/index.js" ]

(Alternatively, copy node_modules from the dependency stage instead of running npm ci again.)

🧹 Nitpick comments (3)
src/app.css (1)

97-97: Replace deprecated clip property with clip-path.

The clip property is deprecated. Use clip-path: inset(50%) instead for better browser compatibility and forward compatibility.

 .visually-hidden {
 	border: 0;
-	clip: rect(0 0 0 0);
+	clip-path: inset(50%);
 	height: auto;
 	margin: 0;
 	overflow: hidden;
 	padding: 0;
 	position: absolute;
 	width: 1px;
 	white-space: nowrap;
 }
.nvmrc (1)

1-1: Node 24.10 bump is fine; consider aligning with Docker image for consistency.

Using Node 24.10 here while the Docker image is 24.11.1 is acceptable (same major), but aligning them exactly can reduce “works locally but not in container” surprises over time.

package.json (1)

10-10: Error suppression in prepare script may hide issues.

The || echo '' suppresses svelte-kit sync failures silently. While this prevents CI failures during initial setup, it could mask legitimate errors.

Consider using a more explicit approach:

-		"prepare": "svelte-kit sync || echo ''",
+		"prepare": "svelte-kit sync || true",

Or add a comment explaining why suppression is intentional.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 494fa4f and 97d5890.

⛔ Files ignored due to path filters (3)
  • package-lock.json is excluded by !**/package-lock.json
  • static/favicon.png is excluded by !**/*.png
  • static/favicon.svg is excluded by !**/*.svg
📒 Files selected for processing (18)
  • .eslintrc.cjs (0 hunks)
  • .github/workflows/continuous_integration.yml (1 hunks)
  • .gitignore (1 hunks)
  • .nvmrc (1 hunks)
  • .prettierignore (1 hunks)
  • Dockerfile (1 hunks)
  • LICENSE (0 hunks)
  • eslint.config.js (1 hunks)
  • package.json (1 hunks)
  • playwright.config.ts (1 hunks)
  • src/app.css (1 hunks)
  • src/app.d.ts (1 hunks)
  • src/app.html (1 hunks)
  • static/robots.txt (1 hunks)
  • svelte.config.js (1 hunks)
  • tsconfig.json (1 hunks)
  • vite.config.ts (1 hunks)
  • vitest-setup-client.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • .eslintrc.cjs
  • LICENSE
🔇 Additional comments (16)
static/robots.txt (1)

1-1: LGTM!

The comment update is neutral and accurately describes the default crawl behavior. The directive remains unchanged (allowing all crawlers to access all content by default).

src/app.d.ts (2)

1-1: Documentation link updated to current Svelte docs location.

Good housekeeping to ensure references point to the correct documentation URL and prevent link rot.


8-8: PageState interface placeholder added.

This addition is consistent with the existing SvelteKit template pattern and provides helpful guidance to developers who may need to define page-level state types.

src/app.css (3)

13-13: Verify WCAG color contrast compliance for text on gradient backgrounds.

The text color rgba(0, 0, 0, 0.7) (70% opaque black) may not meet WCAG AA contrast standards when rendered over the semi-transparent radial gradient overlay on lighter background colors. Verify that contrast ratios meet accessibility standards, especially for body text.

Consider running a contrast checker or adding a note documenting the contrast ratios for the text/background combinations used throughout the stylesheet.

Also applies to: 24-28


14-14: Clarify inconsistent width values.

The CSS variable --column-width is set to 42rem (line 14), but .text-column uses max-width: 48rem (line 72). Confirm whether this difference is intentional or if they should be aligned.

Also applies to: 70-77


1-1: @fontsource/fira-mono is properly listed as a dependency.

The package is present in package.json with version ^5.2.7, so the import in src/app.css is supported and safe for deployment.

src/app.html (1)

5-6: Favicon SVG + improved viewport look good; ensure the asset exists.

Switching to favicon.svg and adding initial-scale=1 to the viewport meta are both good improvements; just confirm that static/assets actually contains favicon.svg at the expected path so this doesn’t 404.

.gitignore (1)

1-1: Sensible ignore additions for test and hosting artifacts.

Ignoring test-results, .netlify, and .wrangler keeps local/hosting artifacts out of Git and is aligned with typical Svelte/Node setups; no issues here.

Also applies to: 7-8

vitest-setup-client.ts (1)

1-2: Vitest browser typings wiring is properly configured.

The /// <reference ...> directives are correct for enabling @vitest/browser matchers and Playwright provider typings. Confirmed that vitest-setup-client.ts is already wired into the Vitest config via setupFiles in vite.config.ts at line 21, so the extra types will flow into your test files as expected.

tsconfig.json (1)

14-18: TS config comments now reflect current SvelteKit guidance.

Removing the explicit target override and extending ./.svelte-kit/tsconfig.json matches SvelteKit's recommended approach. The generated tsconfig automatically includes path aliases like $lib when you run the dev server, so keeping your tsconfig minimal and focused on necessary overrides (while avoiding SvelteKit-required fields) keeps the config maintainable and aligned with the official setup.

playwright.config.ts (1)

1-9: Clean and minimal Playwright configuration.

The simplified config using defineConfig with inline options is idiomatic for Playwright. The testDir: 'e2e' aligns with the test:e2e script in package.json.

eslint.config.js (1)

13-41: Well-organized ESLint flat config structure.

The configuration correctly:

  • Integrates .gitignore patterns via includeIgnoreFile
  • Combines recommended presets from JS, TypeScript, Svelte, and Prettier
  • Sets appropriate globals for both browser and Node environments
  • Disables no-undef for TypeScript with proper justification
  • Configures Svelte parser with projectService: true for type-aware linting
package.json (2)

15-17: Clean test script separation.

The test structure properly separates unit (vitest) and e2e (playwright test) tests. Using --run flag ensures vitest runs once in CI mode, and the && chaining ensures e2e tests only execute if unit tests pass.


24-24: Both packages serve different purposes and are not redundant.

@playwright/test is the test runner framework, while playwright is needed for browser binary installation and direct browser automation outside of tests. According to Playwright's official documentation, both packages are typically required—@playwright/test for testing and playwright (or playwright-core) for the browser binaries.

vite.config.ts (1)

7-33: Well-structured multi-project Vitest configuration.

The separation of client (browser/Svelte) and server (Node) test environments is clean. The requireAssertions: true setting enforces test quality by ensuring each test has explicit assertions.

The extends: './vite.config.ts' pattern correctly reuses the base plugins and settings while allowing each project to override test-specific options.

.prettierignore (1)

1-9: No action needed. The .prettierignore file is appropriate as-is. Prettier respects .gitignore by default, so node_modules/, /build/, and /.svelte-kit/ are already ignored without requiring explicit entries in .prettierignore.

Comment on lines +6 to +9
// Consult https://svelte.dev/docs/kit/integrations
// for more information about preprocessors
preprocess: [vitePreprocess()],

kit: {
// adapter-auto only supports some environments, see https://kit.svelte.dev/docs/adapter-auto for a list.
// If your environment is not supported, or you settled on a specific environment, switch out the adapter.
// See https://kit.svelte.dev/docs/adapters for more information about adapters.
adapter: adapter()
}
preprocess: vitePreprocess(),
kit: { adapter: adapter() }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

recommended minimal svelte.config.js SvelteKit @sveltejs/adapter-node vitePreprocess

💡 Result:

Minimal recommended svelte.config.js:

// svelte.config.js
import adapter from '@sveltejs/adapter-node';
import { vitePreprocess } from '@sveltejs/vite-plugin-svelte';

/** @type {import('@sveltejs/kit').Config} */
const config = {
preprocess: [vitePreprocess()],
kit: {
adapter: adapter()
}
};

export default config;

Notes:

  • vitePreprocess should be imported from @sveltejs/vite-plugin-svelte (not from @sveltejs/kit). [1][2]
  • adapter-node defaults (out: 'build', precompress: true) can be configured by passing options to adapter(). [3]

Sources:

  1. @sveltejs/adapter-node (usage & options). [1]
  2. vitePreprocess documentation (import from @sveltejs/vite-plugin-svelte). [2]
  3. SvelteKit configuration docs. [3]

Svelte config needs preprocess wrapped in array; current form may not work as intended.

The kit: { adapter: adapter() } setup is correct and matches SvelteKit recommendations for adapter-node. However, preprocess: vitePreprocess() should be preprocess: [vitePreprocess()] (as an array) according to official SvelteKit documentation. The direct function call without array wrapping is not the recommended pattern and may cause preprocessing to not function correctly.

🤖 Prompt for AI Agents
In svelte.config.js around lines 6 to 9, the preprocess option is currently set
to a single function call (preprocess: vitePreprocess()) but should be an array;
update the preprocess entry to wrap the vitePreprocess() call in an array
(preprocess: [vitePreprocess()]) while leaving the existing kit adapter
configuration unchanged so SvelteKit picks up preprocessors correctly.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
e2e/demo.test.ts (1)

3-7: Align test title with what is being asserted

The test description says "home page has expected h1", but the assertions target a <nav> containing "Auth +". Consider either renaming the test (e.g. “home page shows Auth + in nav”) or asserting on the actual <h1> element instead.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97d5890 and bf42651.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • e2e/demo.test.ts (1 hunks)
  • package.json (1 hunks)
  • vite.config.ts (1 hunks)
🔇 Additional comments (2)
package.json (2)

10-17: Test script flow matches the new unit + e2e split

"test": "npm run test:unit -- --run && npm run test:e2e" correctly runs Vitest in non-watch mode first and then Playwright e2e tests. This aligns with the new vitest config and playwright setup.


3-5: No CommonJS code found — "type": "module" is safe

The codebase contains no require() or module.exports patterns. Setting "type": "module" is safe and aligns with the modern ESM-based tooling (SvelteKit, Vite, Vitest, Playwright).

Comment on lines +9 to +33
projects: [
{
extends: './vite.config.ts',
test: {
name: 'client',
browser: {
enabled: true,
provider: playwright(),
instances: [{ browser: 'chromium' }]
},
include: ['src/**/*.svelte.{test,spec}.{js,ts}'],
exclude: ['src/lib/server/**'],
setupFiles: ['./vitest-setup-client.ts']
}
},
{
extends: './vite.config.ts',
test: {
name: 'server',
environment: 'node',
include: ['src/**/*.{test,spec}.{js,ts}'],
exclude: ['src/**/*.svelte.{test,spec}.{js,ts}']
}
}
}
]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid self-extending Vitest projects from the same vite.config.ts

Each project currently does:

{
  extends: './vite.config.ts',
  test: { /* project options */ }
}

inside vite.config.ts itself. This is self-referential and risks confusing/recursive config merging when Vitest loads this file.

You can simplify by defining project-specific test options directly, without extends, e.g.:

 export default defineConfig({
   plugins: [sveltekit()],
   test: {
-    expect: { requireAssertions: true },
-    projects: [
-      {
-        extends: './vite.config.ts',
-        test: {
-          name: 'client',
-          browser: {
-            enabled: true,
-            provider: playwright(),
-            instances: [{ browser: 'chromium' }]
-          },
-          include: ['src/**/*.svelte.{test,spec}.{js,ts}'],
-          exclude: ['src/lib/server/**'],
-          setupFiles: ['./vitest-setup-client.ts']
-        }
-      },
-      {
-        extends: './vite.config.ts',
-        test: {
-          name: 'server',
-          environment: 'node',
-          include: ['src/**/*.{test,spec}.{js,ts}'],
-          exclude: ['src/**/*.svelte.{test,spec}.{js,ts}']
-        }
-      }
-    ]
+    expect: { requireAssertions: true },
+    projects: [
+      {
+        name: 'client',
+        browser: {
+          enabled: true,
+          provider: playwright(),
+          instances: [{ browser: 'chromium' }]
+        },
+        include: ['src/**/*.svelte.{test,spec}.{js,ts}'],
+        exclude: ['src/lib/server/**'],
+        setupFiles: ['./vitest-setup-client.ts']
+      },
+      {
+        name: 'server',
+        environment: 'node',
+        include: ['src/**/*.{test,spec}.{js,ts}'],
+        exclude: ['src/**/*.svelte.{test,spec}.{js,ts}']
+      }
+    ]
   }
 })

This avoids self-extension while keeping the intended client/server split.

🤖 Prompt for AI Agents
In vite.config.ts around lines 9 to 33, the two Vitest project entries
incorrectly set "extends: './vite.config.ts'" causing self-referential/recursive
config merging; remove the extends property and put each project's test options
directly on the project object (i.e., keep the project objects with
name/environment/browser/include/exclude/setupFiles fields as-is but delete the
extends key) so Vitest uses those project-specific test settings without
referencing the same config file.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf42651 and c257882.

📒 Files selected for processing (1)
  • .github/workflows/continuous_integration.yml (2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/continuous_integration.yml

17-17: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions

(deprecated-commands)


40-40: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions

(deprecated-commands)


59-59: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions

(deprecated-commands)

🔇 Additional comments (1)
.github/workflows/continuous_integration.yml (1)

13-13: Action version upgrades look good.

The upgrades from v4 to v6 for actions/checkout and actions/setup-node are straightforward modernizations. Once the SonarCloud and set-output issues above are resolved, these changes align well with the updated CI pipeline.

Also applies to: 20-20, 36-36, 43-43, 55-55, 62-62

Comment on lines +27 to +31
- name: SonarCloud Scan
uses: SonarSource/sonarqube-scan-action@v6
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

SonarSource sonarqube-scan-action security advisory v6 v5.3.1 2025

💡 Result:

Short summary — what happened, affected versions, and what to do:

  • Issue: A command‑injection / argument‑injection vulnerability in the SonarQube Scanner GitHub Action that treats untrusted args as shell expressions. CVE‑2025‑58178 (initial) and a follow‑up advisory covering a windows args bypass were published. [1][2][3]
  • Affected versions: sonarqube-scan-action >= 4.0.0 and <= 5.3.0 (initial). A follow‑up advisory covers >=4.0.0 and <6.0.0 for a Windows args bypass. [2][3]
  • Patched versions / recommended upgrades:
    • Immediate patch: update to v5.3.1 (fix for CVE‑2025‑58178). [2]
    • Stronger fix and rewrite: upgrade to v6.0.0 (addresses the later args injection bypass on Windows). SonarSource recommends moving to v6.0.0. [3][1]
  • Dates: original advisory & GHSA published Sept 1–2, 2025; v6 announcement Sept 25, 2025. [1][2][3]

Recommended action: update workflows to at least v5.3.1 immediately; if possible, migrate to v6.0.0 and verify any use of dynamic args is converted to safe GitHub Actions expression syntax. [1][2][3]

Sources:
[1] SonarSource community posts (security advisory and v6 announcement).
[2] GitHub Advisory / GHSA-f79p-9c5r-xg88 (CVE‑2025‑58178) — patched in 5.3.1.
[3] GitHub security advisory / GHSA-5xq9-5g24-4g6f — v6.0.0 fixes Windows args bypass.


🌐 Web query:

sonarqube-scan-action GitHub Action releases security fixes v6

💡 Result:

Short answer: SonarSource released sonarqube-scan-action v6.0.0 (major rewrite from Bash to JS) to fix a command/argument-injection vulnerability (CVE-2025-59844 / GHSA-5xq9-5g24-4g6f). Upgrade to v6.0.0+ and review the changed args quoting behavior. [1][2][3]

Sources:

  • Release v6.0.0 (release notes / breaking change). [1]
  • GitHub security advisory GHSA-5xq9-5g24-4g6f (patched in v6.0.0). [2]
  • OSV / CVE entry for CVE-2025-59844 (details and mitigation). [3]

🏁 Script executed:

# Check if there are any comments or past review history in the repository
git log --oneline --all -- .github/workflows/continuous_integration.yml | head -20

Repository: auth-plus/auth-plus-client

Length of output: 156


🏁 Script executed:

# Look for any documentation about SonarCloud setup or version requirements in the repo
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs grep -l "SonarCloud\|sonarqube-scan" 2>/dev/null | head -10

Repository: auth-plus/auth-plus-client

Length of output: 52


Add fork PR guard condition for SonarCloud step.

GitHub does not expose repository secrets to workflows triggered by pull requests from forked repositories. The SONAR_TOKEN will be empty on fork PRs, causing the scan to fail. Add an if: condition to skip this step for fork PRs:

if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository

The SonarSource/sonarqube-scan-action@v6 version is correct—v6.0.0 is the official recommended version that fixes known command-injection vulnerabilities.

🤖 Prompt for AI Agents
.github/workflows/continuous_integration.yml lines 27-31: the SonarCloud Scan
step will fail on forked PRs because secrets (SONAR_TOKEN) are not available;
add a guard to skip the step for fork PRs by adding an if condition that allows
the step only for pushes or when the PR head repo equals the base repo (use: if:
github.event_name == 'push' || github.event.pull_request.head.repo.full_name ==
github.repository), keeping the existing action version unchanged.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
playwright.config.ts (1)

1-10: Playwright config looks good; reporter duplication with CLI is the only thing to watch

The webServer, testDir: 'e2e', and inline JUnit reporter with outputFile: 'results.xml' are all in line with Playwright’s documented patterns and should work as expected. (playwright.dev)

Since package.json’s test script also passes --reporter=junit, you effectively configure the reporter in two places; consider standardizing on either the config-level reporter or the CLI flag to avoid confusion in future edits.

package.json (1)

20-45: Playwright/Vitest dependency combo is powerful but playwright + @playwright/test together can conflict

The updated devDependencies look coherent overall (ESLint 9 stack, Svelte 5 + SvelteKit 2, Vite 7, Vitest 4, Playwright 1.57, etc.), and align with current ecosystem guidance for this toolchain. (main.vitest.dev)

One thing to be careful about:

  • You have both @playwright/test and playwright installed:
    • @playwright/test – test runner you’re using for test:e2e.
    • playwright – required for Vitest Browser Mode with @vitest/browser-playwright and for direct automation APIs.

Upstream, there is a known issue where installing both can cause the playwright CLI binary to point at the wrong package depending on install order, breaking playwright test invocations. (github.com) That’s directly relevant because your test:e2e script is:

"test:e2e": "playwright test"

Suggestions:

  • At minimum, verify locally and in CI that playwright test is truly invoking the @playwright/test runner (and not the base playwright library CLI) by checking playwright test --version and observing test behavior.
  • If you hit conflicts, consider:
    • Keeping playwright for Vitest Browser Mode but wiring test:e2e to the runner explicitly, e.g. via node node_modules/@playwright/test/cli.js test, or
    • Dropping the direct playwright dependency if you don’t actually need the library API outside Vitest’s browser provider (and rely on Vitest’s documented setup only).
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 476ee28 and 126ca5f.

📒 Files selected for processing (3)
  • package.json (1 hunks)
  • playwright.config.ts (1 hunks)
  • sonar-project.properties (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • sonar-project.properties
🔇 Additional comments (1)
package.json (1)

3-5: ESM switch (\"type\": \"module\") looks fine; verify Node configs and scripts

Marking the package as ESM is reasonable for this stack (Vite/SvelteKit/ESLint 9, etc.), but it can break any remaining CommonJS-based tooling or config files that still rely on require/module.exports and a plain .js extension.

Please double‑check that:

  • Any Node-loaded config (vitest.config.*, playwright.config.*, eslint.config.*, etc.) is either ESM (.mjs/ESM syntax) or explicitly .cjs if still CommonJS.
  • Any custom Node scripts you might have (outside the shown files) are compatible with ESM resolution.

Comment on lines +10 to +17
"prepare": "svelte-kit sync || echo ''",
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"lint": "prettier --check . && eslint .",
"format": "prettier --write .",
"test:integration": "playwright test",
"test:unit": "vitest"
"lint": "prettier --check . && eslint .",
"test:unit": "vitest",
"test": "npm run test:unit -- --run && PLAYWRIGHT_JSON_OUTPUT_NAME=results.xml npm run test:e2e -- --reporter=junit",
"test:e2e": "playwright test"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E "(playwright|tsconfig)" | head -20

Repository: auth-plus/auth-plus-client

Length of output: 104


🏁 Script executed:

fd "playwright" -type f

Repository: auth-plus/auth-plus-client

Length of output: 240


🏁 Script executed:

cat -n package.json | grep -A 10 -B 2 "test"

Repository: auth-plus/auth-plus-client

Length of output: 1475


🏁 Script executed:

cat -n playwright.config.ts 2>/dev/null || echo "File not found, checking for .js variant"
fd "playwright.config"

Repository: auth-plus/auth-plus-client

Length of output: 391


🌐 Web query:

Playwright PLAYWRIGHT_JSON_OUTPUT_NAME PLAYWRIGHT_JUNIT_OUTPUT_NAME environment variables reporters

💡 Result:

Short answer

  • PLAYWRIGHT_JSON_OUTPUT_NAME and PLAYWRIGHT_JUNIT_OUTPUT_NAME are environment variables Playwright Test reads when you run reporters via the CLI to set the base output file name for the json or junit reporter (when using --reporter=json or --reporter=junit). Example:
    • Bash: PLAYWRIGHT_JSON_OUTPUT_NAME=results.json npx playwright test --reporter=json
    • Bash: PLAYWRIGHT_JUNIT_OUTPUT_NAME=results.xml npx playwright test --reporter=junit. [1][2]

Notes / related env vars

  • You can also set full path and/or directory with:
    • PLAYWRIGHT_*_OUTPUT_FILE — full path to the output file (if set, it overrides DIR/NAME).
    • PLAYWRIGHT_*_OUTPUT_DIR — directory where the output is written (ignored if OUTPUT_FILE is set). [1][3]
  • Alternatively, configure reporter output in playwright.config (reporter: [['json',{ outputFile: 'results.json' }]] or ['junit',{ outputFile: 'results.xml' }]). [1][2]

Sources
[1] Playwright official reporters docs.
[2] Playwright JUnit/docs pages (examples).
[3] Community/runebook replica of Playwright reporters docs.


Fix environment variable or simplify to config-level reporter configuration

In the test script:

"test": "npm run test:unit -- --run && PLAYWRIGHT_JSON_OUTPUT_NAME=results.xml npm run test:e2e -- --reporter=junit"

The PLAYWRIGHT_JSON_OUTPUT_NAME env var only affects the JSON reporter, but you're using --reporter=junit. This env var won't set the JUnit output filename.

Since playwright.config.ts already defines reporter: [['junit', { outputFile: 'results.xml' }]], the simplest fix is to remove the redundant CLI reporter flag and env var:

-    "test": "npm run test:unit -- --run && PLAYWRIGHT_JSON_OUTPUT_NAME=results.xml npm run test:e2e -- --reporter=junit",
+    "test": "npm run test:unit -- --run && npm run test:e2e",

This uses the config-level reporter as the single source of truth, avoiding platform-specific env var issues.

If you must use the CLI reporter instead, change the env var to PLAYWRIGHT_JUNIT_OUTPUT_NAME:

-    "test": "npm run test:unit -- --run && PLAYWRIGHT_JSON_OUTPUT_NAME=results.xml npm run test:e2e -- --reporter=junit",
+    "test": "npm run test:unit -- --run && PLAYWRIGHT_JUNIT_OUTPUT_NAME=results.xml npm run test:e2e -- --reporter=junit",
🤖 Prompt for AI Agents
In package.json around lines 10-17, the "test" script sets
PLAYWRIGHT_JSON_OUTPUT_NAME and passes --reporter=junit which is inconsistent
(the env var only affects the JSON reporter) and redundant because
playwright.config.ts already configures the junit reporter; fix by removing the
PLAYWRIGHT_JSON_OUTPUT_NAME environment variable and the CLI --reporter flag
from the test script so Playwright uses the reporter defined in
playwright.config.ts (alternatively, if you prefer CLI configuration, replace
the env var with PLAYWRIGHT_JUNIT_OUTPUT_NAME and pass the appropriate CLI
reporter options, but the simplest fix is to remove both and rely on the
config).

@AndrewHanasiro AndrewHanasiro merged commit af3e124 into main Dec 11, 2025
4 of 5 checks passed
@AndrewHanasiro AndrewHanasiro deleted the feature/update branch December 11, 2025 23:31
@coderabbitai coderabbitai bot mentioned this pull request Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant