docs(web/guides): document env() pattern for config/environment.cfm load-order gap - #2720
Conversation
…oad-order gap Fixes #2709 Co-Authored-By: claude[bot] <claude[bot]@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: A focused docs-only PR that documents the config/environment.cfm load-order gap and the env() fix in both upgrade guide copies (v4-0-0 and v4-0-1-snapshot). The content is accurate, the CHANGELOG and Common Issues entries are present, and the commit message is clean. Three minor nits below — none are blockers. Verdict: comment (no code change required to merge, but the annotation issue is worth addressing before this file set is ever picked up by docs-verify).
Correctness
The documented behavior is accurate. application.env is populated by Application.cfc::onApplicationStart() and is not reliably available before config files run (confirmed in web/sites/guides/src/content/docs/v4-0-0/deployment/production-config.mdx). The env() helper bypasses application.env entirely and the "production" fail-safe default is intentional and correct.
One prose nit: the docs introduce set(environment=application.env.environment) as a pattern users put in config/environment.cfm, but cli/lucli/templates/app/config/environment.cfm scaffolds a hardcoded string (set(environment="development")). The warning therefore targets a pattern users wrote by hand rather than one the generator produced. A single orienting sentence would prevent confusion for someone who just used wheels new:
If you manually wired the running environment from .env — for example, set(environment=application.env.environment) — the same load-order gap applies.
Docs
{test:compile} on the negative-example block — the "unreliable pattern" fence at line 149 of both MDX files carries the {test:compile} annotation alongside the correct-pattern fence. docs-verify.yml only fires on v4-0-0-snapshot/** today so this won't break CI on the current PR. But compile.mjs's native driver runs wheels cfml which executes the snippet directly — application.env.environment will throw when application.env is not populated, returning a non-zero exit and failing the verify run. When coverage is eventually extended to v4-0-0/ this block will be a false-failure.
Negative-example blocks don't need {test:compile}. Removing the annotation from the unreliable-pattern fence in both files avoids the future fragility with no loss of value.
CHANGELOG section placement — the new entry landed in the ### Fixed section (line 60, CHANGELOG.md). All three sibling docs-only entries just below (lines 67-69) are correctly placed under ### Documentation. This PR is a docs change (commit type is docs), so the entry belongs under ### Documentation. The misplacement makes it read as a code fix rather than a documentation improvement.
Commits
Single commit — docs(web/guides): document env() pattern for config/environment.cfm load-order gap — passes commitlint: type is docs (allowed), scope is web/guides (unrestricted, appropriate), subject is 64 characters, not ALL-CAPS. DCO sign-off is present and matches the commit author identity (claude[bot] <41898282+claude[bot]@users.noreply.github.com>). No issues.
Wheels Bot — Reviewer B (round 1)A's review is solid. All three findings are accurate and the verdict of SycophancyNone detected. A's positive statements are fact-backed (commit message is verifiably clean, content is accurate), and the verdict is False positivesNone detected. I verified all three findings against the diff and repo state:
Missed issuesNone significant. Checked: cross-engine compat (not applicable — docs only), test coverage (not applicable), commit DCO and commitlint (verified by A, correct), symmetry between the two MDX files (both receive identical changes). Verdict alignmentA's ConvergenceAligned. The concrete changes needed are: (1) move the CHANGELOG entry from |
- Move the #2709 CHANGELOG entry from `### Fixed` to `### Documentation` since this is a docs-only change. - Drop `{test:compile}` from the negative-example fence in both `v4-0-0` and `v4-0-1-snapshot` `upgrading/3x-to-4x.mdx` — the snippet is intentionally broken and would be a future false-failure once docs-verify coverage extends to `v4-0-0/`. - Add an orienting sentence clarifying that `wheels new` scaffolds a hardcoded `set(environment="development")` and the `application.env.environment` pattern is hand-written, so readers who just ran `wheels new` know whether the section applies to them. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
No findings were intentionally skipped — the consensus set was fully addressed. The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a follow-up commit (92029d0) that directly addresses all three findings from my round-1 review of this docs-only PR. The CHANGELOG entry is now correctly under ### Documentation, the {test:compile} annotation has been removed from the negative-example fence in both MDX files, and the orienting sentence now front-loads the "you're not affected if you used wheels new" context before describing the manual-wiring edge case. No new issues introduced. Verdict: comment (self-approval not permitted; no remaining objections — ready for human sign-off).
Correctness
All three round-1 findings resolved:
-
CHANGELOG placement — entry moved from
### Fixedto### Documentation(now atCHANGELOG.mdline 70), consistent with thedocscommit type and the sibling entries for #2633, #2627, and #2631. -
{test:compile}on negative example — removed from the unreliable-pattern fence in bothv4-0-0/upgrading/3x-to-4x.mdx:149andv4-0-1-snapshot/upgrading/3x-to-4x.mdx:149. The positive-pattern fence at line 158 in both files retains{test:compile}, which is correct. -
Orienting sentence — both files now open the sub-section with:
wheels newscaffolds a hardcodedset(environment="development")inconfig/environment.cfm, so this gap does not affect freshly generated apps. If you wired the running environment from.envby hand...This is cleaner than the suggestion in the round-1 review: it gives the "you're safe" signal first, then qualifies who is affected.
Both MDX files are byte-identical in the changed region. The Common Issues bullet and the "production" fail-safe rationale are unchanged and accurate.
Commits
Two commits total:
db782a8—docs(web/guides): document env() pattern for config/environment.cfm load-order gap— reviewed in round 1.92029d0—docs(web/guides): address Reviewer A/B consensus findings (round 1)— typedocs, scopeweb/guides, subject 47 chars, not ALL-CAPS. DCO sign-off present (Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>). Clean.
One cosmetic note: the subject says "Reviewer A/B consensus findings" but only a Reviewer A review is visible on this PR. Not a blocker — the commit lands correctly regardless.
Wheels Bot — Reviewer B (round 1)A's review of the address-review commit is accurate and thorough. All three round-1 findings are correctly verified as resolved, no new issues were introduced, and A's verdict of SycophancyNone detected. A's positive statements are fact-backed — each finding is traced to a specific diff line — and A does not use False positivesNone detected. I verified all three claims against the diff:
One minor imprecision in A's cosmetic note: A says "only a Reviewer A review is visible on this PR," but Reviewer B did post in round 1 as a PR comment (not a formal review). The commit subject "Reviewer A/B consensus findings" is therefore accurate. Not a false positive in A's findings — just a note on the note. Missed issuesNone. This is a docs-only PR. Cross-engine compat and test coverage are not applicable. Commit DCO and commitlint are clean (A verified; I confirmed the Verdict alignmentA's ConvergenceAligned. A has no remaining findings, all round-1 items are resolved, and the diff introduces nothing new to flag. The PR is review-clean for this SHA and ready for human sign-off. |
Both upgrade guide pages (v4-0-0 and v4-0-1-snapshot) now document the
config/environment.cfmload-order gap that caused production servers to reportenvironment=developmentdespite.envcontainingenvironment=production. The canonical fix —set(environment=env("environment", "production"))— and its deliberate"production"fail-safe default are documented alongside the existingreloadPasswordguidance in item 4. A matching entry is added to the "Common issues" section for discoverability.Fixes #2709
What changed
web/sites/guides/src/content/docs/v4-0-0/upgrading/3x-to-4x.mdx— extended item 4 with a#### config/environment.cfm has the same load-order gapsub-section; added a "Common issues" bullet.web/sites/guides/src/content/docs/v4-0-1-snapshot/upgrading/3x-to-4x.mdx— same changes.CHANGELOG.md—[Unreleased]entry.Out of scope (follow-up required)
The triage also suggested updating the starter-app template at
cli/lucli/templates/app/config/environment.cfm. That file is undercli/**, which is outside the docs-only scope of this stage. A follow-up code PR should update the template so newly scaffolded apps ship withset(environment=env("environment", "production"))rather than theapplication.env.environmentpattern.Screenshots needed
None — no UI changes.