Skip to content

Conversation

@RawSalmon69
Copy link
Member

No description provided.

@RawSalmon69 RawSalmon69 requested a review from Copilot July 4, 2025 12:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR sets up a health-check endpoint and aligns CI/CD pipelines, Docker configurations, and documentation for a dual dev/prod deployment workflow.

  • Adds a GET /api/health route and a local test script to verify server readiness.
  • Switches Astro to server output with the Node adapter, updates layout to include header/footer, and configures Vite proxy.
  • Updates Dockerfiles, GitHub Actions workflows, and project docs to support consistent builds, tests, and deployments.

Reviewed Changes

Copilot reviewed 20 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/pages/api/health.ts New health-check API route with status, uptime, and responseTime
scripts/test-health.js Local Node script to spawn server and validate the health endpoint
astro.config.mjs Enable server output, add @astrojs/node adapter, and set up proxy
src/layouts/MainLayout.astro Import Header/Footer, update lang and wrap body with layout
src/components/common/Header.astro Navigation header with active link styling and mobile menu toggle
src/components/common/Footer.astro Footer component with dynamic year and quick links
src/pages/about.astro Static About page content and “Register Now” CTA
src/lib/api.ts Typed apiRequest utility, ApiError, and convenience methods
package.json Add start script, @astrojs/node, and dotenv dependency
pnpm-workspace.yaml Remove obsolete workspace setting
README.md Expanded architecture, setup, commands, and CI/CD documentation
Dockerfile.alternative Alternative multi-stage Dockerfile with runtime env ARGs
Dockerfile Primary multi-stage Dockerfile with correct healthcheck on /api/health
DEVELOPER_GUIDELINES.md Comprehensive developer rules and conventions
.husky/pre-commit Pre-commit hook to run pnpm build
.github/workflows/pr-checks.yml Lint, build, and Docker test jobs for PR validation
.github/workflows/deploy.yml GCP deploy jobs for dev and prod branches
.env.template Remove obsolete .env.template entry
.env.example Provide .env.local template for development env
.dockerignore Define files to ignore in Docker builds
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)

README.md:61

  • [nitpick] The directory name fdrpkm68-frontend looks like a placeholder. Update it to match the actual repo name or use a generic cd <repo-name> instruction.
cd fdrpkm68-frontend

.github/workflows/pr-checks.yml:32

  • The continue-on-error: true setting on the format check allows formatting failures to pass silently. Remove it so that formatting errors cause the job to fail.
      run: pnpm exec prettier --check .

.github/workflows/pr-checks.yml:36

  • TypeScript/type-check failures are ignored due to continue-on-error: true. Remove that flag to enforce type-checking in CI.
      run: pnpm exec astro check


# Health check
HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \
CMD node -e "require('http').get('http://localhost:4321', (res) => { process.exit(res.statusCode === 200 ? 0 : 1) })"
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

The healthcheck command points to the root path (http://localhost:4321) instead of /api/health. Update it to http://localhost:4321/api/health to accurately verify service health.

Suggested change
CMD node -e "require('http').get('http://localhost:4321', (res) => { process.exit(res.statusCode === 200 ? 0 : 1) })"
CMD node -e "require('http').get('http://localhost:4321/api/health', (res) => { process.exit(res.statusCode === 200 ? 0 : 1) })"

Copilot uses AI. Check for mistakes.
Comment on lines +22 to +23
// Wait for server to start
setTimeout(() => {
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Using a fixed timeout to wait for the server startup can be brittle. Consider implementing a retry loop or polling the health endpoint until it responds instead of relying on an arbitrary delay.

Suggested change
// Wait for server to start
setTimeout(() => {
// Wait for server to start with retry mechanism
const MAX_RETRIES = 10;
const RETRY_DELAY = 500; // in milliseconds
function checkHealth(retriesLeft) {

Copilot uses AI. Check for mistakes.
"@astrojs/node": "^9.3.0",
"@tailwindcss/vite": "^4.1.11",
"astro": "^5.10.1",
"dotenv": "^17.0.1",
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

The dotenv dependency is added but not used anywhere in the code. Either load environment variables via dotenv.config() in your entrypoint or remove this dependency.

Copilot uses AI. Check for mistakes.
@RawSalmon69 RawSalmon69 self-assigned this Jul 4, 2025
@RawSalmon69 RawSalmon69 closed this Jul 4, 2025
BadLuckZ added a commit that referenced this pull request Jul 13, 2025
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