fix(config): rename local.url in $buildRedirectUrl so url scope resolves on Adobe CF - #3056
Conversation
…ves on Adobe CF Issue #3053: #3036 added unscoped url.* reads to $buildRedirectUrl() in public/Application.cfc, a function that already declared a string local named `url` (the redirect target). On Adobe CF unscoped name resolution binds the bare `url` reads to that string local before the URL scope and throws a ScopeCastException (HTTP 500) before applicationStop(), so every password-gated reload and URL environment switch became non-functional on Adobe; Lucee masked it because `url` is a reserved scope that always wins. Rename the local to local.redirectUrl across all four same-lineage copies of public/Application.cfc (demo app, app template, tweet + starter-app examples) so the url.* reads resolve to the URL scope on every engine. This is CLAUDE.md cross-engine anti-pattern #11 verbatim. Adds a structural regression guard (BuildRedirectUrlScopeShadowSpec) that fails if a reserved-scope local.url reappears alongside unscoped url reads in any copy. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Add a second example to CLAUDE.md anti-pattern #11 showing that a `local.*` variable named after a reserved scope (e.g. `local.url`) can shadow bare scope reads in the same function — the inverse of the named- parameter case. On Adobe CF the string local wins over the URL scope, causing `ScopeCastException`; on Lucee the reserved scope wins. Refs #3053. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3053 by renaming the string local local.url to local.redirectUrl inside $buildRedirectUrl() across all four same-lineage copies of public/Application.cfc, so the unscoped url.* reads added in #3036 resolve to the URL scope instead of a string local on Adobe CF (CLAUDE.md cross-engine anti-pattern #11, same family as #2591). I verified the rename is complete — grep finds zero remaining local.url references in any of the four copies — and the new structural spec mirrors the established ReloadEnvironmentSwitchParitySpec.cfc pattern (same repo-root resolution, same IIFE loop capture, same four-target parity list), so it carries no novel cross-engine compile risk for the bundle. The commit is a valid conventional commit with a matching DCO sign-off, the changelog fragment is in place, and the PR targets develop. Verdict: comment — the change itself is clean; the findings below are minor test-robustness nits plus one merge-gate reminder.
Tests
-
The spec's vacuity guard is satisfied by the PR's own explanatory comment —
vendor/wheels/tests/specs/cli/BuildRedirectUrlScopeShadowSpec.cfc:84-90. The sanity assertionexpect( fnBody contains 'StructKeyExists(url, "reload")' ).toBeTrue(...)is meant to fail if the #3036 URL-scope reads ever move out of
$buildRedirectUrl()(per the comment at spec lines 81–83: "If these reads ever move out the test below would pass vacuously"). But the NOTE comment this PR adds inside the function body contains the exact same literal — e.g.public/Application.cfc:472:// (StructKeyExists(url, "reload"), ...) bind to a string local named— and the comment sits inside the slicebodyOf()extracts. So if the real reads are later removed while the comment stays, the guard still passes and the spec's "must still read the URL scope" premise silently stops being checked. This is the fragility CLAUDE.md anti-pattern #14 ("Strip CFML Comments Before Source-Scanning", from #2595) warns about, applied to a test. Two cheap fixes, either is fine: (a) strip//line comments fromfnBodybefore both assertions (the same hardening would also prevent a future false failure of thelocal\.url\bcheck if someone documents the rename inline as literallocal.url), or (b) drop theStructKeyExists(url, "reload")literal from the four in-function comments so only real code can satisfy the guard. The main assertion (zerolocal.urlin the body) is unaffected and works today — this only weakens the meta-guard, hence nit severity. -
Adobe runtime acceptance criteria remain unverified — treat compat-matrix as the merge gate. The PR body discloses this honestly (Lucee-only local runner;
RefsnotFixes), and the structural spec is the right TDD artifact for what the bot environment can reach. But since the entire point of the fix is Adobe CF behavior, a maintainer should confirmadobe2023/adobe2025are green in compat-matrix — ideally with the manual?reload=true&password=...→ 302 probe from the PR's "For human/CI verification" list — before flipping this out of draft and closing #3053. CLAUDE.md's "Verify Adobe CF fixes locally before pushing" guidance applies to whoever picks this up with Docker access.
Verified clean (for the record)
- Correctness / cross-engine: all six
local.urlreferences per copy are renamed (matches the spec's before-fixExpected [0] Actual [6]); call sites in$handleRestartAppRequestalready usedlocal.redirectUrl(public/Application.cfc:426,:466) so naming is now consistent. The spec's slice boundaries hold in all four files (loadEnvFile(immediately follows$buildRedirectUrl(in each). The spec's idioms (arrow functions, IIFE loop capture,containsoperator, raw-string regex) all have prior art in the same directory (ReloadEnvironmentSwitchParitySpec.cfc:55-66), and none of the compile-crash anti-patterns (#5 inline-closure constructor arg, unescaped#in strings) are present. - Commits: single commit,
fix(config): …header is 86 chars, type/subject valid percommitlint.config.js, sign-off matches the author identity. - Docs: changelog fragment
changelog.d/3053-buildredirecturl-url-scope-shadow.fixed.mdfollows the fragment convention (no directCHANGELOG.mdedit); the underlying gotcha is already documented as CLAUDE.md anti-pattern #11, so no new reference-doc obligation.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3053 — the local.url string local in $buildRedirectUrl() shadowed the reserved url scope on Adobe CF, turning every password-gated reload and URL environment switch into an HTTP 500 — by renaming the local to local.redirectUrl across all four same-lineage copies of public/Application.cfc, with a structural regression spec and a CLAUDE.md anti-pattern #11 expansion. The fix is correct, complete, and exactly matches the documented anti-pattern; I verified at the head SHA that no local.url (or var url / arguments.url) remains in any of the four files, the caller already consumes the return value, and both commits are commitlint-clean with matching DCO sign-offs. Verdict: comment — no blocking findings, two non-blocking test-robustness notes and one trivial metadata nit below.
Tests
-
The vacuous-pass sanity check can itself pass vacuously via the new NOTE comment.
vendor/wheels/tests/specs/cli/BuildRedirectUrlScopeShadowSpec.cfc(~line 88) guards against the main assertion going vacuous:expect( fnBody contains 'StructKeyExists(url, "reload")' ).toBeTrue(But the NOTE comment this PR adds inside
$buildRedirectUrl()contains that exact literal —public/Application.cfc:472reads// (StructKeyExists(url, "reload"), ...) bind to a string local named— so if a future refactor moved the #3036 reads out of the function, the sanity check would still pass on the comment text alone. More broadly, the spec does substring/regex matching over CFML source without stripping comments first, which is CLAUDE.md anti-pattern #14 ("Strip CFML Comments Before Source-Scanning") — and that cuts both ways: a future comment mentioninglocal.urlinside the function body would false-fail thereMatchNoCase("local\.url\b", fnBody)assertion (the PR's own NOTE comments carefully avoid that literal, which shows the hazard is real). Suggested fix: runfnBodythrough a comment-stripper before both checks (prior-art helpers are listed under anti-pattern #14, e.g.cli/lucli/services/Analysis.cfc::$stripCfmlComments(); a local strip of//-lines and/* */blocks insidebodyOfwould also do). Non-blocking — today the real reads exist (public/Application.cfc:504) and the four files contain nolocal.urlanywhere, so both assertions currently exercise real code. -
Adobe runtime acceptance criteria remain unverified — disclosed honestly in the PR body (the bot's runner is Lucee-only; the bug is invisible on Lucee). The rename's Adobe behavior follows directly from anti-pattern #11 semantics and the structural spec pins the static defect, so this is fine to land on the strength of compat-matrix CI — but the issue's runtime ACs (
?reload=true&password=...→ 302-and-restart on adobe2023/2025) should be confirmed by the compat-matrix run or a human with the Adobe harness before #3053 is closed. The PR's use ofRefsrather thanFixesis the right call.
Otherwise test coverage is solid: BDD spec extending wheels.WheelsTest, failing-→-passing TDD evidence in the PR body, and the spec mirrors ReloadEnvironmentSwitchParitySpec.cfc prior art faithfully (same expandPath("/wheels/../..") root resolution, same IIFE loop-variable capture, same four-copy parity contract).
Docs
- PR-body checklist inconsistency (trivial): the Feature Completeness Checklist leaves "CLAUDE.md" unchecked with "handled separately by
bot-update-docs.yml", but commite5c8b0f4cin this PR does update CLAUDE.md (the anti-pattern #11 expansion — a good addition, accurately describing the Adobe-resolves-local-first vs Lucee-scope-wins inversion). Worth ticking the box so the record matches the diff.
Changelog fragment is correctly done: changelog.d/3053-buildredirecturl-url-scope-shadow.fixed.md, valid .fixed type, complete bullet with issue ref, no direct CHANGELOG.md edit.
Commits
Both commits conform: fix(config): … (86-char header) and docs: … (71-char header), valid types, bodies explain the why, and Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> matches the commit author on both. The PR title is itself a valid conventional-commit header for the squash-merge gate.
Verified clean
- Correctness: rename is internally consistent in all four copies (assignment branches, query-string re-append at
public/Application.cfc:538, and thereturn);git grep 'local\.url\b'over the four files at the head SHA returns nothing; no other reserved-scope shadows (var url,arguments.url) in those files. - Cross-engine: the change removes an engine divergence; the spec uses only patterns with in-suite prior art (arrow functions,
containsoperator, captured closure vars — all present in the existingcli/andauth/specs). No mutation-through-closure, no bracket-notation calls, no constructor-named-arg closures. - Security: no change to password handling — the reload-password preservation logic is pre-existing #3030 behavior, untouched by the rename.
Summary
Renames the string local
local.urltolocal.redirectUrlinside$buildRedirectUrl()so the unscopedurl.*reads added by #3036 resolve to the URL scope on every engine. #3036 layered sixStructKeyExists(url, "reload")/url.passwordreads into a function that has always assigned a string local namedurl(the redirect target, built fromcgi.path_info/cgi.script_name). On Adobe ColdFusion unscoped name resolution binds the bareurlreads to that string local before the URL scope and throwsScopeCastException("attempted to dereference a scalar variable of type ... java.lang.String as a structure") — an HTTP 500 fired beforeapplicationStop(), so every password-gated reload (?reload=true&password=...), every URL environment switch (?reload=<env>&password=...), the #3038allowEnvironmentSwitchViaUrl=truepath, andwheels reload(drives the same URL) became non-functional on Adobe. Lucee/BoxLang masked the bug becauseurlis a reserved scope that always wins. This is CLAUDE.md cross-engine anti-pattern #11 verbatim.The rename is applied to all four same-lineage copies of
public/Application.cfc(demo app,wheels newapp template, and thetweet+starter-appexamples), matching the parity contract enforced byReloadEnvironmentSwitchParitySpec.cfc. Behavior is otherwise identical on every engine — it is a pure local-variable rename.Refs #3053
Related Issue
Refs #3053
(Using
Refs, notFixes: the root-cause rename is complete and a structural regression guard is added, but the issue's runtime acceptance criteria —?reload=...&password=...returning 302-and-restart on adobe2023/2025, and the optional extension of the #3044 non-dev smoke probes to exercise the Adobe password-reload path — require the Adobe docker harness, which the bot's Lucee-only local runner cannot exercise. Those are left for CI's compat-matrix and human review to confirm before the issue is closed.)Type of Change
Feature Completeness Checklist
Signed-off-by:vendor/wheels/tests/specs/cli/BuildRedirectUrlScopeShadowSpec.cfc(failing → passing)bot-update-docs.yml)bot-update-docs.yml)bot-update-docs.yml); note the fix is already covered by existing anti-pattern New master #11changelog.d/3053-buildredirecturl-url-scope-shadow.fixed.mdTest Plan
TDD: failing → passing structural spec. The runtime repro requires the Adobe docker harness (the bug is invisible on Lucee, which the local SQLite runner uses), so the regression guard asserts the static defect is gone: no reserved-scope
local.urlmay coexist with unscopedurl.*reads inside$buildRedirectUrl()in any of the four copies. This is the "a reserved-scope shadowing in this file can't ship green again" coverage the issue asks for.wheels.tests.specs.clireturned HTTP 417; the 4 new specs failed withExpected [0] Actual [6](sixlocal.urlreferences in each$buildRedirectUrl()body), one per copy:cli/lucli/templates/app/public/Application.cfcpublic/Application.cfcexamples/tweet/public/Application.cfcexamples/starter-app/public/Application.cfcwheels.tests.specs.clireturns HTTP 200,totalFail = 0,totalError = 0(98 pass in the suite bundle); the 4 shadow specs pass and the siblingReloadEnvironmentSwitchParitySpec(app template: reload redirect strips the environment-switch params, making ?reload=<env> a silent no-op #3030/events: explicit set(allowEnvironmentSwitchViaUrl=true) is indistinguishable from the default — documented override impossible #3031 contract) is unaffected.?reload=true&password=...warm-up against the running Lucee 7 server returned 200 after the edit, confirming the demopublic/Application.cfcstill compiles and reloads cleanly (the fix: make URL environment switching work through the app reload restart redirect #3036/fix(events): honor explicit set(allowEnvironmentSwitchViaUrl=true) in production-like envs #3038 Lucee contract is preserved).For human/CI verification (Adobe runtime ACs, unreachable from the Lucee-only local runner):
?reload=true&password=<pw>→ 302 and the app actually restarts.?reload=testing&password=<pw>from development switches environment (302 preservesreload+password).set(allowEnvironmentSwitchViaUrl=true)in testing permits?reload=development&password=<pw>; unset stays blocked.