fix(cli): ship working defaults in base template, not unsubstituted placeholders#3190
Conversation
…laceholders
The ForgeBox wheels-base-template shipped tools/build/base/{server.json,config/app.cfm,config/settings.cfm} with literal |appName| / |cfmlEngine| / |datasourceName| / |reloadPassword| tokens that only the retired cfwheels-cli flow ever substituted. On the README's documented direct-install path (box install wheels-base-template -> box server start) they reached users unresolved, so box server start aborted with 'Invalid slug detected'.
Replace the placeholders with inert working defaults: pin server.json cfengine to lucee@7 and drop its name key (CommandBox derives it from the folder), comment out the app/datasource name lines so Wheels falls back to the folder-name convention already documented above them, and source reloadPassword from env("WHEELS_RELOAD_PASSWORD", "") like the LuCLI app template. Also remove the dead box.json postInstall reference to a non-shipped env.example (.env ships pre-made).
A structural guard spec fails if any |placeholder| token or the env.example reference reappears in the shipped base-template files.
Fixes #3173
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — No doc updatesReviewed this PR's diff and found no docs that need updating (purely internal template bug fix — the placeholders that were replaced are not documented anywhere in the guides, |
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR replaces the never-substituted |appName| / |cfmlEngine| / |datasourceName| / |reloadPassword| tokens in the ForgeBox base template with working defaults and adds a structural guard spec — the approach is right and closely mirrors prior art (buildArtifactLicenseSpec.cfc). However, two findings block: the new spec declares var server, a CFML reserved scope name (documented anti-pattern #11, a cross-engine hazard inside the all-or-nothing core suite), and the new settings.cfm comment tells users .env "never lands in version control" while the shipped template .gitignore does not actually ignore .env. Verdict: request changes — both fixes are small. (Submitted as a comment-state review only because GitHub forbids a request-changes review on the authoring account own PR; treat the two blocking findings below as must-fix before merge.)
Cross-engine
var server is a reserved scope name — rename it.
vendor/wheels/tests/specs/baseTemplatePlaceholdersSpec.cfc:57:
var server = deserializeJSON(fileRead(repoRoot & "/tools/build/base/server.json"));
expect(server.app.cfengine).notToInclude("|");CLAUDE.md anti-pattern #11 lists server among the reserved scope names and the rule is explicit: "never use a reserved scope name as a parameter, local var, or function argument name" (the generalized rule is also restated in .ai/wheels/testing/browser-testing.md:64). Whether the local declaration or the built-in scope wins is engine-dependent, and on an engine where the scope wins, server.app.cfengine reads the real server scope and the assertion errors. This is the only var server in the entire vendor/wheels tree — there is no prior art tolerating it. Because the core suite compiles every spec CFC in one pass (directory="wheels.tests.specs"), a single engine that rejects the declaration takes down the whole suite for that engine. Fix: rename to serverJson (and consider boxJson instead of var box at line 65 for symmetry, though box is not a documented reserved name).
Security
The new comment promises .env is git-ignored — the shipped template .gitignore does not ignore it.
tools/build/base/config/settings.cfm:25-27 (added in this PR):
// Reload your application with ?reload=true&password=(your WHEELS_RELOAD_PASSWORD)
// The secret is read from .env (git-ignored) so it never lands in version control.
set(reloadPassword=env("WHEELS_RELOAD_PASSWORD", ""));tools/build/scripts/prepare-base.sh copies tools/build/base/.gitignore verbatim into the package as its .gitignore, and that file contains only /vendor/*/ and /db/** — no .env entry. It also copies the repo-root .env into the package (cp .env "${BUILD_DIR}/.env"). So a user who follows the comment, puts their reload password in .env, and runs git init && git add . commits the secret — the claim in the comment is false for the artifact as shipped. The LuCLI app template already handles this correctly (cli/lucli/templates/app/_gitignore has .env / .env.* / !.env.example under "Environment files (contain secrets)"). Fix: add the same three lines to tools/build/base/.gitignore. Alternatively soften the comment, but aligning the gitignore with the LuCLI template is the better fix and is one line of follow-through on the design of this PR.
Tests
The guard spec itself is well built: the IIFE loop-variable capture and expandPath("/wheels/../..") root resolution are lifted directly from buildArtifactLicenseSpec.cfc:22,33 (proven cross-engine), ##3173 is correctly escaped inside string literals, and the structural-assertion rationale is documented in the header. No issues beyond the var server rename above.
Docs
Non-blocking: the shipped .env (copied from the repo root) does not define WHEELS_RELOAD_PASSWORD, so out of the box ?reload=true is disabled — which is the safe fail-closed behavior (vendor/wheels/events/onapplicationstart.cfc:371-374 logs a one-time warning), but nothing in the template README tells the user the knob exists. Consider a one-line README note or shipping a commented WHEELS_RELOAD_PASSWORD= line so the feature is discoverable.
Commits
Non-blocking: the commit body has lines up to 474 chars; CLAUDE.md states body lines must stay at or under 100, but the CI commitlint job validates only the PR title (.github/workflows/pr.yml:25-26) and recent develop commits routinely exceed 100, so this is noted, not blocked on. Header, type, and DCO sign-off are all conforming.
|
Superseded by #3196 (merged): a propose-fix draft that raced the ForgeBox path-repair campaign for the same issue; the campaign PR landed first and closed the underlying issue. Closing the duplicate. |
Summary
The ForgeBox
wheels-base-templateshipped the static files undertools/build/base/verbatim —tools/build/scripts/prepare-base.shonly substitutes@build.version@/@build.number@, nothing else. The legacy|appName|/|cfmlEngine|/|datasourceName|/|reloadPassword|tokens inserver.json,config/app.cfm, andconfig/settings.cfmwere only ever resolved by the retired CommandBoxcfwheels-cli(wheels create app) flow, so on the README's documented direct-install path (box install wheels-base-template→box server start) they reached users unresolved andbox server startaborted withERROR: Invalid slug detectedwhile resolving the|cfmlEngine|slug.This replaces the placeholders with inert working defaults that boot out of the box:
server.json—cfenginepinned tolucee@7; thenamekey dropped so CommandBox derives the server name from the install folder.config/app.cfm—this.namecommented out so Wheels derives the application name from the folder (mirrorsvendor/wheels/tests/resources/app/config/app.cfm).config/settings.cfm— thedataSourceName/coreTestDataSourceNameset()lines commented out (the folder-name fallback the surrounding comment already documents);reloadPasswordsourced fromenv("WHEELS_RELOAD_PASSWORD", "")like the LuCLI app template.box.json— removed the deadpostInstallreference to a non-shippedenv.example(.envships pre-made).A structural guard spec (
vendor/wheels/tests/specs/baseTemplatePlaceholdersSpec.cfc, in the spirit ofbuildArtifactLicenseSpec.cfc) fails the suite if any|placeholder|token or theenv.examplereference reappears in the shipped base-template files.Related Issue
Fixes #3173
Type of Change
Feature Completeness Checklist
Signed-off-by:(viagit commit -s)bot-update-docs.ymlbot-update-docs.ymlbot-update-docs.ymlchangelog.d/3173-base-template-placeholders.fixed.mdTest Plan
The new spec asserts the shipped base-template files contain no pipe-delimited
|placeholder|tokens, thatserver.jsoncfengineis a concrete slug, and thatbox.jsonpostInstalldoes not referenceenv.example.No CFML engine /
wheelsCLI binary is available in the bot sandbox, so the BDD runner (bash tools/test-local.sh) could not start a server here (it died withnohup: failed to run command 'wheels': No such file or directory). The spec's red→green transition was therefore verified with the exact equivalent matcher — the spec'sreMatch("\|[A-Za-z][A-Za-z0-9_]*\|", content)is semantically identical to the POSIX-EREgrep -Ebelow:|placeholder|tokens andbox.jsonmatchedenv.example— every assertion would fire.The authoritative cross-engine BDD run happens in CI (
bot-tdd-gate.yml+ the compat matrix).Screenshots / Output