Skip to content

docs(web/guides): document reserved CFML scope names in the controllers guide#2834

Draft
wheels-bot[bot] wants to merge 2 commits into
developfrom
docs/bot-2833-misleading-client-scope-is-not-enabled-when-a-loca
Draft

docs(web/guides): document reserved CFML scope names in the controllers guide#2834
wheels-bot[bot] wants to merge 2 commits into
developfrom
docs/bot-2833-misleading-client-scope-is-not-enabled-when-a-loca

Conversation

@wheels-bot
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot commented May 29, 2026

This PR adds a "Reserved scope names" section to the Controllers and Actions guide, directly addressing the user report in #2833.

The reporter discovered that naming a local variable client in a Wheels controller causes Lucee 7 to throw "client scope is not enabled" — an error that looks like an application configuration problem rather than a naming problem. The root cause is CFML scope precedence: Lucee resolves client to the (disabled) built-in client scope instead of the local var declaration.

The new section:

  • Lists all reserved CFML scope identifiers that must not be used as local variable names: client, url, form, session, cgi, request, application, cookie, server, arguments, variables
  • Explains the client case in detail, including the exact Lucee 7 error message
  • Shows a wrong/right code example
  • Includes an Aside calling out var client = $testClient() in test code as the most common occurrence

Fixes #2833

Screenshots needed

None — this PR is text-only documentation.

Follow-up not included in this PR

The testing guide examples in testing/controller-tests.mdx, testing/functional-tests.mdx, testing/view-and-form-tests.mdx, and testing/integration-tests.mdx all use var client = $testClient() as the canonical pattern. Renaming those (~22 occurrences across 4 files, plus all subsequent client.get(...) / client.post(...) / client.assertXxx(...) uses) would add significant churn and widen the scope of this PR. A separate PR should rename client to httpClient or testClient throughout those examples for consistency with the guidance added here.

Documents that client, url, form, session, cgi, request, application,
cookie, server, arguments, and variables must not be used as local
variable names in controllers or models. Calls out the client collision
specifically — Lucee 7 throws "client scope is not enabled" when
clientManagement is off, making the cause hard to spot.

Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This PR adds a well-needed "Reserved scope names" section to the Controllers guide and a matching CHANGELOG entry, directly addressing the Lucee 7 client scope footgun in #2833. The docs content is accurate and the code examples are correct. I have three minor nits — one CHANGELOG categorisation mistake, one scope-list omission worth calling out, and one forward-looking concern about an immediate doc inconsistency the PR itself flags. No correctness, cross-engine, or security issues. Verdict: comment (minor nits only, no blocking issues).


Docs

1. CHANGELOG entry is in ### Changed, not ### Added

File: CHANGELOG.md, line 25 (the new bullet).

The entry reads:

### Changed

- Added a "Reserved scope names" section …

Keep a Changelog format reserves ### Changed for modifications to existing behaviour; a brand-new guide section belongs under ### Added. The word "Added" even appears in the bullet itself, which makes the categorisation look inconsistent.

Suggested fix: move the bullet to ### Added (create the heading if this is the first Added entry in [Unreleased]).


2. CHANGELOG bullet says "in controllers or models" — the guide only covers controllers

Same line, CHANGELOG.md:25:

…that must not be used as local variable names in controllers or models…

The changed file is controllers-and-actions.mdx and the new section lives under the "Controllers and Actions" page. Nothing in the diff touches a models guide or explains how the rule applies to model CFCs specifically. "or models" implies coverage that doesn't exist yet.

Suggested fix: drop "or models" or broaden to "in CFML components" if the intent is to cover the general case:

…that must not be used as local variable names in Wheels controllers (and CFML components generally)…

3. local and this are missing from the reserved-scope identifier list

File: web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx, line 257 (new content):

`client`, `url`, `form`, `session`, `cgi`, `request`, `application`, `cookie`, `server`, `arguments`, `variables`

Two additional CFML scopes cause real footguns:

  • local — the implicit function-local scope. Writing var local = gateway; local.charge(amount) stores gateway at local.local (because var writes into the local scope), so local.charge resolves against the scope, not the object. This is a subtle trap for anyone naming a service variable local.
  • this — the component public scope. var this = someStruct inside a function shadows component state in ways that differ across engines.

CLAUDE.md § Anti-Pattern #11 lists the same eleven identifiers and also omits these two — so this is a pre-existing gap in both places — but since this PR's purpose is precisely to document the hazard list, it's worth making the list more complete here, even if a separate pass is needed to update CLAUDE.md.

Suggested addition at the end of the list:

`client`, `url`, `form`, `session`, `cgi`, `request`, `application`, `cookie`, `server`, `arguments`, `variables`, `local`, `this`

Consistency (forward-looking, not blocking)

The new Aside correctly flags var client = $testClient() as the most common occurrence, and the PR body honestly notes ~18 remaining uses across four testing guide files. That's a real inconsistency a reader will hit immediately: the controllers page now says "don't use client as a variable name" and the testing page right beside it shows var client = $testClient() as the canonical pattern throughout. The follow-up PR mentioned in the body should ideally be opened at the same time this one merges (or as a direct follow-on) rather than left open-ended.


No commits to flag — the single docs(web/guides): commit is well-formed, subject is under 100 chars, and the scope is appropriate.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 29, 2026

Wheels Bot — Reviewer B (round 1)

A's review is accurate, well-supported by the diff, and free of sycophancy. All three nits check out against the actual changed lines. I agree with A's findings and the actionable suggestions. Emitting converged-changes so the consensus fixes get applied.

Sycophancy

None detected. A gave a specific verdict ("comment"), cited exact lines for each nit, and did not approve unconditionally.

False positives

None detected.

  • Finding 1 (CHANGELOG category): Confirmed. The diff places the new bullet under ### Changed while the bullet itself says "Added a...". Correct category is ### Added.
  • Finding 2 ("or models" overstep): Confirmed. The diff only touches controllers-and-actions.mdx. The CHANGELOG text implies model coverage that isn't there.
  • Finding 3 (local and this missing): Confirmed. local is the implicit function-local scope in CFML. var local = x writes to local.local, leaving local bound to the scope struct, not x. That is a real footgun in the same class as the listed identifiers. this (the component public scope) is a weaker case but still valid. A's suggestion to append both to the list is sound.

Missed issues

One minor gap: CLAUDE.md Anti-Pattern 11 carries the identical eleven-identifier list and has the same omission. A explicitly acknowledged this as a "pre-existing gap" and deferred it to a separate pass, which is a reasonable scoping call for a docs PR. However, since bot-address-review.yml will be touching this PR anyway, it would be low-cost to add local and this to CLAUDE.md Anti-Pattern 11 in the same commit. Not a blocker, just worth including in the address pass.

Verdict alignment

A's "comment" verdict is consistent with the findings: the guide content itself is correct, and all three nits are in the CHANGELOG text or the scope list — none warrant blocking the merge outright.

Convergence

Aligned. A has three concrete, actionable findings (CHANGELOG category, CHANGELOG text scope, missing local/this in the identifier list). Joint recommendation: apply the consensus fixes and merge. Triggering address-review.

- Move the new "Reserved scope names" CHANGELOG entry from `### Changed`
  to `### Added`, matching Keep a Changelog semantics for brand-new
  documentation content (CHANGELOG.md).
- Drop "or models" from the CHANGELOG bullet, since the diff only
  touches `controllers-and-actions.mdx`; rephrase to "Wheels
  controllers (and CFML components generally)" so the wording matches
  the scope of the actual change (CHANGELOG.md).
- Append `local` and `this` to the reserved-scope identifier list in
  the Controllers and Actions guide. `local` is the implicit
  function-local scope and `var local = x` writes to `local.local`;
  `this` is the component public scope. Both produce real footguns in
  the same class as the listed identifiers
  (web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx).
- Apply the same `local`/`this` addition to CLAUDE.md Anti-Pattern 11,
  which carries the identical hazard list and had the same omission
  (CLAUDE.md). B suggested this as a low-cost bundle since
  address-review was touching the PR anyway.

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 29, 2026

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

  • CHANGELOG category fix — Moved the new "Reserved scope names" bullet out of ### Changed and into a new ### Added heading in [Unreleased]. Keep a Changelog reserves ### Changed for behavior modifications; a brand-new guide section belongs under ### Added. (CHANGELOG.md:23-25)
  • CHANGELOG "or models" overstep — Dropped "or models" from the bullet and rephrased to "Wheels controllers (and CFML components generally)", since the diff only touches controllers-and-actions.mdx and nothing in the change set explains the rule's application to model CFCs specifically. (CHANGELOG.md:25)
  • local and this added to the reserved-scope list — Appended both identifiers to the list in the Controllers and Actions guide. local is the implicit function-local scope (var local = x writes to local.local, leaving local bound to the scope struct); this is the component public scope. Both are real footguns in the same class as the eleven previously listed identifiers. (web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx:257)
  • CLAUDE.md Anti-Pattern 11 brought in line — Applied the same local/this addition to the identical hazard list in CLAUDE.md § Anti-Pattern 11. A flagged this as a pre-existing gap deferrable to a separate pass; B noted it was low-cost to bundle here since address-review was touching the PR anyway. Bundled. (CLAUDE.md:172)

Skipped:

  • A's "Consistency (forward-looking, not blocking)" note about renaming var client = $testClient() across testing/controller-tests.mdx, testing/functional-tests.mdx, testing/view-and-form-tests.mdx, and testing/integration-tests.mdx — explicitly flagged as forward-looking and non-blocking, and the PR body already calls out the follow-up. Out of scope for this convergence round.

No test run was required — docs/bot-* PR, all edits confined to CHANGELOG.md, CLAUDE.md, and web/sites/guides/.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on approve or the outer-loop cap (5 rounds) is reached.

Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: Round-1 address-review commit correctly resolves all three findings from the previous review: the CHANGELOG entry is now under ### Added, the "or models" wording is dropped in favour of "(and CFML components generally)", and local and this are appended to the identifier list in both the controllers guide and CLAUDE.md Anti-Pattern 11. No correctness, cross-engine, or security issues in the new material. Verdict: approve (submitting as comment because the bot cannot approve its own PR).


Docs

All three round-1 items are clean:

  1. CHANGELOG under ### Added — confirmed at CHANGELOG.md:23. ✓
  2. Wording — bullet now reads "Wheels controllers (and CFML components generally)". ✓
  3. Identifier list — both local and this appear at controllers-and-actions.mdx:257 and in CLAUDE.md Anti-Pattern 11. ✓

The description for local ("the engine may resolve the identifier to the scope struct instead of the value you declared") is technically accurate: var local = x writes local.local = x (the var keyword targets the local scope), so a subsequent local.charge(amount) resolves through the scope rather than the declared value. Adequate for a warning section.


Commits

Two commits, both well-formed:

  • 4844f3b1adocs(web/guides): add reserved scope names section to controllers guide — subject under 100 chars, correct type/scope, DCO sign-off present and matching (claude[bot]). ✓
  • a0ffc93c4docs(web/guides): address Reviewer A/B consensus findings (round 1) — same; sign-off matches author. ✓

The first commit also carries a wheels-bot[bot] sign-off trailer from the write-docs run. That extra trailer is harmless — the DCO app keys on the author-matching trailer, which is present.


No new correctness, cross-engine, test-coverage, or security issues introduced.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 29, 2026

Wheels Bot — Reviewer B (round 1)

A's review of the round-1 address commit is accurate and well-supported by the diff. All three prior findings are confirmed resolved. No sycophancy, no false positives, no missed issues on a pure documentation change. Emitting converged-approve.

Sycophancy

None detected. A verified each of the three prior findings against specific diff locations before declaring approve, and noted the bot-cannot-approve-its-own-PR constraint rather than silently upgrading to a formal approval.

False positives

None detected.

  • Finding 1 (CHANGELOG under ### Added): Confirmed in the diff. The new bullet appears under ### Added before the existing ### Changed block.
  • Finding 2 (wording "and CFML components generally"): Confirmed. The CHANGELOG bullet reads "Wheels controllers (and CFML components generally)" — the overreaching "or models" phrase is gone.
  • Finding 3 (local and this in both identifier lists): Confirmed in both changed files. The controllers guide and CLAUDE.md Anti-Pattern 11 both now include local and this. A's explanation of the local collision mechanism (var local = x writes local.local = x, leaving local bound to the scope struct) is technically correct.

A's note that the second commit's Signed-off-by trailer for claude[bot] matches the configured git author is also correct per git log.

Missed issues

None detected. This is a docs-only change across CHANGELOG.md, CLAUDE.md, and one .mdx guide file. No executable code paths, no cross-engine concerns, no migration, no DI, no security surface. A's call that no test run was required is correct.

Verdict alignment

A's "approve" (submitted as comment due to bot-self-review constraint) is consistent with all prior findings being resolved and no new issues introduced.

Convergence

Aligned. A and I agree the PR is review-clean on this SHA. No pending findings on either side. Joint recommendation: approve and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misleading "client scope is not enabled" when a local var client = ... is used as a component handle

0 participants