Feat - Made the app more self hostable#1
Conversation
- Created .dockerignore to exclude unnecessary files from Docker context. - Updated docker-compose.yml to include Redis service and improved PostgreSQL configuration with environment variables. - Added migration script (scripts/migrate.ts) for database migrations using drizzle-orm. - Modified Dockerfile to streamline dependency installation and build process. - Updated package.json to reflect changes in dependencies, replacing 'pg' with 'postgres' and updating 'drizzle-orm'. - Refactored database connection logic in src/db/index.ts to use postgres-js. - Added health check endpoint in src/app/api/health/route.ts for application status monitoring. - Updated start script to use bun for running migrations and starting the application. - Adjusted .env.example to include new environment variables for Redis and other configurations.
- Updated .env.example to include required variables for Docker deployment and local development. - Created DEPLOY.md for a comprehensive self-hosting guide, detailing setup and configuration steps. - Modified Dockerfile to use a smaller Alpine base image and improved health check command. - Updated README.md to reflect new self-hosting capabilities and quick start instructions.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces Docker-based deployment and local dev configuration, adds Redis and a migration workflow, switches DB client from node-postgres/pg to postgres-js, adds a health API route, restructures environment variables, updates documentation (README, DEPLOY), and adjusts build/runtime via Dockerfile and docker-compose. Removes dotenv auto-loading and adds a TypeScript migration script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as docker-compose
participant PG as postgres
participant RD as redis
participant MG as migrate (bun ts-node)
participant APP as app (Next.js/Bun)
Note over Dev: docker-compose up
Dev->>PG: start container
Dev->>RD: start container
PG-->>Dev: health=healthy
RD-->>Dev: health=healthy
Dev->>MG: run scripts/migrate.ts
MG->>PG: connect via DATABASE_URL
MG->>PG: apply drizzle migrations
PG-->>MG: migrations done
MG-->>Dev: exit 0
Dev->>APP: start container
APP->>APP: bun server.js
APP-->>Dev: healthcheck /api/health
sequenceDiagram
autonumber
participant Env as process.env
participant Script as scripts/migrate.ts
participant Client as postgres()
participant Drizzle as drizzle(db)
participant FS as drizzle/migrationsFolder
participant DB as PostgreSQL
Script->>Env: read DATABASE_URL
alt DATABASE_URL missing
Script-->>Script: throw Error and exit(1)
else present
Script->>Client: create single-connection client
Script->>Drizzle: init with client
Drizzle->>FS: load migrations
Drizzle->>DB: run migrations
DB-->>Drizzle: success
Script-->>Script: log success, exit(0)
end
Script->>Client: close with timeout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR makes Command Ops fully self-hostable by introducing comprehensive Docker deployment capabilities, modernizing the database stack, and providing extensive documentation for self-hosting.
- Replaced
pgwithpostgreslibrary anddrizzle-orm/node-postgreswithdrizzle-orm/postgres-jsfor better compatibility - Created production-ready Docker setup with multi-stage builds, dedicated migration service, and health checks
- Added comprehensive self-hosting documentation and improved project README
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/db/index.ts | Updated database client from pg to postgres library |
| src/app/api/health/route.ts | Added health check endpoint for Docker monitoring |
| scripts/start.sh | Changed migration command from npx to bunx |
| scripts/migrate.ts | New standalone migration script for Docker environments |
| package.json | Replaced pg with postgres dependency |
| drizzle.config.ts | Removed dotenv import for cleaner environment handling |
| docker-compose.yml | Complete overhaul with Redis, migration service, and production configuration |
| README.md | Enhanced with self-hosting focus and deployment guide links |
| Dockerfile | Production-ready multi-stage build with security improvements |
| DEPLOY.md | Comprehensive self-hosting guide with OAuth setup instructions |
| .env.example | Restructured with clear Docker vs local development sections |
| .dockerignore | Added to optimize Docker build context |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
|
|
There was a problem hiding this comment.
Remove unnecessary blank line at the end of the file.
Docstrings generation was requested by @Yavnik. * #1 (comment) The following files were modified: * `src/app/api/health/route.ts` * `src/db/index.ts`
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
76-76: Remove unused@types/pgfrom devDependencies.Since the
pgpackage was removed and replaced withpostgres, the corresponding TypeScript types are no longer needed. Thepostgrespackage includes its own TypeScript definitions.Apply this diff to remove the unused dependency:
- "@types/pg": "^8.15.5",
🧹 Nitpick comments (4)
src/app/api/health/route.ts (1)
1-3: Consider enhancing the health check with dependency verification.The basic health check is functional, but for production deployments it's valuable to verify critical dependencies like database and Redis connectivity. This helps load balancers and orchestrators make informed routing decisions.
If you'd like, I can generate an enhanced health check implementation that includes database and Redis connectivity checks with appropriate error handling. Would you like me to do that?
DEPLOY.md (1)
79-96: Consider adding backup and restore procedures.For self-hosted deployments, it's valuable to document how to backup and restore data. This would include PostgreSQL volume backup strategies and Redis data considerations.
Would you like me to draft a "Backup & Restore" section for the deployment guide?
scripts/migrate.ts (1)
21-23: Consider increasing the client.end() timeout.The 1-second timeout in
client.end({ timeout: 1 })may be insufficient if there are in-flight queries or cleanup operations. A timeout of 5-10 seconds is more common for graceful shutdown.Apply this diff to increase the timeout:
- await client.end({ timeout: 1 }); + await client.end({ timeout: 5 });src/db/index.ts (1)
1-11: Migration to postgres-js looks good; consider exporting the client.The migration from
drizzle-orm/node-postgrestodrizzle-orm/postgres-jsis correctly implemented. Thepostgrespackage (v3.4.7) uses lazy connection pooling and is well-suited for this use case.Consider these enhancements:
- Export the client for advanced use cases (e.g., raw queries, connection cleanup in tests, or custom type handling):
const client = postgres(requireDatabaseUrl()); export const db = drizzle(client); +export { client };
- Add connection options for better control in production:
-const client = postgres(requireDatabaseUrl()); +const client = postgres(requireDatabaseUrl(), { + max: 10, // max connections + idle_timeout: 20, // idle connection timeout in seconds + prepare: false, // disable prepared statements if using PgBouncer in transaction mode +});Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.dockerignore(1 hunks).env.example(1 hunks)DEPLOY.md(1 hunks)Dockerfile(4 hunks)README.md(2 hunks)docker-compose.yml(1 hunks)drizzle.config.ts(0 hunks)package.json(2 hunks)scripts/migrate.ts(1 hunks)scripts/start.sh(1 hunks)src/app/api/health/route.ts(1 hunks)src/db/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- drizzle.config.ts
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/migrate.ts (1)
src/db/index.ts (1)
db(11-11)
🪛 Checkov (3.2.334)
Dockerfile
[medium] 27-28: Basic Auth Credentials
(CKV_SECRET_4)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 44-44: [UnorderedKey] The NEXT_PUBLIC_POSTHOG_HOST key should go before the NEXT_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The REDIS_DB key should go before the REDIS_HOST key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
README.md
132-132: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (16)
package.json (2)
4-22: LGTM! Metadata additions improve package discoverability.The added project metadata (description, author, license, repository, etc.) is well-structured and aligns with npm best practices for open-source projects.
54-54: LGTM! Dependency updates align with the postgres-js migration.The upgrade to
drizzle-orm@^0.44.6and the replacement ofpgwithpostgres@^3.4.7are correct and align with the migration todrizzle-orm/postgres-jsthroughout the codebase.Based on learnings, both versions are current and compatible.
Also applies to: 61-61
.dockerignore (2)
1-84: LGTM! Comprehensive .dockerignore patterns.The ignore patterns appropriately exclude build artifacts, dependencies, environment files, IDE configs, and development files from the Docker build context, which reduces build time and image size.
34-36: Exclude Docker files from build context: No COPY references to Dockerfile, docker-compose, or .dockerignore found in your Dockerfiles; excluding them is safe.scripts/start.sh (1)
5-8: LGTM! Startup script correctly uses Bun tooling.The migration from
npxtobunxaligns with the project's use of Bun as the runtime, and theexeccommand correctly replaces the shell process with the application process for proper signal handling in containerized environments.DEPLOY.md (1)
1-114: LGTM! Comprehensive self-hosting guide.The deployment documentation is well-structured, covering all essential aspects: quick start, environment configuration, OAuth setup, maintenance commands, and troubleshooting. The clear separation between required and optional configuration is particularly helpful for users.
scripts/migrate.ts (1)
11-12: LGTM! Single connection is appropriate for migrations.Using
max: 1for the connection pool is correct for a one-off migration script, as it prevents connection overhead and ensures sequential migration execution.README.md (2)
1-132: LGTM! README effectively communicates the self-hosting focus.The rewritten README clearly positions Command Ops as a self-hostable solution, with comprehensive sections covering quick start, deployment, local development, and contributing. The structure is logical and the content is informative.
132-132: Emphasis styling is acceptable here.The markdownlint warning about using emphasis instead of a heading on Line 132 is a false positive in this context. The line "Built with ❤️ for self-hosters and productivity enthusiasts" is intentionally styled as an emphasized footer rather than a heading, which is a common pattern in README files.
.env.example (2)
1-52: LGTM! Clear environment variable organization.The restructured
.env.examplewith distinct sections for Docker deployment, local development, OAuth, and analytics greatly improves clarity for both self-hosters and local developers. The inline comments effectively explain which variables apply to which scenarios.
44-44: Static analysis key ordering warnings are pedantic.The dotenv-linter warnings about key ordering (suggesting alphabetical sorting within sections) are stylistic preferences that don't affect functionality. The current grouping by related functionality (e.g., PostHog keys together, Redis keys together) is more maintainable than strict alphabetical ordering.
Also applies to: 52-52
Dockerfile (2)
27-33: Build-time dummy credentials are appropriate.The dummy credentials set here are only used during the Next.js build phase and won't be used at runtime. The static analysis warning (CKV_SECRET_4) is a false positive since these values:
- Are explicitly marked as dummy/placeholder values
- Never reach the runtime environment (overridden by docker-compose environment)
- Are necessary to satisfy Next.js build requirements when the app code references these env vars
55-61: Migration artifacts properly copied.The explicit copying of migration scripts and their dependencies (postgres, drizzle-orm) from the deps stage is necessary because Next.js standalone build excludes dev dependencies. This ensures the migration script can run in the runtime container.
docker-compose.yml (3)
19-29: Redis service properly configured with security and health checks.The Redis service configuration follows best practices:
- Password protection enabled via
--requirepass- Health check authenticates before testing connectivity
- Data persistence via volume
- Appropriate restart policy
31-44: Migration service correctly implements one-off execution.The migration service design is sound:
- Runs only after postgres is healthy
- Uses
restart: "no"to prevent re-running migrations- App service depends on
service_completed_successfullyto ensure migrations finish before starting
46-76: App service properly configured with comprehensive dependencies and environment.The app service configuration is well-designed:
- Waits for postgres health, redis health, and migration completion before starting
- Environment variables properly interpolated with sensible defaults for optional features (OAuth, analytics)
- Appropriate restart policy for long-running service
| # Health check using wget (already available in Alpine) | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=60s --retries=3 \ | ||
| CMD bun --version || exit 1 | ||
| CMD wget --no-verbose --tries=1 --spider http://localhost:3000/api/health || exit 1 |
There was a problem hiding this comment.
🧩 Analysis chain
Health check correctly implemented.
Using wget for the health check is appropriate for Alpine-based images, and the /api/health endpoint provides a lightweight verification that the app is responsive.
Verify that the health endpoint exists and returns a successful status:
🏁 Script executed:
#!/bin/bash
# Check if health endpoint exists
fd -e ts -e tsx "health" src/app/api/ --exec cat {}Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search entire repository for any occurrences of “health” in TS/JS files
rg -n 'health' -C2 -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 728
Add or correct the /api/health handler
No implementation for /api/health was found in the codebase; either create the API route (e.g. app/api/health/route.ts returning 200) or update the Docker HEALTHCHECK to use an existing endpoint.
🤖 Prompt for AI Agents
In Dockerfile around lines 72 to 74, the HEALTHCHECK targets /api/health which
has no handler in the codebase; either add a minimal API route at
app/api/health/route.ts that returns HTTP 200 (JSON or plain OK) for GET or
adjust the Dockerfile to point to an existing health endpoint. To add the route:
create app/api/health/route.ts exporting a GET handler that returns a 200
response quickly; if you prefer updating the Dockerfile, change the URL in the
HEALTHCHECK to the existing, implemented health/status endpoint and ensure path
and port match the running app.
There was a problem hiding this comment.
The health check route already exists in app/api/health/route.ts
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the health check route exists and show its content
fd -e ts "route.ts" src/app/api/health/ --exec cat {}
Length of output: 69
@Yavnik, you're absolutely right, and I apologize for the incorrect comment. The health check route does exist at src/app/api/health/route.ts with a proper implementation:
export async function GET() {
return Response.json({ ok: true });
}The health check in the Dockerfile is correctly configured to use this endpoint. Thank you for the correction!
This pull request introduces comprehensive improvements to the self-hosting and deployment experience for Command Ops, focusing on Docker-based deployment, environment configuration, and documentation. The changes streamline the Docker setup, add a dedicated migration process, update dependencies, and provide clear, user-friendly guides for both deployment and development.
Dockerization & Deployment Improvements
Dockerfileusing Bun with Alpine, optimized for smaller images and secure, non-root execution. The build process now uses dummy secrets to avoid leaking real credentials at build time, and includes only the necessary dependencies for runtime and migrations. [1] [2] [3] [4]docker-compose.ymlto add dedicated services for PostgreSQL, Redis, migrations, and the app itself. Environment variables are now securely injected, and service dependencies and health checks ensure reliable startup sequencing.Environment & Configuration
.env.exampleto clearly separate Docker and local development variables, document required and optional settings, and provide guidance for OAuth and analytics configuration.drizzle.config.tsand database code, as environment variables are now managed externally. [1] [2]Database & Migration Handling
pgtopostgresand fromdrizzle-orm/node-postgrestodrizzle-orm/postgres-jsfor improved compatibility and performance. [1] [2]scripts/migrate.ts) for running migrations in Docker and CI environments.scripts/start.sh) to use Bun for running migrations.Documentation & Developer Experience
DEPLOY.md) with step-by-step Docker instructions, OAuth setup, and troubleshooting tips.README.mdwith quick start, architecture, and contribution sections, and linked to the new deployment guide. [1] [2].dockerignorefile to ensure clean Docker builds by excluding unnecessary files and directories.Other Enhancements
src/app/api/health/route.ts) for Docker to verify the app is running.Most Important Changes:
1. Dockerization and Deployment
Dockerfileanddocker-compose.ymlfor robust, production-ready Docker deployment with dedicated services for the app, migrations, PostgreSQL, and Redis, and improved security and startup reliability. [1] [2] [3] [4] [5]2. Environment and Configuration
.env.exampleto clarify required/optional variables for Docker and local development, and improved documentation for OAuth and analytics setup.3. Database and Migrations
pgtopostgresand fromdrizzle-orm/node-postgrestodrizzle-orm/postgres-js, and introduced a dedicated migration script for Docker/CI. [1] [2] [3]4. Documentation
DEPLOY.md) and improvedREADME.mdwith clear quick start, architecture, and contribution instructions. [1] [2] [3]5. Build and Runtime Cleanliness
.dockerignoreto optimize Docker builds and prevent unnecessary files from being included in images.These changes make it much easier and safer to self-host Command Ops, provide a better developer experience, and lay the groundwork for future scalability and maintainability.
Summary by CodeRabbit
New Features
Documentation
Chores