Skip to content

feat:add Docker support and auto ngrok url replace on restart#18

Open
kaihere14 wants to merge 4 commits into
raroque:mainfrom
kaihere14:docker-setup
Open

feat:add Docker support and auto ngrok url replace on restart#18
kaihere14 wants to merge 4 commits into
raroque:mainfrom
kaihere14:docker-setup

Conversation

@kaihere14
Copy link
Copy Markdown

This pull request introduces Docker and Docker Compose support for local development, improves the developer experience with better process management, and streamlines the skill descriptions for the Convex agent skills. The most important changes are grouped below:

Dockerization and Local Development:

  • Added a Dockerfile to build and run the project in a containerized environment, including non-root user setup, dependencies, and ports exposure.
  • Introduced a docker-compose.yml file to orchestrate the local development environment, including volume mounts, environment variables, health checks, and persistent data directories.
  • Added a .dockerignore file to exclude unnecessary files and directories from Docker build context, improving build performance and security.
  • Updated Vite dev server configuration to listen on all interfaces (host: "0.0.0.0") for container compatibility.

Process Management and Developer Experience:

  • Improved scripts/dev.mjs to distinguish between critical and non-critical child processes, ensuring only essential services trigger a shutdown on failure, and added better handling and messaging for ngrok process exits. [1] [2] [3]

Convex Agent Skills Documentation:

  • Shortened and clarified the descriptions in all Convex agent skill .md files to be more concise and focused, making it easier to understand when to use each skill. [1] [2] [3] [4] [5] [6]
  • Updated skill hashes in skills-lock.json to reflect the documentation changes.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

This PR adds Docker/Docker Compose support for local development, makes ngrok non-critical so its failure no longer kills the whole dev session, and trims Convex skill descriptions. The process-management improvement in dev.mjs is correct, but two previously-flagged docker-compose.yml issues (UID 1001 bind-mount ownership on Linux, restart: unless-stopped looping on a fresh clone) are still present in the files.

Confidence Score: 4/5

Safe to merge for macOS users; Linux developers will hit the UID/bind-mount write failure and the fresh-clone restart loop documented in previous review threads that remain unresolved.

Two P1-level issues flagged in prior review rounds (UID 1001 ownership mismatch on Linux, restart: unless-stopped on first-run failure) are still present in the submitted files. New findings are all P2. Per scoring guidance, an unresolved P1 caps confidence at 4/5.

docker-compose.yml — UID mismatch and restart policy; scripts/dev.mjs — clean ngrok exit not surfaced to the user.

Important Files Changed

Filename Overview
scripts/dev.mjs Separates ngrok from critical children so its exit no longer tears down the whole dev environment; two P2 issues: clean ngrok exit (code 0) goes unannounced, and the criticalChildren/children initialization is redundant.
Dockerfile Adds containerized build with non-root user, ngrok, and Claude Code; the ngrok APT repo is pinned to buster on a bookworm base (flagged in previous review).
docker-compose.yml Orchestrates local dev with volume mounts and healthcheck; UID 1001 / bind-mount ownership mismatch on Linux and restart-loop on fresh clone remain unresolved from previous review; ngrok inspector port 4040 not exposed (P2).
.dockerignore Correctly excludes node_modules, env files, build artifacts, and local data directories from the Docker build context.
debug/vite.config.ts Adds host: true so the Vite dev server binds to all interfaces (0.0.0.0), required for Docker container compatibility.
skills-lock.json Hash updates reflect the shortened descriptions in the six Convex SKILL.md files; all six entries updated consistently.
.agents/skills/convex/SKILL.md Description shortened to a concise routing line; semantics preserved.

Sequence Diagram

sequenceDiagram
    participant D as Docker / Host
    participant M as dev.mjs (orchestrator)
    participant S as server (tsx watch)
    participant C as convex dev
    participant V as vite (debug)
    participant N as ngrok

    D->>M: npm run dev
    M->>S: spawn (critical)
    M->>C: spawn (critical)
    M->>V: spawn (critical)
    M->>N: spawn (non-critical)

    par Core services ready
        S-->>M: ready (listening on :3456)
        C-->>M: ready (Convex functions ready)
        V-->>M: ready (Local: http...)
    and Tunnel ready
        N-->>M: ngrok URL via :4040 API
    end

    M->>M: showBanner(ngrokUrl)

    alt Critical child exits non-zero
        S-->>M: exit(code != 0)
        M->>S: shutdown all children
        M->>C: kill
        M->>V: kill
        M->>N: kill
    else ngrok exits (any code)
        N-->>M: exit(code)
        M->>M: log warning, keep running
        note over S,V: Local dev continues uninterrupted
    end
Loading

Reviews (2): Last reviewed commit: "refactor: set Vite dev server host to tr..." | Re-trigger Greptile

Comment thread docker-compose.yml
- "3456:3456"
- "5173:5173"
volumes:
- .:/app
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Bind-mount ownership mismatch breaks writes on Linux hosts

The .:/app bind mount exposes all source files owned by the host user (e.g. UID 1000). The container runs as boop (UID 1001), so any write attempted during dev — Vite's .vite cache, Convex's convex/_generated/ type files, *.tsbuildinfo — will fail with EACCES on Linux. macOS users won't hit this because Docker Desktop virtualises the FS, but CI and Linux developers will.

The common fix is to propagate the caller's UID/GID into the compose service:

    user: "${UID:-1001}:${GID:-1001}"

Or set UID and GID explicitly in a .env (beside docker-compose.yml) and document the requirement. The useradd -u 1001 in the Dockerfile would also need to match whatever UID is chosen, or the container can drop the fixed UID and rely entirely on the compose override.

Comment thread Dockerfile
RUN apt-get update \
&& apt-get install -y --no-install-recommends ca-certificates curl git gnupg \
&& curl -fsSL https://ngrok-agent.s3.amazonaws.com/ngrok.asc | tee /etc/apt/trusted.gpg.d/ngrok.asc >/dev/null \
&& echo "deb https://ngrok-agent.s3.amazonaws.com buster main" | tee /etc/apt/sources.list.d/ngrok.list \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 ngrok APT repo uses buster dist on a bookworm base image

node:22-bookworm-slim is Debian 12. Pinning the ngrok APT source to buster (Debian 10) is what ngrok's own docs prescribe (they only publish one dist), but it can cause apt-get update to emit warnings on stricter APT configurations and may silently break if ngrok ever publishes a bookworm entry that conflicts. Adding a comment explaining why buster is intentional would prevent future confusion.

Comment thread docker-compose.yml
image: boop-agent:local
command: npm run dev
init: true
restart: unless-stopped
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 restart: unless-stopped loops on first-time setup failure

scripts/dev.mjs exits with code 1 when convex/_generated/api.js doesn't exist yet (standard on a fresh clone). With restart: unless-stopped, the container will restart endlessly until the types are generated, which can't happen inside this same container without npx convex dev --once being run first. Consider using restart: "no" for the initial experience, or documenting a docker-compose run boop npx convex dev --once prerequisite step.

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