feat(cli): scaffold Dockerfile and .dockerignore in wheels deploy init#2686
Conversation
Closes #2673. wheels deploy init now emits a starter Dockerfile and .dockerignore alongside config/deploy.yml and .kamal/secrets, so the init -> setup walkthrough no longer dead-ends at "no Dockerfile to build from". The Dockerfile mirrors the canonical pattern from the deployment docs: multi-stage (Node builder + lucee/lucee:7-tomcat10-jre21 runtime), EXPOSE 8080 to match the default proxy.app_port, /health HEALTHCHECK, and production env-var defaults. The .dockerignore keeps secrets (.kamal/secrets, .env*), test fixtures, and IDE noise out of the build context. --force now also gates an existing Dockerfile, with the same DeployMainCli.InitAlreadyExists abort as deploy.yml. An existing .dockerignore is preserved unless --force is passed.
|
Wheels Bot -- Reviewer B (round 1) Reviewer A's submission is a broken non-review: the body is the literal string REVIEW_BODY_UPDATE and there are no line comments. There is nothing to align with, so this round focuses on what A should have found in the diff. Sycophancy: None detected -- but only because the review has no content at all. False positives: None detected -- no claims were made. Missed issues A filed a COMMENTED review with body REVIEW_BODY_UPDATE and zero line comments. The entire diff was left unexamined. Issues A should have flagged:
Verdict alignment: A's state is COMMENTED with an empty body. The verdict is neither approve nor request-changes, which is inconsistent with a diff that has real concerns worth surfacing. A left the PR in an ambiguous state. Convergence: Not aligned. A's review provided no substantive content and cannot be the basis for a joint recommendation. A needs to re-review the diff and address the missed issues above -- specifically the RELOAD_PASSWORD default and the .dockerignore test gaps -- before convergence is possible. |
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR closes a real gap — wheels deploy init used to dead-end users who hadn't written a Dockerfile. The overall shape is solid: templates are idiomatic, the force-flag guard is correctly placed before any writes, and the new specs cover the main paths. Four issues are worth addressing before merge, ranging from a misleading success message to a test coverage gap and a subtle Docker trap.
Correctness
1. Success message claims .dockerignore was created even when it was preserved
cli/lucli/services/deploy/cli/DeployMainCli.cfc, line 369:
return "Created config/deploy.yml, .kamal/secrets, Dockerfile, and .dockerignore." & chr(10)The return message is unconditional, but the .dockerignore write at line 365 is gated:
if (force || !fileExists(dockerignorePath)) {
fileWrite(dockerignorePath, ...);
}When an existing .dockerignore is preserved (force=false, file present), the output still says "Created ... .dockerignore" — inaccurate. Track the write in a local boolean and emit "preserved" vs "created" accordingly, or simply omit .dockerignore from the message when it wasn't written.
2. Builder stage COPY package.json fails for apps without npm
cli/lucli/templates/deploy/init/Dockerfile.mustache, line 23:
COPY package.json package-lock.json* ./package.json is a literal filename — Docker COPY fails with "no such file or directory" if it is absent from the build context, even in BuildKit. The majority of Wheels apps have no npm setup; a first-time user running docker build . immediately after wheels deploy init would get an opaque build error before reading the "Drop this stage if..." comment at line 16.
Suggested fix: restructure the builder-stage COPY to be resilient, e.g. copy the full context first and then conditionally install:
COPY . .
RUN if [ -f package.json ]; then npm ci --no-audit --no-fund; fi
RUN if [ -f package.json ] && node -e "..."; then npm run build; fiOr, keep the current two-step pattern but note more prominently in the generated output that the builder stage must be removed for non-npm projects.
Security
3. RELOAD_PASSWORD=change-me baked into image ENV; name does not match the deploy.yml secret
cli/lucli/templates/deploy/init/Dockerfile.mustache, line 42:
ENV WHEELS_ENV=production \
WHEELS_DATASOURCE=app \
RELOAD_PASSWORD=change-me \
...Two concerns:
- Image layer history:
RELOAD_PASSWORD=change-mepersists in every layer of images built from this scaffold, visible indocker inspectand container registry metadata, even if overridden at runtime. - Name mismatch: the pre-existing
deploy.yml.mustache(line 29) injects the secret asWHEELS_RELOAD_PASSWORD; the Dockerfile setsRELOAD_PASSWORD. When deployed viawheels deploy, Kamal putsWHEELS_RELOAD_PASSWORD=<value>in the container environment, but the Dockerfile ENV isRELOAD_PASSWORD=change-me— these are different names, so the Kamal secret does not override the baked-in default. Additionally, the default generated app (cli/lucli/templates/app/config/settings.cfm, line 25) uses a static literal forreloadPassword, notgetenv(), so neither env var name is read at all without user modification.
Recommended: remove RELOAD_PASSWORD from the Dockerfile ENV block entirely. Add a comment explaining that the reload password must be injected at runtime (e.g. via WHEELS_RELOAD_PASSWORD in .kamal/secrets after updating config/settings.cfm to read the env var), and align the env var name with the secrets template.
Tests
4. No spec for the .dockerignore silent-preservation path
cli/lucli/tests/specs/deploy/cli/DeployMainCliSpec.cfc — there is no test that verifies: when a .dockerignore already exists and force=false, it is preserved (not overwritten) and the call still succeeds. The implementation at DeployMainCli.cfc:365 has this behavior but it is untested. Mirror the pattern of the Dockerfile guard spec at line 220 but assert the opposite: .dockerignore survives and init_stub returns without throwing.
5. force=true spec doesn't verify .dockerignore is also overwritten
cli/lucli/tests/specs/deploy/cli/DeployMainCliSpec.cfc, line 237–253: the spec pre-creates config/deploy.yml and Dockerfile but not .dockerignore. Because the .dockerignore isn't pre-created, the if (force || !fileExists(...)) branch always takes the !fileExists path — the force || ... part is never exercised. Pre-create .dockerignore with sentinel content, then assert it was overwritten.
Docs
The init.mdx and first-deploy.mdx updates are accurate and clear. The asymmetric behavior (.dockerignore silently preserved vs Dockerfile hard-aborts) is correctly documented in "What it creates."
One small omission: the plugins/ directory is not in the runtime COPY block (Dockerfile.mustache, lines 54–57). It's deprecated but still supported and users may have it. Worth a one-liner comment noting it.
Commits
Single commit feat(cli): scaffold Dockerfile and .dockerignore in wheels deploy init — valid type, in-scope, subject is 54 chars and sentence-case. Clean.
- Tolerate Wheels apps without an npm pipeline by copying the whole build context once and gating npm install/build on package.json existence. The prior `COPY package.json ...` line failed builds for apps lacking one. - Remove the baked-in RELOAD_PASSWORD=change-me from the Dockerfile ENV. It was both a layer-history leak and named differently from the secret shipped in deploy.yml/.kamal/secrets (WHEELS_RELOAD_PASSWORD), so it never overrode anything anyway. Replaced with a comment pointing to the secrets-injection path. - Report ".dockerignore" honestly in the init summary — say "preserved" when the existing file was kept, "created" only when written. - Cover the silent-preserve path and force-overwrite path for .dockerignore in DeployMainCliSpec. - Note the deprecated plugins/ COPY in the Dockerfile comment for users still on the legacy directory.
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: All five issues raised in the previous round have been cleanly addressed — the RELOAD_PASSWORD env leak is gone, the npm/no-npm Dockerfile now works for any Wheels app, the summary message is accurate, and the .dockerignore preservation paths are fully tested. What remains are two minor nits: a healthcheck path mismatch between the two generated templates, and a missing CHANGELOG entry for a user-visible new feature.
Correctness
1. Healthcheck path mismatch between Dockerfile and deploy.yml (minor, worth a comment)
cli/lucli/templates/deploy/init/Dockerfile.mustache, line 72:
CMD curl -fsS http://127.0.0.1:8080/health || exit 1cli/lucli/templates/deploy/init/deploy.yml.mustache, line 16:
healthcheck:
path: /upThese are technically independent probes — the Docker HEALTHCHECK fires from inside the container, while proxy.healthcheck.path tells kamal-proxy which HTTP path to poll before swapping traffic. But a first-time user who takes both files as-generated will need to add two distinct routes (/health and /up) — or change one of the paths. The Dockerfile comment on line 10 says "add [a /health route] to keep liveness probes off your slow paths," but the deploy.yml already names /up. Aligning both to the same path (or documenting the split explicitly) would reduce friction. Not a bug, but worth a one-sentence note in the Dockerfile comment.
Docs
2. No CHANGELOG entry for the new scaffold behaviour
CHANGELOG.md under [Unreleased] has no mention of this PR. wheels deploy init now emits two additional files and the --force guard now also protects the Dockerfile — these are user-visible behavioural changes. A brief entry under ### Added (or ### Changed for the guard) would follow the pattern of other CLI additions in the same file. Example:
- `wheels deploy init` now scaffolds a starter `Dockerfile` (Lucee 7 multi-stage, `/health` HEALTHCHECK) and `.dockerignore` alongside `config/deploy.yml` and `.kamal/secrets`. `--force` also gates an existing `Dockerfile`. (#2673)Commits
Two commits on this PR:
feat(cli): scaffold Dockerfile and .dockerignore in wheels deploy init— validfix(cli): address Reviewer A feedback on deploy init scaffolding— valid
Both conform to commitlint: correct type, in-scope, subject ≤ 100 chars, not ALL-CAPS.
Everything from the previous review round is resolved. The two findings above are nits — merge can proceed once the CHANGELOG entry is added. The healthcheck-path note is advisory; it doesn't block users but is a DX improvement worth a quick comment in the generated Dockerfile.
- Add an Unreleased "Added" entry for `wheels deploy init` scaffolding the Dockerfile + .dockerignore. - Switch the generated Dockerfile HEALTHCHECK from /health to /up so it matches the kamal-proxy healthcheck path already named in the bundled config/deploy.yml. A first-time user no longer has to add two distinct liveness routes for the same app.
Wheels Bot — Reviewer B (round 1)Reviewer A's review was submitted against commit SycophancyA's TL;DR opens with "All five issues raised in the previous round have been cleanly addressed." Round B-1 did not raise five discrete issues — it raised a structural complaint (A's original review body was the literal placeholder The overall tone is positive and proportionate. The two findings A did surface (healthcheck path mismatch, missing CHANGELOG) are legitimate, so the near-approval stance is not empty praise. No sycophancy beyond the inflated issue-count summary. False positivesNone. A's two findings are accurate against the diff A reviewed:
A correctly characterised both as nits rather than blockers. Missed issues1. A third commit ( The third commit ( 2. The third commit aligned - `Dockerfile` — multi-stage build targeting Lucee 7 on Java 21 with a `/health` HEALTHCHECK and `EXPOSE 8080`The actual generated file now probes 3. In 4. Sequential abort-check ordering is unchanged (from round B-1) A did not address the UX concern raised in round B-1: if CommitsThe third commit type is ConvergenceThe substantive concerns are narrow:
A's overall assessment (merge-ready after nits) was correct at the SHA A reviewed, and the nits have since been addressed. The one residual actionable item is the Convergence: aligned — request changes on the |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The two findings from the previous round (missing CHANGELOG entry, healthcheck path mismatch) have been addressed in ca17811f9. The CHANGELOG entry is accurate, the Dockerfile template now probes /up throughout (CMD and the header comment), and the generated template aligns with config/deploy.yml's proxy.healthcheck.path. One minor doc inconsistency remains in init.mdx.
Docs
One stale /health reference in init.mdx
web/sites/guides/src/content/docs/v4-0-1-snapshot/command-line-tools/commands/deploy/init.mdx, line 32:
- `Dockerfile` — multi-stage build targeting Lucee 7 on Java 21 with a `/health` HEALTHCHECK and `EXPOSE 8080` ...
The commit updated the Dockerfile template (Dockerfile.mustache:73) and its header comment (Dockerfile.mustache:10-11) to use /up, but this prose description in init.mdx was not updated and still says /health. A user reading the docs after init will see the wrong path.
Suggested fix — change /health to /up in that bullet:
- `Dockerfile` — multi-stage build targeting Lucee 7 on Java 21 with a `/up` HEALTHCHECK and `EXPOSE 8080` (matches the default `proxy.app_port` and `proxy.healthcheck.path` in `deploy.yml`). Adjust the `COPY` lines and base image to match your app.
Commits
Three commits, all conforming to commitlint:
feat(cli): scaffold Dockerfile and .dockerignore in wheels deploy init— validfix(cli): address Reviewer A feedback on deploy init scaffolding— validdocs(cli): changelog + align Dockerfile healthcheck path with deploy.yml— valid
All subjects <= 100 chars, sentence-case, correct types.
Everything else from prior rounds is resolved. The single finding above is a one-word doc fix. Merge can proceed once /health -> /up is corrected in init.mdx.
Commit ca17811 switched the generated Dockerfile HEALTHCHECK from /health to /up to match the kamal-proxy healthcheck path in the bundled deploy.yml, but the init.mdx description still named /health. Update the bullet so the docs match the templates a freshly initialised app actually emits.
Wheels Bot — Address Review (round 1)The convergence verdict in Reviewer B's round-1 comment named one residual actionable finding:
That finding was addressed by commit
No new commit is being made by this round — the consensus change is already in the PR at the current head. Skipped (not in the consensus action list per B's convergence):
The next Reviewer A run on this SHA should align with |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: All issues from the previous three rounds have been resolved. The latest commit (0f5971870) is a clean one-line doc fix: the /health path in init.mdx's Dockerfile bullet has been updated to /up, matching the template and deploy.yml. One informational note about stale PR description prose -- no blocking issues remain.
Docs
Stale PR description text (informational, not blocking)
The original PR body under "What changed" still reads:
Includes a
/healthHEALTHCHECK and production env defaults (WHEELS_ENV,WHEELS_DATASOURCE,RELOAD_PASSWORD,CATALINA_OPTS).
Both /health and RELOAD_PASSWORD were superseded by the fix commit (3dc5cc344). The actual templates (Dockerfile.mustache lines 10-11 and 72-73) correctly use /up, and RELOAD_PASSWORD was removed from the ENV block in the same fix commit. A maintainer merging this PR will not be misled by the commits themselves, but the description text does not reflect the current state of the PR.
Optional cleanup before merge:
Includes a `/up` HEALTHCHECK aligned with the generated deploy.yml, and production env defaults
(WHEELS_ENV, WHEELS_DATASOURCE, CATALINA_OPTS). Secrets (reload password, DB password, registry
password) are injected at deploy time via .kamal/secrets -- not baked into the image.
Summary of all rounds
| Round | Finding | Status |
|---|---|---|
| 1 | Success message claimed .dockerignore created when preserved |
Fixed in 3dc5cc344 |
| 1 | Builder stage COPY package.json failed for no-npm apps |
Fixed in 3dc5cc344 |
| 1 | RELOAD_PASSWORD=change-me baked into image ENV |
Fixed in 3dc5cc344 |
| 1 | No spec for .dockerignore silent-preservation path |
Fixed in 3dc5cc344 |
| 1 | force=true spec did not exercise the force branch for .dockerignore |
Fixed in 3dc5cc344 |
| 2 | Healthcheck path mismatch between Dockerfile (/health) and deploy.yml (/up) |
Fixed in ca17811f9 |
| 2 | No CHANGELOG entry | Fixed in ca17811f9 |
| 3 | Stale /health reference in init.mdx |
Fixed in 0f5971870 |
All code, test, and doc issues are clean. The implementation is correct, the CFML changes are engine-agnostic (only fileWrite, fileExists, directoryCreate calls), the templates emit no baked-in secrets, and the BDD specs cover the happy path, abort-without-force, force-overwrite, and silent-preserve paths.
Wheels Bot — Reviewer B (round 1)A's review is substantive and the round-history table is a useful audit trail. One false positive mars an otherwise clean read. The PR is ready to merge. SycophancyNone detected. A's positive assessment is backed by a concrete commit-by-commit summary table and is consistent with the actual diff I re-read. False positivesA's stale PR description finding is not supported by the current PR body. A quotes the description as:
The actual PR body (as returned by
The description already uses Missed issuesA submitted COMMENTED rather than APPROVED. A's assessment is unambiguously clean — no blocking issues, no request-changes, only an informational note that turns out to be a false positive. Leaving the PR in COMMENTED state rather than APPROVED is inconsistent with the finding and leaves the PR without a bot approval signal. Verdict alignmentA's verdict (COMMENTED with no actionable findings) is inconsistent with A's own conclusions. Given a clean diff and a false-positive-only finding, APPROVED would have been the correct state. This does not change the substance of A's review, but it is a form error. ConvergenceAfter reading the diff at the current head ( |
2 participants
Summary
Closes #2673.
wheels deploy initnow emits a starterDockerfileand.dockerignorealongsideconfig/deploy.ymland.kamal/secrets, so the init → setup walkthrough no longer dead-ends at "noDockerfileto build from."What changed
cli/lucli/templates/deploy/init/:Dockerfile.mustache— multi-stage build (Node builder +lucee/lucee:7-tomcat10-jre21runtime), matching the canonical pattern documented atdeployment/docker-deployment.mdx.EXPOSE 8080lines up with the defaultproxy.app_portindeploy.yml. Includes a/upHEALTHCHECK aligned with the generated deploy.yml'sproxy.healthcheck.path, and production env defaults (WHEELS_ENV,WHEELS_DATASOURCE,CATALINA_OPTS). The builder stage works for any Wheels app — projects without an npm pipeline pass through unchanged; projects with apackage.jsoninstall + build automatically. Secrets (reload password, DB password, registry password) are injected at deploy time via.kamal/secrets, not baked into the image.dockerignore.mustache— keeps.kamal/secrets,.env*,vendor/wheels/tests, sourcemaps, and IDE noise out of the build context. (Filename keeps the leading dot off so the template isn't hidden from tooling that filters dotfiles.)DeployMainCli.init_stub()renders both files.--forcenow also gates an existingDockerfile(sameDeployMainCli.InitAlreadyExistsabort asdeploy.yml). An existing.dockerignoreis preserved unless--forceis passed — users often have their own. The success message reports.dockerignoreas either "created" or "preserved existing" accordingly.init_stubhappy-path spec now assertsDockerfileand.dockerignoreare emitted.lucee/lucee:7-tomcat10-jre21,EXPOSE 8080, has aHEALTHCHECK, and contains the service name.--forceon an existingDockerfile(aborts and leaves the file untouched).force=truespec extended to pre-create.dockerignorewith sentinel content and verify both Dockerfile and.dockerignoreare overwritten (exercising theforce ||branch)..dockerignoresurvives a no-force init, and the success message reflects "preserved" rather than "created".$cliInstallDir()regression spec now also checks the two new templates exist on disk.command-line-tools/commands/deploy/init.mdx— flag table and "What it creates" section list the new files.deployment/first-deploy.mdx— "Before you start" precondition rewritten (no longer says you must write a Dockerfile yourself); Step 1 enumerates the four files now emitted.CHANGELOG.md—[Unreleased] / Addedentry under the canonical Keep-a-Changelog ordering.v4-0-0/docs untouched — that's the released-stable line and reflects the older behavior.Test plan
bash tools/test-cli-local.sh— runs the deploy CLI specs that coverinit_stub(cannot run from this sandbox; CI will exercise).DeployMainCli.cfc+ the spec; no engine-specific surface.Generated by Claude Code