Encapsulate Deployments in Project Folders#40
Conversation
This Sprint 1 refactor moves all project files into a single folder for better
ownership and simplify the port system from 4 ports per project to 3.
Key Changes:
- Encapsulated structure: Each project now has preview/ and production/ folders
under the main project directory
- Simplified ports: Removed nginx routing, now expose productionPort directly
on localhost:8000-9999 range (deterministic based on project ID)
- Production builds now use preview/src/ dist/ and deploy to production/{hash}/
- All deployment handlers updated to work without nginx (using docker run directly)
- Rollback now rebuilds container rather than changing nginx routing
Benefits:
- Delete project = delete everything (no separate production/ folder needed)
- No nginx requirement (simpler deployment)
- Consistent production URLs (localhost:{productionPort} always the same for a project)
- All versions tracked in production/{hash}/ folders with easy rollback
Affected files:
- Schema: Made productionPort NOT NULL with default 8001
- Paths: Added getProjectPreviewPath, getProjectProductionPath
- Port allocation: Added allocateProjectProductionPort (8000-9999 range)
- Setup: Creates preview/ and production/ subdirectories
- Handlers: Updated productionBuild, productionStart, productionStop
- Actions: Updated getProductionStatus, getProductionHistory, rollback
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR refactors production deployment from nginx-based routing to Docker containers, introduces per-project production port allocation, reorganizes project file structure into separate preview and production directories, and enhances bootstrap/startup workflows with new scripts and log streaming capabilities. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ProjectCreate as Project<br/>Create
participant PortAlloc as Port<br/>Allocator
participant FileSetup as File<br/>System Setup
participant Database as Database
participant ProjectQueue as Queue<br/>(Production Build)
Client->>ProjectCreate: Create project
ProjectCreate->>PortAlloc: Allocate port<br/>(8000-9999)
PortAlloc->>PortAlloc: Compute hash,<br/>check availability
PortAlloc-->>ProjectCreate: productionPort
ProjectCreate->>FileSetup: Setup preview & production<br/>directories
FileSetup->>FileSetup: Create preview/<br/>production paths
FileSetup->>FileSetup: Write .env with<br/>productionPort
FileSetup-->>ProjectCreate: { projectPath,<br/>productionPort }
ProjectCreate->>Database: Create project record<br/>with productionPort
Database-->>ProjectCreate: Success
ProjectCreate->>ProjectQueue: Enqueue<br/>production:build
ProjectQueue-->>Client: Project created
Note over ProjectQueue: Later: Build & Start<br/>Production Container
ProjectQueue->>ProjectQueue: Build Docker image<br/>doce-prod-<id>-<hash>
ProjectQueue->>ProjectQueue: Run container:<br/>productionPort→3000
ProjectQueue->>ProjectQueue: Update symlink<br/>to current hash
ProjectQueue-->>Client: Production running
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
- Move --exclude flags before the source directory (.) - tar requires options to precede positional arguments - This fixes 'Exiting with failure status due to previous errors' in PR deployments
|
❌ Preview deployment failed. |
|
ProviderModelNotFoundError |
|
❌ Preview deployment failed. |
|
❌ Preview deployment failed. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug Review Comments
Bug: Wrong path in getProductionCurrentSymlink()
File: src/server/projects/paths.ts:102
The function getProductionCurrentSymlink() uses getProductionsPath() which is deprecated and returns the wrong path structure.
Issue
- Returns:
data/productions/{projectId}/current - Should return:
data/projects/{projectId}/production/current
This breaks the new encapsulated project structure described in the PR.
Affected Code
The function at line 102:
return path.join(getProductionsPath(), projectId, "current");Suggested Fix
export function getProductionCurrentSymlink(projectId: string): string {
return path.join(getProjectProductionPath(projectId), "current");
}This bug affects symlink operations in:
- cleanup.ts:71, 144
- productionStart.ts:134
- actions/projects.ts:472
There was a problem hiding this comment.
Style Issue: Unnecessary type assertions
File: src/server/queue/handlers/productionStart.ts:126-130, 212-215
The code uses awkward as unknown as type assertions to satisfy the typing for logger calls.
Line 126-130
logger.info(
{
projectId: project.id,
containerName,
productionPort,
} as unknown as typeof runResult & {
containerName: string;
productionPort: number;
projectId: string;
},
"Production container started",
);The assertion as unknown as typeof runResult & {...} is confusing and unnecessary.
Line 212-215
logger.debug(
{ projectId, containerName } as unknown as {
projectId: string;
containerName: string;
},
"Stopped and removed production container",
);Same issue - the type assertion as unknown as { ... } is unnecessary.
Suggestion
Remove the type assertions and create simple, readable objects:
logger.info({
projectId: project.id,
containerName,
productionPort,
}, "Production container started");and
logger.debug({
projectId,
containerName,
}, "Stopped and removed production container");While the current code works by satisfying the logger type constraints with the unnecessary assertions, it reduces code readability and follows an anti-pattern. The simpler version without assertions is cleaner and more maintainable.
There was a problem hiding this comment.
Critical Bug: Missing Dockerfile.prod in production build
Files:
- src/server/queue/handlers/productionBuild.ts:79-90
- src/server/queue/handlers/productionStart.ts:65-69
The Issue
The production build handler copies files from preview/ to production/{hash}/ but fails to copy Dockerfile.prod.
In productionBuild.ts (lines 79-90), only these files are copied:
- package.json
- pnpm-lock.yaml
- dist/
But then in productionStart.ts (line 65-69), it tries to build a Docker image:
const buildResult = await spawnCommand(
"docker",
["build", "-t", imageName, "-f", "Dockerfile.prod", "."],
{ cwd: productionPath },
);This will FAIL because Dockerfile.prod does not exist in production/{hash}/.
Impact
All production deployments will fail with: "docker build failed: unable to prepare context: stat .../Dockerfile.prod: no such file or directory"
Suggested Fix
Add in productionBuild.ts after line 90:
// Copy Dockerfile.prod for production builds
await fs.copyFile(
path.join(previewPath, "Dockerfile.prod"),
path.join(productionPath, "Dockerfile.prod"),
);
// Copy docker-entrypoint.prod.sh if it exists
try {
await fs.copyFile(
path.join(previewPath, "docker-entrypoint.prod.sh"),
path.join(productionPath, "docker-entrypoint.prod.sh"),
);
} catch (error) {
// Entrypoint script missing, but container might work without it
}Also consider copying any production-only configuration files that might be needed.
Note
Since production deployment now uses direct docker build/run instead of docker-compose, the docker-compose.yml file created by createProductionComposeFile() is now unused. Either:
- Remove
createProductionComposeFile()call, or - Keep it as documentation/artifact only
But definitely copy Dockerfile.prod!
There was a problem hiding this comment.
Code Cleanup: Remove deprecated functions
File: src/server/ports/allocate.ts:84-124
The allocateProjectBasePort() function is no longer used anywhere in the codebase.
Background
According to the PR description, the old port system was:
- Before: 4 ports per project (dev, opencode, base port, version port)
- After: 3 ports per project (dev, opencode, productionPort)
The function allocateProjectBasePort() was part of the old 4-port system and is now superseded by allocateProjectProductionPort().
Verification
grep -r "allocateProjectBasePort" --include="*.ts" src/Result: Only the function declaration itself, no callers.
Suggestion
Remove the unused function:
allocateProjectBasePort()(lines 84-124)- Related
registerBasePort()(lines 174-177) if also unused
This keeps the codebase clean and removes confusing, outdated APIs.
Note
Similarly, deriveVersionPort() and registerVersionPort() functions might be defunct if the old version port system was completely removed. Verify if these are still used before removing.
|
❌ Preview deployment failed. |
|
🚀 Preview deployed! |
This commit fixes several critical bugs identified in the deployment refactor:
Critical Fixes:
- Fix getProductionCurrentSymlink() to use correct path (data/projects/{id}/production/current)
- Fix symlink target to use full path instead of just hash name
- Add missing file copies in productionBuild (Dockerfile.prod, config files, src/)
- Fix production container cleanup in projectDelete handler
- Fix rollback action to use correct symlink paths
Improvements:
- Remove unused docker-compose.yml creation from productionBuild (using docker run)
- Add Docker image cleanup in productionStop and projectDelete
- Fix race condition in temp symlink names (add Math.random())
These fixes ensure deployments work correctly with the new encapsulated structure.
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
| hash: payload.productionHash.slice(0, 8), | ||
| containerName, | ||
| productionPort, | ||
| } as unknown as typeof runResult & { |
There was a problem hiding this comment.
"## Style Issue: Unnecessary type assertions in logger calls
File: src/server/queue/handlers/productionStart.ts:126-130
The code uses awkward `as unknown as` type assertions that reduce readability and maintainability.
Issue
The assertion `as unknown as typeof runResult & {...}` is confusing and unnecessary for simple logger calls.
Suggested Fix
Remove the type assertion and create a simple, readable object:
```typescript
logger.info({
projectId: project.id,
containerName,
productionPort,
}, "Production container started");
```"
| await spawnCommand("docker", ["rm", containerName]); | ||
|
|
||
| logger.debug( | ||
| { projectId, containerName } as unknown as { |
There was a problem hiding this comment.
"## Style Issue: Unnecessary type assertions in stopProductionContainer
File: src/server/queue/handlers/productionStart.ts:214-220
The `stopProductionContainer` function also uses awkward `as unknown as` type assertions.
Issue at Line 215
```typescript
logger.debug(
{ projectId, containerName } as unknown as {
projectId: string;
containerName: string;
},
"Stopped and removed production container",
);
```
Suggested Fix
```typescript
logger.debug({
projectId,
containerName,
}, "Stopped and removed production container");
```"
|
LGTM The PR implements the Sprint 1 deployment refactor well. The code follows the AGENTS.md guidelines. Issues found and addressed:
Code quality:
Suggested cleanup (optional): The unused |
|
Done. I've reviewed PR #40 against the AGENTS.md style guide and posted comments for the violations found. Here's the summary: LGTM - The code follows the guidelines well, with one style issue identified. Style Issue Found:
Previous Issues Already Fixed:
The only other note is an optional cleanup suggestion - the unused All code changes adhere to clean code principles, use |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
|
❌ Preview deployment failed. |
|
🚀 Preview deployed! |
|
🚀 Preview deployed! |
1 participant
Overview
This Sprint 1 refactor encapsulates all project files into a single folder structure, making deployment management simpler and more intuitive.
Key Changes
New Project Structure
Simplified Port System
Previously: 4 ports per project (dev, opencode, base port, version port)
Now: 3 ports per project (dev, opencode, productionPort)
URL pattern:
localhost:{productionPort}- always consistent for a project!Removed nginx Dependency
basePort(3000-3999) →versionPort(5000-5999)Benefits
✅ Complete encapsulation - Delete project = delete everything (dev + production)
✅ Simpler ports - Predictable production URLs (8000-9999 range)
✅ No nginx - No config files, no root access needed
✅ Easy rollback - Keep last N versions in production/{hash}/
✅ Atomic deployments - Switch symlink changes active version instantly
Implementation Details
Updated Files
productionPortNOT NULL (default: 8001)getProjectPreviewPath(),getProjectProductionPath()allocateProjectProductionPort()for deterministic port assignmentpreview/andproduction/subdirectories during project creationpnpm buildinpreview/src/, copies toproduction/{hash}/dist/Breaking Changes
productionPort(default: 8001)Testing
Create a project, deploy, and verify:
data/projects/{projectId}/preview/andproduction/localhost:xxxx(8000-9999 range)production/{hash}/foldersSummary by CodeRabbit
Release Notes
New Features
Chores