Skip to content

fix: copy packages/remote-web in Dockerfile (pnpm workspace glob)#3337

Open
Whaleylaw wants to merge 1 commit into
BloopAI:mainfrom
Whaleylaw:claude/fix-dockerfile-remote-web-84jxK
Open

fix: copy packages/remote-web in Dockerfile (pnpm workspace glob)#3337
Whaleylaw wants to merge 1 commit into
BloopAI:mainfrom
Whaleylaw:claude/fix-dockerfile-remote-web-84jxK

Conversation

@Whaleylaw

@Whaleylaw Whaleylaw commented Apr 8, 2026

Copy link
Copy Markdown

Note

Low Risk
Low risk change limited to the Docker build context; main impact is build behavior (pnpm install/cache layers) rather than runtime logic.

Overview
Fixes the frontend Docker build by copying packages/remote-web/package.json before pnpm install, and then copying the full packages/remote-web/ directory into the image.

This ensures pnpm workspace globs/dependency resolution include remote-web and prevents Docker layer caching/install from failing due to a missing workspace package.

Reviewed by Cursor Bugbot for commit 285e1fe. Bugbot is set up for automated code reviews on this repo. Configure here.

@Whaleylaw

Copy link
Copy Markdown
Author

Test

@Whaleylaw

Copy link
Copy Markdown
Author

Fixes the frontend Docker build by copying packages/remote-web/package.json before pnpm install, and then copying the full packages/remote-web/ directory into the image.

@Juanlucasbg

Copy link
Copy Markdown

Aggressive review summary — PR #3337

Ran the four-reviewer battery (structural, security, adversarial, conventions) plus local Phase B checks. Verdict: clean — recommend merge.

Why this fix is necessary

pnpm-workspace.yaml:2 declares packages/* as the workspace glob. pnpm-lock.yaml has an importers: entry for packages/remote-web: — the lockfile expects that workspace member to be present on disk. Without packages/remote-web/package.json in the fe-builder stage, pnpm install --frozen-lockfile (Dockerfile:25) fails with ERR_PNPM_OUTDATED_LOCKFILE. The PR adds the missing copy and matches the existing pattern for local-web/ui/web-core.

Findings

Structural — 1 favorable HIGH (fix is necessary), 2 MED, 1 LOW, 1 NIT.
Security — 0 HIGH, 0 MED, 0 LOW. .dockerignore already excludes node_modules, .env*, .git. packages/remote-web/package.json declares no prepare/postinstall/preinstall scripts. Runtime image (Dockerfile:137) only copies the server binary; remote-web source never reaches the runtime stage.
Adversarial — 0 HIGH against the PR. One MED process gap noted: no CI workflow exercises the Dockerfile (grep -l Dockerfile .github/workflows/*.yml returns nothing), so the latent break only surfaced at deploy time. Not this PR's fault.
Conventions — LGTM. find packages -maxdepth 2 -name package.json confirms exactly four workspace members (local-web, ui, web-core, remote-web); post-PR all four are copied. packages/public/ correctly stays source-only (asset bundle, no package.json).

Non-blocking suggestions (consider in a follow-up)

  • Dockerfile:32 — the source copy of packages/remote-web/ is dead bloat. The fe-builder only runs pnpm -C packages/local-web build (Dockerfile:33). The minimum fix is the package.json copy on Dockerfile:23. Dropping the source copy avoids busting the fe-builder cache when remote-web source changes (unrelated rebuilds of local-web).
  • Sort the package.json copy block alphabetically: local-web, remote-web, ui, web-core (currently local-web, ui, web-core, remote-web).
  • Wire a docker build --target fe-builder . smoke job into CI to catch this kind of drift earlier.

Phase B verification

  • pnpm i --frozen-lockfile clean (exit 0) on full repo.
  • pnpm run check clean (exit 0).
  • pnpm run lint failed locally with an SSH-auth error against the private vibe-kanban-private repo (crates/remotebilling dep). Same failure reproduces on main — environmental, not introduced by this PR. CI (which has the SSH key) shows Cursor Bugbot SUCCESS.

— Reviewed by automated four-tool battery (structural / security / adversarial / conventions).

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.

3 participants