Skip to content

Namefeature/service detail pages#95

Open
kumudranjan6127-debug wants to merge 5 commits into
hrx01-dev:mainfrom
kumudranjan6127-debug:namefeature/service-detail-pages
Open

Namefeature/service detail pages#95
kumudranjan6127-debug wants to merge 5 commits into
hrx01-dev:mainfrom
kumudranjan6127-debug:namefeature/service-detail-pages

Conversation

@kumudranjan6127-debug

@kumudranjan6127-debug kumudranjan6127-debug commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Description

Summary

  • Restores scripts/prerender.js (reverted in Revert "reduce the time of prerender screen for better seo and crawbility" #87) with --no-sandbox --disable-setuid-sandbox args so Puppeteer can launch in GitHub Actions on Ubuntu 23.10+ (AppArmor blocks unprivileged user namespaces without this flag)
  • Adds puppeteer: ^25.1.0 to devDependencies and restores vite build && node scripts/prerender.js build script
  • Bumps Node.js in CI from 20 → 24 (Node 20 is deprecated by the runner)

Root cause

Job 82516834683 failed with:
[FATAL] No usable sandbox! If you are running on Ubuntu 23.10+...

Fixes the CI build failure.

Summary by CodeRabbit

  • New Features

    • Introduced automatic static HTML prerendering for select application routes during the build phase, enhancing page delivery performance and search engine optimization.
  • Chores

    • Updated the continuous integration workflow to use Node.js version 24.
    • Added Puppeteer as a development dependency to support build processes.

kumudranjan6127-debug and others added 5 commits June 21, 2026 03:25
The pre-rendering script failed in GitHub Actions because Puppeteer could
not launch Chrome without sandbox support (Ubuntu 23.10+ / AppArmor).

Fix: pass --no-sandbox and --disable-setuid-sandbox to puppeteer.launch()
so Chrome starts correctly in the headless CI environment.

Also restore scripts/prerender.js and re-add the build step with puppeteer
as a devDependency. Update ci.yml Node from 20 to 22 (deprecation notice).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

A new scripts/prerender.js post-build script is added that spins up a local HTTP server over the dist directory, launches a headless Puppeteer browser, renders configured routes to static HTML, and writes the output back into dist. The build script is updated to run this after vite build, puppeteer ^25.1.0 is added as a devDependency, and the CI workflow is updated to Node.js 24.

Changes

Puppeteer Pre-render Pipeline

Layer / File(s) Summary
Build script, dependency, and CI wiring
.github/workflows/ci.yml, package.json
build script chains node scripts/prerender.js after vite build; puppeteer ^25.1.0 added to devDependencies; CI Node.js version bumped from 20 to 24.
Pre-render script: static server and Puppeteer workflow
scripts/prerender.js
Defines ROUTES, a local static-file HTTP server with containment check (403 on traversal, 404 on missing), and prerender() which validates dist, starts the server, launches headless Puppeteer with CI-safe flags, renders each route via waitUntil: 'networkidle2', writes captured HTML to the correct dist output path, accumulates failures, and cleans up in finally.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • hrx01-dev/Servio#42: Introduces the same Puppeteer-based pre-rendering flow, directly overlapping with this PR's changes to package.json build script and scripts/prerender.js.
  • hrx01-dev/Servio#87: Removes scripts/prerender.js and the prerender build step, directly conflicting with this PR's re-introduction of the same file and behavior.

Suggested reviewers

  • hrx01-dev

Poem

🐇 Hop hop, the pages now bake,
Puppeteer renders for goodness sake!
A server spins up, routes rendered to file,
Static HTML stretching mile by mile.
Node twenty-four keeps the CI bright —
The rabbit pre-renders through the night! 🌙

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Namefeature/service detail pages' does not align with the PR's actual objectives of restoring pre-rendering functionality and fixing a CI build failure. Update the title to reflect the main changes, such as 'Restore pre-rendering and fix Node.js version in CI' or 'Fix Puppeteer sandbox issue and update CI Node version'.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
scripts/prerender.js (1)

92-97: ⚡ Quick win

Scope --no-sandbox flags to CI-only execution.

Keeping sandbox disabled for all environments weakens local security posture; apply these args only when process.env.CI is truthy.

Proposed fix
-    browser = await puppeteer.launch({
-      headless: true,
-      args: ['--no-sandbox', '--disable-setuid-sandbox'],
-    });
+    const launchArgs = process.env.CI
+      ? ['--no-sandbox', '--disable-setuid-sandbox']
+      : [];
+    browser = await puppeteer.launch({
+      headless: true,
+      args: launchArgs,
+    });
🤖 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 `@scripts/prerender.js` around lines 92 - 97, The puppeteer.launch() call
unconditionally disables sandbox security features for all environments,
weakening local security. Modify the args array passed to puppeteer.launch() to
conditionally include the '--no-sandbox' and '--disable-setuid-sandbox' flags
only when process.env.CI is truthy, ensuring sandbox protection is maintained in
local development while allowing these flags only in CI/CD environments.
🤖 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 `@scripts/prerender.js`:
- Line 18: The hardcoded PORT constant set to 3000 creates port collision issues
in local development environments, and the startServer function does not
properly handle or reject on listen errors. Modify the PORT constant to use a
dynamic value from environment variables (with a fallback) instead of hardcoding
it to 3000, and update the startServer function to properly catch and reject
when the server fails to listen on the assigned port. Apply these changes
throughout the file wherever PORT is referenced and wherever startServer is
called (including the locations mentioned in the comment at lines 22-24, 70-73,
90, and 111).
- Around line 35-40: The directory traversal protection check using
filePath.startsWith(distDirResolved) is vulnerable because it only performs a
prefix comparison without checking for path separators. This allows bypassing
the check with sibling paths that share the same prefix. Replace this check with
a separator-safe containment verification by ensuring that after the
distDirResolved path, there is either nothing or a path separator (use
path.sep), or alternatively use path.relative() to verify the resolved path does
not escape the directory with parent references like '..'.

---

Nitpick comments:
In `@scripts/prerender.js`:
- Around line 92-97: The puppeteer.launch() call unconditionally disables
sandbox security features for all environments, weakening local security. Modify
the args array passed to puppeteer.launch() to conditionally include the
'--no-sandbox' and '--disable-setuid-sandbox' flags only when process.env.CI is
truthy, ensuring sandbox protection is maintained in local development while
allowing these flags only in CI/CD environments.
🪄 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: e2563ce8-248d-486b-aa7e-610aeaad09e8

📥 Commits

Reviewing files that changed from the base of the PR and between e26e5f4 and a7344f6.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • package.json
  • scripts/prerender.js

Comment thread scripts/prerender.js
Comment thread scripts/prerender.js
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