Skip to content

Comments

Dockerfile: build backend and frontend into container#40

Merged
techbuzzz merged 8 commits intotechbuzzz:mainfrom
Boodya:main
Feb 19, 2026
Merged

Dockerfile: build backend and frontend into container#40
techbuzzz merged 8 commits intotechbuzzz:mainfrom
Boodya:main

Conversation

@Boodya
Copy link
Contributor

@Boodya Boodya commented Feb 5, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 5, 2026 15:30
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 pull request adds a multi-stage Dockerfile that builds both the Vue.js frontend and Go backend into a single container image, and updates the backend server to serve the frontend static files. This enables a unified deployment model where the backend can optionally serve the frontend directly.

Changes:

  • Added multi-stage Dockerfile with frontend (Node.js) and backend (Go) build stages
  • Backend now serves static files from ./web/dist with SPA fallback routing
  • Static file serving is optional - backend runs standalone if frontend is not built

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
Dockerfile Added 3-stage build: frontend (Node.js/Vue), backend (Go), and final Alpine image with both artifacts
cmd/server/main.go Added static file serving with SPA routing support, registered before MCP catch-all routes

}

// Try to serve the requested file
path := distDir + req.URL.Path
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Using string concatenation to construct file paths is vulnerable to path traversal attacks and doesn't handle path separators correctly across operating systems. Import "path/filepath" and use filepath.Join with filepath.Clean to safely construct the path. For example: path := filepath.Join(distDir, filepath.Clean(req.URL.Path)). This prevents attackers from using paths like "/../../../etc/passwd" to access files outside the intended directory. This follows the established convention in the codebase (see cmd/server/migrations_test.go and internal/task/store.go).

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +14
# Stage 1: Build Frontend (Vue.js)
FROM node:18-alpine AS frontend-builder

WORKDIR /app

# Copy frontend package files
COPY web/package*.json ./
RUN npm install

# Copy frontend source
COPY web/ ./

# Build frontend for production
RUN npm run build
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Consider adding a .dockerignore file to exclude unnecessary files from the Docker build context. This will reduce build time and image size by excluding files like node_modules, .git, test files, and development artifacts. Create a .dockerignore with entries such as: .git, node_modules, web/node_modules, .test, coverage., .vscode, .idea, *.md (except those explicitly copied), and postgres_data/.

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +11
# Copy frontend source
COPY web/ ./
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The COPY command copies the entire web/ directory including any locally built files, node_modules, or other artifacts that might exist in the build context. Since npm install has already been run at line 8, this could cause conflicts or include unnecessary files. Consider using a .dockerignore file to exclude web/node_modules, web/dist, and other build artifacts from the build context. Alternatively, copy only the necessary source files explicitly (e.g., COPY web/src ./src, COPY web/public ./public, etc.).

Suggested change
# Copy frontend source
COPY web/ ./
# Copy frontend source and config (exclude node_modules, dist, etc.)
COPY web/src ./src
COPY web/public ./public
COPY web/vite.config.* ./
COPY web/tsconfig*.json ./
COPY web/.eslintrc* ./
COPY web/postcss.config.* ./
COPY web/tailwind.config.* ./

Copilot uses AI. Check for mistakes.
Comment on lines 146 to 173
// Serve static files from web/dist (if exists) - BEFORE catch-all routes
distDir := "./web/dist"
if _, err := os.Stat(distDir); err == nil {
log.Println("Serving frontend from ./web/dist")
fs := http.FileServer(http.Dir(distDir))
r.PathPrefix("/").Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
// Skip static serving for API/MCP routes
if strings.HasPrefix(req.URL.Path, "/api") ||
strings.HasPrefix(req.URL.Path, "/mcp") ||
strings.HasPrefix(req.URL.Path, "/ws") ||
strings.HasPrefix(req.URL.Path, "/a2a") ||
strings.HasPrefix(req.URL.Path, "/.well-known") ||
req.URL.Path == "/health" {
// Let other handlers handle it
http.NotFound(w, req)
return
}

// Try to serve the requested file
path := distDir + req.URL.Path
if _, err := os.Stat(path); os.IsNotExist(err) {
// File doesn't exist, serve index.html for SPA routing
http.ServeFile(w, req, distDir+"/index.html")
return
}
// File exists, serve it
fs.ServeHTTP(w, req)
}))
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The new static file serving logic lacks test coverage. Given that the codebase has integration tests for handlers (see tests/a2a/integration_test.go), consider adding integration tests for the static file serving functionality. Tests should cover: serving existing static files, serving index.html for non-existent paths (SPA routing), handling of API/MCP route exclusions, and path traversal attack prevention.

Copilot uses AI. Check for mistakes.
techbuzzz and others added 2 commits February 19, 2026 14:21
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@techbuzzz techbuzzz merged commit 809753f into techbuzzz:main Feb 19, 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.

2 participants