fix(docker): copy scripts/ into appservice builder before pnpm install #1452

Merged
fred merged 1 commits from fix/1451-appservice-dockerfile-scripts into next 2026-08-27 11:28:48 +00:00
Collaborator

Fixes #1451.

Root cause: the root package.json prepare script runs node scripts/install-hooks.mjs at install time. docker/appservice.Dockerfile ran pnpm install --frozen-lockfile in the builder stage without copying scripts/, so the prepare hook dies MODULE_NOT_FOUND and the image build fails.

gateway.Dockerfile and web.Dockerfile already COPY scripts/ before install (added in 91e692e3, which missed appservice). This applies the identical one-line fix, same comment.

Why it surfaced now: the .woodpecker/publish.yml image_build_when anchor change made build-appservice run on next pushes, and trunk pipeline 2883 (next head 01904b8f) was the first trunk push pipeline to reach a terminal state — every earlier one was killed. Step log for build-appservice on 2883 shows the MODULE_NOT_FOUND from the prepare hook. Pre-existing defect, not introduced by the merges 2883 covered (docs-only).

Verification: diff is byte-parallel to gateway.Dockerfile lines 13-15; PR CI plus the post-merge trunk pipeline's build-appservice step is the real gate.

Fixes #1451. Root cause: the root package.json prepare script runs `node scripts/install-hooks.mjs` at install time. `docker/appservice.Dockerfile` ran `pnpm install --frozen-lockfile` in the builder stage without copying `scripts/`, so the prepare hook dies MODULE_NOT_FOUND and the image build fails. `gateway.Dockerfile` and `web.Dockerfile` already COPY `scripts/` before install (added in 91e692e3, which missed appservice). This applies the identical one-line fix, same comment. Why it surfaced now: the `.woodpecker/publish.yml` image_build_when anchor change made build-appservice run on next pushes, and trunk pipeline 2883 (next head 01904b8f) was the first trunk push pipeline to reach a terminal state — every earlier one was killed. Step log for build-appservice on 2883 shows the MODULE_NOT_FOUND from the prepare hook. Pre-existing defect, not introduced by the merges 2883 covered (docs-only). Verification: diff is byte-parallel to gateway.Dockerfile lines 13-15; PR CI plus the post-merge trunk pipeline's build-appservice step is the real gate.
fred added 1 commit 2026-08-27 11:11:07 +00:00
The root package.json prepare script runs node scripts/install-hooks.mjs
at install time. gateway.Dockerfile and web.Dockerfile COPY scripts/
before pnpm install --frozen-lockfile; appservice.Dockerfile was the one
missed by 91e692e3, so build-appservice fails MODULE_NOT_FOUND on next
pushes (first terminal trunk run: pipeline 2883). Same one-line COPY,
same comment, matching gateway line 14.
rev-code-01 approved these changes 2026-08-27 11:13:15 +00:00
rev-code-01 left a comment
Member

APPROVED — rev-code-01, pinned to head 837d2ac6e1998991e524524eb238df1dee77fffa (re-verified unmoved immediately before posting).

Scope: one file docker/appservice.Dockerfile (+2: comment + COPY scripts/ ./scripts/), fixes #1451. Note: this delta is byte-identical to PR #1447's fix (same pre/post blob hashes a92e2522→cf6a4cd8), re-commissioned against the moved trunk (base 01904b8f); my #1447 review (id 331) ran the direct red controls — old-builder state reproduces the exact MODULE_NOT_FOUND rc=1; fixed state with git absent gives git not found; skipping rc=0 — and they carry over verbatim. Freshly measured at THIS head:

  1. Parity: the added comment + COPY are textually identical to gateway.Dockerfile:13-14 and web.Dockerfile:11-12, placed at the same position (after manifest copies, immediately before RUN pnpm install --frozen-lockfile).
  2. Layer-cache sanity: scripts/ is its own COPY layer before install — a scripts/ change now busts the install cache, the same tradeoff both sibling Dockerfiles already accept (rare changes); the full-context COPY . . remains after install, so the existing ordering is preserved.
  3. Nothing smuggled: full-range name-status is exactly M docker/appservice.Dockerfile; runner stage unchanged (3 COPY --from=builder paths — scripts/ cannot reach the runner image); no .dockerignore exists to interfere.
  4. Failure evidence measured: trunk pipeline 2883 (next head 01904b8f) shows build-appservice → failure (exit 1) while build-gateway and build-web succeed in the same pipeline — the sibling builds having the COPY isolates the defect to the appservice Dockerfile exactly as the issue states; 2883 was the first terminal trunk run because earlier push pipelines were killed.
  5. Gates: format:check rc=0, typecheck rc=0 (45/45), lint rc=0 (25/25).

CI note: 2884 running on this head at review time. Merge waits on 2884 terminal green; its build-appservice step (and the post-merge trunk pipeline) is the fix's verification.

**APPROVED — rev-code-01, pinned to head `837d2ac6e1998991e524524eb238df1dee77fffa`** (re-verified unmoved immediately before posting). Scope: one file `docker/appservice.Dockerfile` (+2: comment + `COPY scripts/ ./scripts/`), fixes #1451. Note: this delta is byte-identical to PR #1447's fix (same pre/post blob hashes `a92e2522→cf6a4cd8`), re-commissioned against the moved trunk (base 01904b8f); my #1447 review (id 331) ran the direct red controls — old-builder state reproduces the exact MODULE_NOT_FOUND rc=1; fixed state with git absent gives `git not found; skipping` rc=0 — and they carry over verbatim. Freshly measured at THIS head: 1. **Parity:** the added comment + COPY are textually identical to `gateway.Dockerfile:13-14` and `web.Dockerfile:11-12`, placed at the same position (after manifest copies, immediately before `RUN pnpm install --frozen-lockfile`). 2. **Layer-cache sanity:** `scripts/` is its own COPY layer before install — a scripts/ change now busts the install cache, the same tradeoff both sibling Dockerfiles already accept (rare changes); the full-context `COPY . .` remains after install, so the existing ordering is preserved. 3. **Nothing smuggled:** full-range name-status is exactly `M docker/appservice.Dockerfile`; runner stage unchanged (3 COPY --from=builder paths — `scripts/` cannot reach the runner image); no `.dockerignore` exists to interfere. 4. **Failure evidence measured:** trunk pipeline 2883 (next head 01904b8f) shows `build-appservice → failure (exit 1)` while `build-gateway` and `build-web` succeed in the same pipeline — the sibling builds having the COPY isolates the defect to the appservice Dockerfile exactly as the issue states; 2883 was the first terminal trunk run because earlier push pipelines were killed. 5. **Gates:** `format:check` rc=0, `typecheck` rc=0 (45/45), `lint` rc=0 (25/25). CI note: 2884 running on this head at review time. Merge waits on 2884 terminal green; its build-appservice step (and the post-merge trunk pipeline) is the fix's verification.
fred merged commit bf8bc2128d into next 2026-08-27 11:28:48 +00:00
Sign in to join this conversation.