Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the preview functionality in Docker by addressing two critical issues: nginx routing and runtime dependency availability. The preview feature (/api/preview/... endpoints) was broken because the SPA fallback was intercepting API requests, and because dynamically imported build tools (esbuild, tailwindcss, postcss) were missing at runtime.
Changes:
- Fixed nginx location directive to use
^~modifier, preventing SPA fallback from intercepting/api/routes - Externalized esbuild, tailwindcss, and postcss from the API bundle and installed them separately in dist/node_modules/
- Simplified Dockerfile dependency and source copying using directory-level operations and find command
- Added deprecation endpoint for renamed API route with 410 Gone status
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docker/nginx.conf | Added ^~ modifier to /api/ location to prioritize proxy over SPA fallback |
| docker/nginx-single.conf | Added ^~ modifier to /api/ location to prioritize proxy over SPA fallback |
| apps/api/src/routes/stages.ts | Added tombstone endpoint for deprecated /books/:label/steps/run route returning 410 Gone |
| apps/api/scripts/bundle-server.mjs | Marked esbuild, tailwindcss, postcss as external to preserve runtime dynamic import capability |
| Dockerfile | Replaced individual file COPY with directory-level COPY; added runtime installation of external dependencies into dist/node_modules/ using npm; improved maintainability with find-based package.json discovery |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| app.post("/books/:label/steps/run", (c) => { | ||
| return c.json( | ||
| { | ||
| error: | ||
| "This endpoint was removed. Use POST /books/:label/stages/run " + | ||
| "with body { fromStage: string, toStage: string }.", | ||
| }, | ||
| 410 | ||
| ) | ||
| }) |
There was a problem hiding this comment.
The deprecated endpoint handler should include test coverage. Other route files in this codebase (books.test.ts, pages.test.ts, prompts.test.ts, etc.) have comprehensive test coverage, but stages.ts has no test file. Consider adding a test to verify that requests to the deprecated endpoint return a 410 status with the expected error message.
No description provided.