fix(mapper): reject redundant namespace prefix in to= and controller=#2794
Conversation
Inside `.namespace("foo")` (or equivalent `.scope()` / `.package()`),
writing `to="foo/dashboard##index"` instead of `to="dashboard##index"`
silently produced a `foo.foo/dashboard` controller path that downstream
got flattened to a `Foodashboard`-style class lookup with an opaque
`Wheels.ViewNotFound` error — leaving users to chase the symptom rather
than the route definition.
`$match()` now detects when the parsed controller starts with the scope's
package converted to slash form and throws `Wheels.MapperArgumentInvalid`
at registration time. The error names the namespace and the offending
value and points at the correct shorter form.
Fixes #2791
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: This PR fixes the silent-corruption bug from issue 2791 by rejecting a redundant namespace prefix at route-registration time rather than at first HTTP request. The implementation is correct, the six BDD specs are thorough, and there are no cross-engine compatibility concerns. One minor UX nit in the error message detail; everything else is clean. Verdict: comment (no blocking issues).
Correctness
The guard is placed in the right spot inside $match() - after the scope stack has set arguments.package (lines 297-303) and after to= has been split into controller + action (lines 306-310), so both code paths (to= and controller=) are covered by the same check.
Nit: misleading "likely from to=" hint when controller= is used directly (line 330)
vendor/wheels/mapper/matching.cfc line 330 always emits "likely from to=..." in the error detail, even when the caller passed controller= directly. By the time the guard runs, the to key has already been deleted from arguments (line 309), so there is no way to distinguish the two cases. The fix suggestion ("use controller=dashboard") is still correct, so this is not a blocker - but a user who called $match() with an explicit controller= argument would see the misleading hint.
Possible fix: snapshot local.fromTo = StructKeyExists(arguments, "to") before the parse block, then branch on it when building local.detail - emitting "passed as controller=" rather than "likely from to=" in the direct case.
Acceptable limitation (worth a comment)
The guard matches against the full accumulated package path (e.g. admin.users maps to admin/users/) but not a partial ancestor prefix. Inside .namespace("admin").namespace("users"), writing to="admin/profile" with just the outer-namespace prefix still silently produces a bad controller path. This is a deliberate scope choice appropriate for the reported issue; a short comment at the guard noting the limitation would help future readers.
Cross-engine
No issues found.
Left(arguments.controller, Len(local.prefix))at line 326 is safe on Lucee 7:local.prefixis alwayspackageAsPath & "/", soLen(local.prefix) >= 1always. The Lucee 7Left(str, 0)crash (CLAUDE.md invariant 8) cannot trigger.- No
struct.map()member-function collision. - No
applicationscope function members. - No inline closure as constructor named arg.
- CFML
==is case-insensitive on both Lucee and Adobe, so mixed-case redundant prefixes are correctly rejected.
Tests
Six BDD specs in MatchingSpec.cfc (lines 205-266) covering: to= error path, controller= error path, nested namespaces, .package() helper, near-miss prefix that must NOT be rejected, and canonical accepted form. The final spec asserts r[1].controller equals "datapai.dashboard", which verifies the downstream route struct rather than just the absence of an exception.
Commits
Single commit fix(mapper): reject redundant namespace prefix in to= and controller= - valid type (fix), valid scope (mapper), subject under 100 chars, DCO sign-off present. All good.
Docs
CHANGELOG.md entry under [Unreleased] > Fixed is thorough and names the new exception type. Framework doc updates deferred to bot-update-docs.yml as noted in the checklist.
|
Wheels Bot -- Reviewer B (round 1) A's review has body 'test' and zero line comments. This is a placeholder submission, not a real review. I've re-scanned the diff independently so the record is useful. Sycophancy: None of the usual markers (LGTM, approve, blank approvals of security/migration/DI changes). The problem is the opposite: zero content means zero analysis. False positives: None detected -- A made no claims at all. Missed issues (from independent diff re-scan):
Nothing in the diff touches security, auth, migrations, or the DI container -- no mandatory human-only hold needed on those grounds. Verdict alignment: A's verdict is COMMENTED with body 'test'. There is no verdict to be consistent or inconsistent with. Convergence: Not aligned. A has not provided any actual review content for me to agree or disagree with. A needs to submit a real review covering the detection logic in matching.cfc, cross-engine invariants (particularly Invariant 8), test coverage, and the CHANGELOG entry before convergence is possible. |
Wheels Bot — Reviewer B (round 2)Round 1 B critiqued a placeholder "test" submission. Review 4341412336 is a real, substantive review. A has addressed the prior "not aligned" stance by providing full analysis. I re-scanned the diff to verify A's claims independently. SycophancyNone detected. A gives a specific "comment" verdict backed by concrete findings rather than a blanket LGTM. False positivesNone detected. Every claim checks out against the actual diff:
Missed issuesMulti-line comment blocks violate CLAUDE.md style rules. CLAUDE.md: "Don't add error handling… multi-line comment blocks — one short line max."
Neither is a correctness issue, but both are concrete CLAUDE.md style violations A should have flagged. Verdict alignmentA's "comment" verdict (no blocking issues, one UX nit) is consistent with the findings. There is nothing in the diff that warrants ConvergenceAligned on verdict: both A and B agree this is "comment" territory — no blocking correctness, security, or cross-engine issues. The consensus changes are:
All three are minor, concrete, and safe to apply automatically. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A response (round 1)
Note: B's round-1 critique was based on a placeholder "test" review submitted at 23:26 UTC. My substantive review (containing the full analysis, cross-engine notes, and Lucee 7 confirmation) was submitted three minutes later at 23:29 UTC under the same SHA. B's independent re-scan is nonetheless valuable — two of B's four findings hold up, and I am conceding both. The third (Lucee 7 invariant) was explicitly covered in my actual review. The fourth (empty-package edge case) is a valid correctness concern I missed; conceding that too.
Conceded points
-
Multi-line block comment in
matching.cfc(7 lines, diff lines 312–319). CLAUDE.md is unambiguous: "Never write multi-line docstrings or multi-line comment blocks — one short line max." The guard's inline comment spans seven consecutive//lines. It should be collapsed to a single line referencing #2791, or removed entirely since theThrowmessage already explains the problem. -
Multi-line block comment in
MatchingSpec.cfc(4 lines, diff lines 200–203). Same rule, same violation. The four-line explanation above theit()group should be one line or cut entirely. -
Empty-package edge case (correctness, not style). If
arguments.packageis ever set to an empty string,local.prefixbecomes"/"and any controller path that starts with"/"would trigger a spuriousWheels.MapperArgumentInvalidthrow. The outerStructKeyExistscheck does not guard against a zero-length value. A minimal fix isLen(arguments.package) > 0added to the outerifcondition, plus a matching spec asserting that.namespace("")(or direct$match()withpackage="") is not falsely rejected.
Defended points
- Lucee 7
Left(str, 0)invariant (CLAUDE.md invariant 8). B noted this should have been confirmed explicitly. It was — in my actual review (PRR_kwDOABMUH88AAAABAsSx8A): "local.prefixis alwayspackageAsPath & "/", soLen(local.prefix) >= 1always. The Lucee 7Left(str, 0)crash cannot trigger." B's critique was written against the placeholder; the analysis was already in the record.
Updated verdict
Request-changes: the two multi-line comment blocks are CLAUDE.md violations that should be trimmed before merge, and the empty-package edge case (Len(arguments.package) > 0 guard missing) is a correctness gap that warrants a fix and a spec.
- Snapshot `local.fromTo` / `local.originalTo` before the `to=` parse block so the error detail can distinguish `to=` vs direct `controller=` callers (Reviewer A nit). - Add `Len(arguments.package) > 0` to the guard's outer condition so an empty package does not yield `prefix = "/"` and spuriously reject controllers whose path starts with a slash (Reviewer A response, Reviewer B round-1 missed-issue). - Collapse multi-line block comments above the guard in `matching.cfc` and above the new `it()` group in `MatchingSpec.cfc` to one-liners to comply with CLAUDE.md style (both reviewers). - Add a spec asserting `$match()` with `package = ""` and a controller starting with `/` is not falsely rejected. 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 2):
Verification: ran Skipped (intentionally not in consensus):
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 the updated round-1 state of the mapper redundant-prefix guard (PR for #2791). The second commit (fa83fc7e1) correctly addresses all three consensus findings: the empty-package edge case is now guarded, the multi-line block comment in matching.cfc is collapsed to one line, and the error detail now correctly distinguishes to= from direct controller= callers by snapshotting local.originalTo before the parse. Two minor items remain before merge: one 2-line comment block in the spec file that was not trimmed in round 1, and a local.hh value that produces a single # in the suggestion text where users need to type ##. Neither is a correctness bug; verdict: comment.
Conventions
1. 2-line comment block still present inside it() body — MatchingSpec.cfc lines 241–242
Round 1 correctly collapsed the 4-line header comment above the it() group, but the 2-line comment inside the "Allows controllers…" spec body was not touched:
it("Allows controllers whose name only happens to share a prefix with the namespace", function(){
// "foobar/dashboard" inside .namespace("foo") is NOT a redundant prefix
// — "foobar" != "foo". Must not be rejected.CLAUDE.md: "Never write multi-line docstrings or multi-line comment blocks — one short line max." Collapse to one line, e.g.:
// "foobar/dashboard" inside .namespace("foo") is not a redundant prefix — "foobar" != "foo".Correctness (UX nit)
2. local.hh = "##" produces a single # in the error suggestion — matching.cfc line 328
In a CFML string literal "##" evaluates to a single # character. The error detail therefore reads:
use to="dashboard#index" instead.
But users must type to="dashboard##index" in their CFML code to pass a string value containing a single #. The suggestion matches the runtime value, not the source text, so it could mislead a user who copies it verbatim and gets a CFML syntax error or a stray expression-delimiter warning.
Fix: use "####" (evaluates to ##) so the suggestion echoes exactly what to type:
local.hh = "####";
// detail now contains: use to="dashboard##index" instead.Confirmed resolved (round 1 → round 2)
- Empty-package edge case:
Len(arguments.package) > 0added to outer guard condition. ✓ - New spec "Does not falsely reject when package is empty" added. ✓
- 7-line block comment in
matching.cfccollapsed to one line. ✓ - 4-line header comment above spec group collapsed to one line. ✓
local.fromTo/local.originalTosnapshotted before theto=parse block; error detail now distinguishes the two call forms. ✓
Cross-engine
No new concerns introduced by the round-1 commit.
Left(arguments.controller, Len(local.prefix))at line 325:Len(arguments.package) > 0guaranteesLen(local.prefix) >= 2, so Lucee 7Left(str, 0)crash (CLAUDE.md invariant 8) cannot trigger. ✓- Case-insensitive
==on both Lucee and Adobe handles mixed-case redundant prefixes correctly. ✓
Commits
Both commits carry DCO sign-off and conform to commitlint.config.js (type fix, scope mapper, subject ≤ 100 chars). ✓
Wheels Bot — Reviewer B (round 1)A's review on the updated SHA is substantive and accurate. Both findings are verified against the diff. Converging with A on SycophancyNone detected. A's "comment" verdict is backed by two concrete, specific findings rather than a blanket LGTM. False positivesNone detected.
Missed issuesNone detected.
Verdict alignmentA's "comment" verdict (not ConvergenceAligned with A. Both findings are real, verified, and concrete enough for address-review: collapse the 2-line comment inside the "Allows controllers..." spec to one line, and change |
- vendor/wheels/mapper/matching.cfc:328 — change local.hh = "##" to local.hh = "####" so the error-suggestion detail renders as to="dashboard##index" (source-correct CFML), not to="dashboard#index" (Reviewer A finding, Reviewer B verified). - vendor/wheels/tests/specs/mapper/MatchingSpec.cfc:241–242 — collapse the 2-line comment inside the "Allows controllers..." spec body to a single line per CLAUDE.md "one short line max" rule (both reviewers). Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 2)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1 on the round-1 commit):
Verification: ran the full core suite against Lucee 7 / SQLite via the local test server. No findings were skipped — both consensus items were applied. 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: Round-2 review of the mapper redundant-prefix guard (fixes #2791). Both findings from the previous round — local.hh = "##" producing a single # in the error suggestion, and the 2-line comment block inside the "Allows controllers…" spec body — are correctly resolved in commit 092bde188. The three-commit history is now clean, all previous findings are confirmed closed, and no new issues were introduced. Verdict: approve (submitted as comment because GitHub blocks self-review on bot-authored PRs).
Confirmed resolved (round 2 → round 3)
1. local.hh = "##" → local.hh = "####" — matching.cfc line 328
"####" evaluates to ## at runtime, so the error-detail suggestion now renders as:
use to="dashboard##index" instead.
which is exactly the CFML source text the user must type (a string literal "dashboard##index" where ## escapes the separator #). The previous "##" evaluated to #, which would have produced to="dashboard#index" — syntactically valid CFML but carrying a stray expression-delimiter that would silently drop the action name at eval time. Fix is correct. ✓
2. 2-line comment collapsed — MatchingSpec.cfc lines 241–242
The two-line comment:
// "foobar/dashboard" inside .namespace("foo") is NOT a redundant prefix
// — "foobar" != "foo". Must not be rejected.is now:
// "foobar/dashboard" inside .namespace("foo") is not a redundant prefix — "foobar" != "foo".CLAUDE.md: "Never write multi-line docstrings or multi-line comment blocks — one short line max." Compliant. ✓
Residual checks (carried from rounds 1–2, still clean)
- Empty-package guard:
Len(arguments.package) > 0in outerif— preventslocal.prefix = "/"spuriously matching leading-slash controllers. ✓ local.fromTo/local.originalTosnapshotted before theto=parse block — error detail correctly distinguishes the two call forms. ✓- Single-line comment above guard in
matching.cfc. ✓ - Single-line comment above new spec group in
MatchingSpec.cfc. ✓ - 7 BDD specs covering:
to=rejection,controller=rejection, nested namespaces,.package(), near-miss prefix (must NOT reject), canonical accepted form, and empty-package false-positive. ✓
Cross-engine
No new concerns. Len(arguments.package) > 0 guarantees Len(local.prefix) >= 2, so the Left(arguments.controller, Len(local.prefix)) call cannot hit the Lucee 7 Left(str, 0) crash (CLAUDE.md invariant 8). CFML case-insensitive == correctly handles mixed-case redundant prefixes on both Lucee and Adobe. ✓
Commits
All three commits carry Signed-off-by: claude[bot], use type fix, scope mapper, subjects ≤ 100 chars, and are not ALL-CAPS. Commit bodies describe the why, not just the what. ✓
Docs
CHANGELOG.md entry under [Unreleased] > Fixed is thorough and names the new exception type. Framework docs deferred to bot-update-docs.yml as noted in the checklist. ✓
Wheels Bot — Reviewer B (round 1)A's round-3 review on SHA SycophancyNone detected. The "approve" verdict is backed by specific, cited line-level claims, not a blanket LGTM. False positivesNone detected.
Missed issuesNone detected. Re-scan found nothing A skipped:
Verdict alignmentA's "approve" (submitted as comment due to GitHub's self-review block) is consistent with the findings: no open correctness, cross-engine, or policy issues remain. ConvergenceAligned with A. All prior consensus changes were applied correctly. No further changes are needed from this review cycle. The PR is ready for human merge. |
…-redundant-to-foo-cont Resolves a CHANGELOG.md conflict: both branches added entries under [Unreleased] > Fixed. Kept all entries — the PR's #2791 mapper-prefix guard sits at the top of the section above the develop-side entries for #2790 (WheelsTest auto-bind), the model SELECT identifier quoting fix, the packages-install dispatch fix, the BrowserTest NotWired guard, and #2779 (BrowserTest base URL resolution). vendor/wheels/mapper/matching.cfc and the MatchingSpec are untouched by the merge. Signed-off-by: Peter Amiri <peter@alurium.com>
|
Merged Conflict was in
|
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The current head (b0f22adf5) is a merge commit that brings develop into the PR branch, resolving only a CHANGELOG.md conflict. The mapper guard code in matching.cfc and all seven BDD specs in MatchingSpec.cfc are explicitly noted in the commit message as untouched by the merge, and verified so by direct inspection. Every finding from rounds 1–3 remains resolved. No new issues were introduced. Verdict: approve (submitted as comment because GitHub blocks self-review on bot-authored PRs).
Confirmed merge resolution
The CHANGELOG.md conflict was resolved correctly: the PR's #2791 mapper-prefix guard entry sits at the top of [Unreleased] > Fixed, above the develop-side entries for #2790 (WheelsTest auto-bind), the model SELECT identifier quoting fix, the packages-install dispatch fix, the BrowserTest NotWired guard, and #2779 (BrowserTest base URL resolution). No entries were lost or reordered.
Confirmed resolved (rounds 1–3, still clean)
- Empty-package guard:
Len(arguments.package) > 0in outerifat line 319. ✓ local.fromTo/local.originalTosnapshotted before theto=parse block (lines 306–307); error detail correctly distinguishes the two call forms. ✓local.hh = "####"at line 328 evaluates to##at runtime, producingto="dashboard##index"in the suggestion text — exactly what the user must type. ✓- Single-line comment above guard in
matching.cfc(line 316). ✓ - Single-line comment inside the "Allows controllers…" spec body in
MatchingSpec.cfc(line 241). ✓ - 7 BDD specs covering:
to=rejection,controller=rejection, nested namespaces,.package(), near-miss prefix (must NOT reject), canonical accepted form, and empty-package false-positive. ✓
Cross-engine
No new concerns from the merge. Len(arguments.package) > 0 guarantees Len(local.prefix) >= 2, so Left(arguments.controller, Len(local.prefix)) cannot hit the Lucee 7 Left(str, 0) crash (CLAUDE.md invariant 8). ✓
Commits
The merge commit carries Signed-off-by: Peter Amiri <peter@alurium.com> — DCO present. The three bot-authored commits below it (42089da71, fa83fc7e1, 092bde188) all carry Signed-off-by: claude[bot], use type fix, scope mapper, subjects ≤ 100 chars, and are not ALL-CAPS. ✓
Wheels Bot — Reviewer B (round 1)A's review on the merge commit is accurate and complete. The diff is a single CHANGELOG.md entry resolving a conflict; all code in SycophancyNone detected. The "approve" verdict is backed by explicit, line-level verification of every previously-resolved finding — not a blanket LGTM. False positivesNone detected. All cited items are confirmed in the diff:
Missed issuesNone detected. The only new change is the CHANGELOG.md merge-conflict resolution, which A verified correctly. No security, migration, DI, or deploy-path code was touched. Commit DCO sign-offs and subject format are clean. Verdict alignmentA's "approve" verdict is consistent with the findings: a merge-only commit whose sole change is an accurate CHANGELOG entry, with all previously-approved code intact. Appropriate. ConvergenceAligned with A. The PR is review-clean on this SHA. No further changes needed — ready for human merge. |
* chore: bump develop snapshot target to 4.0.2 (#2770)
Manual bump after the v4.0.1 GA — `bump-develop-version.yml` fired via
`repository_dispatch` (the #2609 fix worked) but failed in 12s on a
second issue: `peter-evans/create-pull-request@v6` hit
`remote: Duplicate header: "Authorization"` because the `actions/checkout`
step left credentials persisted that conflict with the action's own token.
See run 26173817714 for the failed log.
Setting `wheels.json` to `4.0.2` so subsequent develop snapshots are
tagged `4.0.2-snapshot.<run>`. This is a baseline, not a commitment —
the next GA's scope decision is made at tag-cut time.
Follow-up issue tracks the workflow fix (add `persist-credentials: false`
to the checkout step).
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(release): fix bump-develop-version.yml duplicate Authorization header (#2771)
`actions/checkout@v6` defaults to `persist-credentials: true`, which
writes `http.https://github.com/.extraheader = AUTHORIZATION: basic <GITHUB_TOKEN>`
to the local `.git/config`. `peter-evans/create-pull-request@v6` then
sets its own `extraheader` for the dispatch token, and the next git
operation sends both Authorization headers — GitHub returns HTTP 400
with `remote: Duplicate header: "Authorization"`.
First observed on the v4.0.1 GA (2026-05-20, run 26173817714); manual
workaround was #2770. Setting `persist-credentials: false` keeps
peter-evans/create-pull-request as the sole Authorization authority.
This is a documented peter-evans/create-pull-request gotcha when the
caller uses a non-default token.
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(web/blog): Wheels 4.0.1: Adobe CF hardening, Windows Scoop fixes, and the post-GA shakeout (#2772)
Walks through the ~100 PRs that landed between 4.0.0 and 4.0.1: Adobe CF
2023/2025 attributeCollection + onError + env() + Vite asset-walk chain,
the Windows Scoop wheels.cmd cmd.exe pre-parser fix, paginationNav()
viewStyle presets, whereIn([]) short-circuit, CORS preflight/Vary/multi-
origin fixes, plural mappings, Oracle bulk-insert + CockroachDB advisory
locks, BoxLang adapter fixes, deploy CLI hardening, and Rocky Linux RPM
fixes from the titan production cutover.
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): guard application.wo in onError so init failures don't cascade (#2774)
* fix(cli): guard application.wo in onError so init failures don't cascade
When the Wheels Injector fails to load during onApplicationStart (a stale
/wheels mapping under Lucee Express 7 is the symptom users hit on the
"Your First 15 Minutes" tutorial), application.wo is never assigned. The
existing recovery try/catch inside onError swallows a second failure
silently and then unconditionally calls application.wo.$getRequestTimeout(),
which throws "The key [WO] does not exist." and replaces the real
diagnostic with a cryptic cascade.
Add a StructKeyExists(application, "wo") guard right after the recovery
try/catch in cli/lucli/templates/app/public/Application.cfc (the template
behind `wheels new`) and the demo public/Application.cfc. When the global
isn't there, render a minimal HTML error page and return — the user sees
"Wheels failed to initialize" plus the original exception message instead
of the cascade.
Fixes #2773
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(web/guides): note application error fallback and init failure in troubleshooting docs
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(cli): address Reviewer A/B consensus findings (round 1)
- Set HTTP 500 status code in the onError fallback in both
cli/lucli/templates/app/public/Application.cfc and public/Application.cfc
so monitoring tools and CDNs don't cache the Wheels-init failure as
a successful response. Uses a plain struct for cfheader's
attributeCollection per CLAUDE.md cross-engine invariant #10
(Adobe CF 2023/2025 reject the arguments scope on built-in tags).
- Document the no-nested-braces assumption behind catchClosePattern in
vendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc so a future
edit that adds nested braces inside the outer catch knows why the
silent fallback to scanFrom=1 is the safety net.
- Fix the contradictory recovery steps in the first-15-minutes guide
(wheels reload requires a running server) at
web/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx.
- Replace the speculative "pre-4.0.2" wording in
.ai/wheels/troubleshooting/common-errors.md with "4.0.1 or earlier"
since the fix is still in [Unreleased].
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* chore(web): refresh visual baseline(s) (all)
Manually triggered baseline refresh via
.github/workflows/refresh-visual-baselines.yml
on branch fix/bot-2773-first-15-minutes-tutorial-fails-the-key-wo-does-no.
Run when an intentional content/layout change makes the visual-regression
check fail. The new PNG(s) under web/tests/visual-baselines/ are now the
expected rendering; re-run the failing visual-regression job to flip the
check green.
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix(ci): stop double-nesting framework inside Linux .deb/.rpm packages (#2776)
* fix(ci): stop double-nesting framework inside Linux .deb/.rpm packages
The nfpm contents rule pointed `src` at `./build/framework/` for the
framework staging step. `wheels-core-VER.zip` carries a top-level
`wheels/` directory that `unzip` preserves, so the resulting tree was
`./build/framework/wheels/...`. nfpm `type: tree` copies *contents* of
src into dst, which meant the inner `wheels/` wrapper itself landed at
the destination — producing
`/opt/wheels/module/vendor/wheels/wheels/Injector.cfc` instead of
`/opt/wheels/module/vendor/wheels/Injector.cfc`.
After the user-side wrapper sync (`/opt/wheels/module/*` →
`~/.wheels/modules/wheels/*`) and `wheels new <app>` copy, every fresh
Linux install ended up with the framework one directory level too deep.
Lucee's `/wheels` mapping pointed at the (empty) outer directory, so
`new wheels.Injector("wheels.Bindings")` in the generated
`public/Application.cfc` threw `could not find component or class with
name [wheels.Injector]` on the first request. The existing onError
handler then dereferenced `application.wo` (which was never assigned
because Injector init failed), surfacing only the cryptic cascade
`The key [WO] does not exist.` — issue #2773.
The brew formula handles this correctly by re-introducing the wheels/
wrapper at stage time
(`(share/"wheels/framework/wheels").install Dir["*"]`). Both Linux nfpm
configs now pin `src` at `./build/framework/wheels/` so the contents
flatten into `/opt/wheels/module/vendor/wheels/` as intended.
The published 4.0.1 .deb / .rpm artifacts ship the broken layout (1
.deb download, 0 .rpm at time of fix). A re-released 4.0.2 will be
needed to deliver the fix to users — the change here is to the build
config only, not to any framework or CLI code.
Tests: `vendor/wheels/tests/specs/cli/LinuxPackageStagingSpec.cfc`
gains a per-channel `it()` that asserts `src: ./build/framework/wheels/`
+ `dst: /opt/wheels/module/vendor/wheels/` are paired in each nfpm
yaml. Structural assertion follows the existing #2700 pattern (the
file already pins four other packaging invariants the same way).
Note on local verification: the structural spec was sanity-checked via
equivalent grep / perl POSIX patterns over the YAMLs (positive match
for the fixed form, zero matches for the buggy form). Running the
spec through the CFML runner locally was blocked by a port-8081
collision with two stale wheels server processes from prior dev
sessions — CI compat-matrix will run the spec across every engine ×
DB on this PR.
Closes #2773
Signed-off-by: Peter Amiri <peter@alurium.com>
* test(ci): add negative guard for buggy framework src in nfpm yamls
Reviewer A on PR #2776 (wheels-bot) flagged that the new framework-src
spec only asserted the *fixed* form was present, without a matching
`toBeFalse` for the buggy `./build/framework/` form. The file's existing
wrapper-routing checks (lines 60-68 / 81-106) already use a dual-
assertion pattern; the new spec was a one-sided outlier.
Add the negative guard: if a future copy-paste leaves both the bare
`src: ./build/framework/` and the fixed `src: ./build/framework/wheels/`
in the same yaml, nfpm would stage both — the bare one reintroduces
the double-nesting and breaks every fresh Linux install. The spec now
fails loudly in that scenario instead of silently passing on the
presence of the fixed entry.
The two regexes are mutually exclusive by construction: the positive
matches `framework/wheels/` followed by whitespace + `dst:`; the
negative matches `framework/` followed *immediately* by whitespace +
`dst:`. Since `wheels` isn't whitespace, `[[:space:]]+` can't bridge
across it, so the negative regex cannot false-positive on the fixed
form. Confirmed via perl POSIX equivalent against both nfpm yamls
plus a synthetic buggy fixture.
Also adds an inline comment to the positive assertion documenting why
`[[:space:]]+` works across the YAML line break (POSIX `[[:space:]]`
resolves to Java's `\s` in both Lucee and Adobe CF, which includes
`\n`) — addresses Reviewer A's Nit 2 observation that the cross-line
match hadn't been locally verified.
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs(web/guides): correct Linux bleeding-edge install URLs to wheels-be_* (#2777)
* docs(web/guides): correct Linux bleeding-edge install URLs to wheels-be_*
PR #2759 (2026-05-18) renamed the snapshot Linux artifacts from `wheels_*`
to `wheels-be_*` (debs) and `wheels-be-*.x86_64.rpm` (rpms) so the package
name itself differentiates the channel. The install guides were not
updated alongside that rename, so every documented `curl -fsSLO ...`
command for Linux bleeding-edge install resolves to a 404 against the
actual snapshot release assets.
Verified against v4.0.2-snapshot.1923 (published 2026-05-20):
Guide says: .../wheels_4.0.2.snapshot.1923_amd64.deb → 404
Actual asset: .../wheels-be_4.0.2.snapshot.1923_amd64.deb
Fix all six pages where the snippets / prose examples appear (three
unique pages mirrored across v4-0-0 and v4-0-1-snapshot doc versions):
start-here/installing.mdx — "Want bleeding-edge?" aside
start-here/release-channels.mdx — main BE install snippets +
"Switching channels" snippets
+ tilde-mangling prose
command-line-tools/installation.mdx — bleeding-edge install snippets
The substitutions are scoped to bleeding-edge contexts (snippets using
`${SNAP_FILENAME_VER}` and prose `wheels_4.0.0.snapshot.*` filename
examples). Stable-channel snippets, which use `${WHEELS_VERSION}` and
fetch from `wheels-dev/wheels` (not `wheels-snapshots`), are unchanged —
they correctly retain the bare `wheels_` / `wheels-` prefixes because
the stable package name on Linux is still just `wheels`.
Without this fix, users cannot install or test bleeding-edge / develop
snapshots on Linux via the documented flow. This blocks user-side
verification of develop-only fixes before they ship in the next stable
patch — including PR #2776 (Linux .deb framework nesting fix) and
PR #2774 (defensive onError guard), both of which close issue #2773.
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs(web/guides): fix release-channels.mdx — missed BE Tab URLs + Linux switching semantics
Round-1 reviewer findings on PR #2777:
A's Nit 1 — primary install Tabs at lines 104-105 (Debian/Ubuntu BE)
and 112 (Fedora/RHEL BE) of `release-channels.mdx` still resolved to
404. My initial verification sweep grep'd for `${SNAP_FILENAME_VER}`,
but these snippets bind the tag to `${WHEELS_FILENAME_VER}` (a
different bash var name). The fix is the same — point at the
`wheels-be_` / `wheels-be-` artifacts.
A's Nit 2 + B's catch — the "Switching channels" section had three
related staleness bugs after #2759 renamed the BE package:
1. Line 129 prose claimed "only a single package name (`wheels`)
is published per channel today" — false post-rename.
2. Lines 142-143 inline comment ("upgrades in place — no uninstall
step needed") was true when both channels shared the `wheels`
name, but the new world depends on the actual nfpm-declared
`Replaces:` / `Conflicts:` metadata. B caught the contradiction
between A's proposed line-129 prose and the existing line-142
comment.
3. Lines 158-172 (Linux BE → stable, both Debian and Fedora) had
the *same* conceptual bug as 142-143: they prescribed
`--allow-downgrades` (apt) / `dnf downgrade`, both of which
assume same-package-name version transitions. With different
names, both would fail with a `/usr/bin/wheels` file conflict
because the stable `wheels` package doesn't declare
`Replaces:`/`Obsoletes: wheels-be`. Reviewers didn't explicitly
flag this set, but it's the same root cause and listing them
inconsistently would have left readers worse off.
Verified the actual nfpm metadata before rewriting (so the prose
matches what the packages really declare):
wheels-be deb: Replaces: wheels + Conflicts: wheels
wheels-be rpm: Conflicts: wheels (no Obsoletes)
wheels deb: no Replaces/Conflicts against wheels-be
wheels rpm: no Conflicts/Obsoletes against wheels-be
The new prose at line 129 explains the asymmetry up front; each
snippet now carries a short comment naming the specific metadata
that drives its action (or the lack of metadata that requires the
explicit `apt remove` / `dnf remove`).
Stable-channel snippets and stable install Tabs are unchanged.
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs(web/guides): name the actual Conflicts declaration in BE→stable comments
Reviewer A round-2 nit on PR #2777: the BE → stable (Debian) snippet's
comment said apt "would fail with a /usr/bin/wheels file conflict",
framing the failure mode as a dpkg-level file-ownership conflict. The
actual blocker is the package-level `Conflicts: wheels` declaration in
wheels-be's deb metadata — apt refuses the install with a package
conflict error before dpkg ever attempts to unpack files. An advanced
user debugging the actual error message would be confused by the
file-conflict framing.
Rewrite the Debian comment per A's suggestion, naming the actual
mechanism: `wheels-be declares Conflicts: wheels`. Kept the secondary
note about the missing `Replaces: wheels-be` in stable since it
explains why apt also wouldn't auto-remove (relevant context if a
reader wonders whether a single command could swap them).
Updated the Fedora BE → stable comment to use parallel framing for
consistency — same root cause (`wheels-be` declares `Conflicts:
wheels`, applies bidirectionally on rpm too). Reviewer A only flagged
the Debian site explicitly, but leaving the two comments inconsistent
would have invited the same "two sites must agree" finding that
caught round 1's line-142 / line-129 contradiction.
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(test): BrowserTest reports unwired this.browser with browserDescribe() hint (#2782)
* fix(test): BrowserTest reports unwired this.browser with browserDescribe() hint
Plain describe() blocks inside BrowserTest subclasses left this.browser
as an empty string, so the first DSL call surfaced as
"function [visitUrl] does not exist in the String" — a misleading error
that hits every newcomer on iteration 1. Install an UnwiredBrowserGuard
sentinel at this.browser before browserDescribe() wires a real
BrowserClient (and after $endBrowserContext tears it down) so any method
call throws Wheels.BrowserTest.NotWired with a message naming
browserDescribe() as the fix.
Fixes #2778
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(web/guides): note Wheels.BrowserTest.NotWired when describe() used instead of browserDescribe()
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* chore: rename Phase 2 bucket repos to apt-wheels / yum-wheels for naming consistency (#2788)
Phase 2 Linux native-repo templates and dispatch wiring referenced
`wheels-dev/apt-wheels-dev` and `wheels-dev/yum-wheels-dev`. The `-dev`
suffix appeared to mirror the DNS form (`apt.wheels.dev`
→ `apt-wheels-dev`) but reads as a redundant org echo inside the
`wheels-dev` org and breaks the established `<package-manager>-wheels`
naming used by the other sister repos (`homebrew-wheels`,
`scoop-wheels`, `chocolatey-wheels`).
Rename everywhere to drop the `-dev` suffix:
- wheels-dev/apt-wheels-dev → wheels-dev/apt-wheels
- wheels-dev/yum-wheels-dev → wheels-dev/yum-wheels
The actual bucket repos were just created under the new names today
(2026-05-21 ~20:15 UTC), so this PR brings the templates / docs /
release-workflow dispatch in sync with the on-GitHub reality before
the first end-to-end dispatch fires. No live infrastructure references
the old names yet — Cloudflare Pages, DNS, and the bucket-side CI
secrets all post-date this rename.
Mechanical substitution across 9 files (27 references). Stable `wheels`
package name (the bare `wheels` in nfpm configs and `apt install
wheels` snippets) is untouched — only the org-namespaced repo names
change.
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(test): resolve BrowserTest base URL through layered lookup at instance time (#2783)
* fix(test): resolve BrowserTest base URL through layered lookup at instance time
Specs running against a non-default port (Titan on 60050, scaffolds on
60080) previously had to compare getBaseUrl() against a sentinel string
and override it manually because $resolveBaseUrl() returned
http://localhost:8080 unconditionally and the only escape hatch
(WHEELS_BROWSER_TEST_BASE_URL) is cached by the JVM at process start.
$resolveBaseUrl() now consults, in order: this.baseUrl per-spec override,
get("browserTestBaseUrl") Wheels setting, -Dwheels.browserTest.baseUrl
JVM property, WHEELS_BROWSER_TEST_BASE_URL env, $detectBaseUrlFromCgi()
auto-detect (the test runner reaches the suite over HTTP, so cgi already
names the right host:port), then the localhost:8080 default. The CGI
auto-detect skips when port==8080 so existing default-port runs are
unchanged.
Fixes #2779
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(web/guides): document BrowserTest layered base-URL resolution (#2779)
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(test): address Reviewer A/B consensus findings (round 1)
- Strip canonical ports (http:80, https:443) in $detectBaseUrlFromCgi to
match URL conventions; update the https:443 spec expectation and add a
dedicated http:80 case (vendor/wheels/wheelstest/BrowserTest.cfc:297-299,
vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc:51-61).
- Document why the "falls back through layers" assertion is intentionally
weak — JVM env vars are read-only from CFML and the Wheels get() setting
needs a live framework context, so layer isolation isn't fully testable
at that level (vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc:28-43).
- Update browser-test guides (v4-0-0 + v4-0-1-snapshot L319) to recommend
setting this.baseUrl in the component pseudo-constructor instead of
beforeAll — super.beforeAll() calls $resolveBaseUrl() before a
beforeAll-override can take effect, silently inheriting layer 2-6 results.
- Mirror the same ordering note in .ai/wheels/testing/browser-testing.md.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
* fix(cli): make `wheels packages install` a real alias for `add` in dispatch (#2786)
* fix(cli): make `wheels packages install` a real alias for `add` in dispatch
The `case "install":` branch in `Module.cfc::packages()` previously printed
a warning to stdout and returned an empty string instead of installing
anything. That was wrong for every caller path that actually reaches
module dispatch — the stdio MCP server, scripted in-process clients, and
the spec suite — because `PackagesMainCli.install()` itself has been a
transparent alias for `add()` since #2729. The dispatch layer was the
only place where the alias broke.
The shell-facing `wheels packages install <name>` is still intercepted
by LuCLI's built-in extension installer upstream of module dispatch and
remains broken on that path (documented in the module-owned `--help`
text). This change only fixes the paths that LuCLI does NOT intercept.
Both verbs now share a single fall-through case body so validation,
error shape, and install behavior cannot drift apart again.
Fixes #2785
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(web/guides): clarify install-as-alias behavior in packages CLI section
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(cli): address Reviewer A/B consensus findings (round 1)
- PackagesCommandSpec: add `expect(installResult.type).toBe(addResult.type)`
after the existing `.notToBe("")` assertion so the equivalence claim in
the surrounding comment is actually enforced. A regression where
`install` throws at argument validation (before the registry call)
would have satisfied `.notToBe("")` but diverged from `add`'s shape;
the new assertion pins it.
- CHANGELOG: terminal period on the new `[Unreleased] / ### Fixed`
entry for consistency with surrounding entries.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(web/guides): address Reviewer A/B consensus findings (round 2)
- web/sites/guides/src/content/docs/v4-0-0/digging-deeper/packages.mdx
(line 320) — scope the install-as-alias note to v4.0.1+. The previous
wording asserted the alias was transparent on MCP / in-process paths,
but that's only true after this PR (which targets v4.0.1). On v4.0.0
itself, MCP also no-ops; the versioned v4.0.0 docs now say so
explicitly and point readers to the v4.0.1 snapshot for the alias
behavior. The v4-0-1-snapshot/ copy was already correct and is
untouched.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
* fix(model): quote column identifiers in SELECT clause builder (#2787)
* fix(model): quote column identifiers in SELECT clause builder
The WHERE and ORDER BY clause builders already routed column names
through the adapter's $quoteIdentifier, but $createSQLFieldList — the
SELECT/GROUP BY engine — appended the column part raw. Models backed
by tables with reserved-word column names (e.g. `key`, `order`,
`group`) blew up on `findAll`/`findOne`/dynamic finders with cryptic
SQL syntax errors as soon as the SELECT list mentioned the column.
Also strips quote chars from the property extracted by the
duplicate-column rename loop so the alias replacement still matches
the unquoted ` AS <alias>` form, and updates the empty-pagination
columnList extraction in read.cfc to strip identifier quotes before
stripping the table prefix.
Fixes #2784
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(web/guides): note reserved-word column support via property alias in models guide
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(model): address Reviewer A/B consensus findings (round 1)
- Condense 4-line block comments at vendor/wheels/model/read.cfc:217
and vendor/wheels/model/sql.cfc:634 to single-line comments
(CLAUDE.md: "Never write multi-paragraph docstrings or multi-line
comment blocks — one short line max").
- Stop using $quoteColumn() for the table-name argument in
vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc;
switch to the model's public $quotedTableName() helper so the spec
names match what each helper actually quotes.
- Add a zero-row paginated findAll spec to
reservedColumnQuotingSpec.cfc that exercises the QueryNew branch in
vendor/wheels/model/read.cfc:225 with an aliased column, covering
the path the original spec did not reach.
- Mention ORDER BY alongside SELECT and GROUP BY in
web/sites/guides/src/content/docs/v4-0-1-snapshot/basics/models-and-the-orm.mdx
so readers do not infer ORDER BY is unsafe with reserved-word
columns (ORDER BY already routes through $quoteIdentifier).
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(model): address Reviewer A/B consensus findings (round 2)
Condense the remaining multi-line comment blocks in reservedColumnQuotingSpec.cfc
to single lines per CLAUDE.md ("Never write multi-paragraph docstrings or
multi-line comment blocks — one short line max"):
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:10 — 3-line
block about City's id -> countyid alias condensed.
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:18 — 3-line
block about Author.firstName (property == column) condensed.
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:40 — 6-line
block added in round 1 inside the zero-row pagination it() condensed to
the single-line form Reviewer A supplied.
The line-30 GROUP BY comment was already single-line; A's "30-32" citation
was off-by-one for that one. No production code changed; pure comment-style
fix. Test totals unchanged at 4 pass / 0 fail in the spec; full model suite
remains 839 pass / 0 fail / 0 error / 11 skipped across 35 bundles.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
* fix(test): auto-bind include-injected globals into WheelsTest spec scope (#2793)
* fix(test): auto-bind include-injected globals into WheelsTest spec scope
`WheelsTest`'s pseudo-constructor used `getMetaData(application.wo).functions`
to discover which Wheels globals to copy into a spec's `variables`/`this`
scope. That metadata enumerates only methods declared on the CFC body and
silently skips symbols merged in via `cfinclude` — which is how
`vendor/wheels/Global.cfc` pulls user helpers from `app/global/functions.cfm`.
Apps with custom helpers (`can()`, `hasRole()`, etc.) had to manually rebind
each one in `beforeAll()`.
The loop now iterates `application.wo` as a struct and binds every UDF
detected by `isCustomFunction()`, while preserving the existing
public-only filter for metadata-declared methods and the don't-clobber
guard for scope members the spec (or its base class) already provides.
Fixes #2790
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(test): address Reviewer A/B consensus findings (round 1)
- Promote include-injected UDFs from `variables` to `this` in
`vendor/wheels/Global.cfc` after `include "/app/global/functions.cfm"`,
so the auto-bind loop in `vendor/wheels/WheelsTest.cfc` discovers them
uniformly across Lucee, Adobe CF, and BoxLang. Lucee's struct-iteration
over a CFC instance surfaces both `this` and `variables` scopes, but
Adobe CF only reliably exposes `this`-scope members — without the
promotion, the original bug (#2790) would silently persist on Adobe CF
even with the new iteration path in WheelsTest.cfc.
- Fix the misleading header comment in
`vendor/wheels/tests/specs/wheelstest/WheelsTestAutoBindIncludesSpec.cfc`.
Bracket-notation assignment from outside writes to `this` scope, not
`variables` — so the probe simulates the post-promotion shape, not the
raw include shape. Comment now spells this out explicitly.
- Add a new `it` case that asserts the probe key is enumerated by
`for (key in application.wo)`. Guards the iteration mechanism the
auto-bind loop depends on, so failures on any engine where struct-
iteration is narrower than expected would fail this spec rather than
silently pass-but-not-test downstream.
Addresses Reviewer A's cross-engine concern (Adobe CF struct-iteration
contract) and Reviewer B's joint recommendation option (b): promote
include-injected helpers to `this` so the iteration path is uniform.
The accompanying spec correction handles A's "spec injects via wrong
scope" finding and B's "misleading header comment" note.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(test): seed `local` scope before pseudo-constructor for-iterator
The promotion loop added in round 1 of the consensus fixup crashed
Lucee 7 with `variable [local] doesn't exist` at Global.cfc:3861 —
the test runner couldn't even reach a spec before bailing out.
In a CFC pseudo-constructor (component body, not inside a function),
the `local` scope is not auto-created. Direct assignment to
`local.X = ...` will seed it, but `for (local.X in Y)` tries to read
`local` first as the iterator's target parent and fails. WheelsTest.cfc
gets away with the same loop shape only because it does
`local.metaIndex = {}` earlier in its own pseudo-constructor; Global.cfc
had no such seeding line.
Add the minimum seeding statement (`local.varKey = "";`) directly above
the loop and document the cross-engine reason inline. The loop's
filter logic is unchanged.
The original review couldn't catch this — the round-1 address-review
sandbox lacked a working test runner so the fix went out unverified.
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
* fix(mapper): reject redundant namespace prefix in to= and controller= (#2794)
* fix(mapper): reject redundant namespace prefix in to= and controller=
Inside `.namespace("foo")` (or equivalent `.scope()` / `.package()`),
writing `to="foo/dashboard##index"` instead of `to="dashboard##index"`
silently produced a `foo.foo/dashboard` controller path that downstream
got flattened to a `Foodashboard`-style class lookup with an opaque
`Wheels.ViewNotFound` error — leaving users to chase the symptom rather
than the route definition.
`$match()` now detects when the parsed controller starts with the scope's
package converted to slash form and throws `Wheels.MapperArgumentInvalid`
at registration time. The error names the namespace and the offending
value and points at the correct shorter form.
Fixes #2791
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(mapper): address Reviewer A/B consensus findings (round 1)
- Snapshot `local.fromTo` / `local.originalTo` before the `to=` parse
block so the error detail can distinguish `to=` vs direct `controller=`
callers (Reviewer A nit).
- Add `Len(arguments.package) > 0` to the guard's outer condition so an
empty package does not yield `prefix = "/"` and spuriously reject
controllers whose path starts with a slash (Reviewer A response,
Reviewer B round-1 missed-issue).
- Collapse multi-line block comments above the guard in `matching.cfc`
and above the new `it()` group in `MatchingSpec.cfc` to one-liners
to comply with CLAUDE.md style (both reviewers).
- Add a spec asserting `$match()` with `package = ""` and a controller
starting with `/` is not falsely rejected.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(mapper): address Reviewer A/B consensus findings (round 2)
- vendor/wheels/mapper/matching.cfc:328 — change local.hh = "##" to
local.hh = "####" so the error-suggestion detail renders as
to="dashboard##index" (source-correct CFML), not to="dashboard#index"
(Reviewer A finding, Reviewer B verified).
- vendor/wheels/tests/specs/mapper/MatchingSpec.cfc:241–242 — collapse
the 2-line comment inside the "Allows controllers..." spec body to a
single line per CLAUDE.md "one short line max" rule (both reviewers).
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
* fix(events): re-include app/global/*.cfm on bare ?reload=true when files change (#2795)
* fix(events): re-include app/global/*.cfm on bare ?reload=true when files change
Adding a helper to `app/global/functions.cfm` (or anything it `<cfinclude>`s)
used to require the password-gated `?reload=true&password=...` path. Bare
`?reload=true` re-ran config and routes but left `application.wo` (the
`Global.cfc` instance) intact, so the symbols merged into its variables scope
at construction time stayed frozen — the page rendered without error and the
new helper was silently undefined.
The fix follows the Rails/Phoenix per-request mtime-check pattern recommended
by the research comment: snapshot `app/global/*.cfm` mtimes on application
start, and on bare `?reload=true` in development re-evaluate the include if
any tracked file has been added, removed, or touched. The password-gated
`applicationStop()` path still does a full re-init unchanged — this just
makes the muscle-memory path actually work.
Three new helpers on `wheels.Global`:
- `$snapshotGlobalIncludes(directory)` — struct of `path → dateLastModified`
- `$globalIncludesChanged(snapshot, directory)` — diff against current state
- `$reincludeGlobals(file)` — re-evaluate the include against the live
Global instance
New setting `reloadOnGlobalChange` defaults to `true` in development and
`false` everywhere else; opt out with `set(reloadOnGlobalChange=false)`.
Fixes #2792
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(web/guides): document reloadOnGlobalChange setting and bare reload behavior
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(events): address Reviewer A/B consensus findings (round 1)
- Replace `DirectoryCreate(baseDir, true)` with `java.io.File.mkdirs()`
in `reloadGlobalsSpec` — the createPath flag is Lucee-only and the
call lives in beforeEach, so every spec in the group fails on Adobe CI.
- Assert `IsDefined("application.wo.fxReinclude")` after each
`$reincludeGlobals` call so the contract (re-included helpers must
remain callable on application.wo) is guarded against silent no-ops.
- Lift include-declared functions from local/variables onto this in
`$reincludeGlobals` so the contract holds across engines: include
inside a method body would otherwise leave declarations in scopes
that aren't reachable via `application.wo.helper()`.
- Wrap the bare ?reload=true re-include in a double-checked
`lock name="wheels_reload_globals"` to eliminate the race between
two concurrent ?reload=true hits.
- Document the development-only environment guard as intentional so a
future maintainer doesn't try to enable `reloadOnGlobalChange` in
staging and debug a silent no-op.
- Add docblocks to `$globalIncludesChanged` and `$reincludeGlobals` so
all three new global-includes helpers carry consistent documentation.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(events): address Reviewer A/B consensus findings (round 2)
- vendor/wheels/Global.cfc:$reincludeGlobals — drop the
!ArrayFind(beforeVars, key) snapshot-diff guard from the second loop.
On Adobe CF the include updates variables[key] in place across calls,
so the guard silently skipped re-binding the updated function onto
`this` on the second ?reload=true. Re-lifting is idempotent and the
path is development-only.
- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc:88 — replace
the second IsDefined assertion with a return-value check so a stale
Adobe-CF binding cannot slip past CI (expect(...fxReinclude()).toBe("second")).
Local verification on Lucee 7 + SQLite after server reload:
core suite 3698 pass / 0 fail / 0 error; global suite 113 pass / 0 fail
/ 0 error (the 7 reloadGlobals specs all green).
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(test): address Reviewer A/B consensus findings (round 3)
- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc — hoist
`g` and `baseDir` out of the `describe()` arrow-function callback
and into `run()` as a shared `ctx` struct, then update every
beforeEach / afterEach / it closure to reference `ctx.g` and
`ctx.baseDir`. On Adobe CF 2023/2025 CFML closures cannot reach
an enclosing function's `local` scope (CLAUDE.md cross-engine
invariant ##3); the prior layout silently relied on Lucee 7's
lexical capture and would have thrown "variable baseDir is
undefined" inside every nested closure on Adobe CI, crashing all
seven specs.
Local verification on Lucee 7 + SQLite (existing test server,
forced ?reload=true&password=wheels first):
global directory (wheels.tests.specs.global): 113 pass / 0 fail / 0 error
full core suite: 3698 pass / 0 fail / 0 error
Lucee was already green before this fix because it captures the
enclosing arrow-function `local`; the change is to make the Adobe
CI legs match. Adobe verification is left to CI as the local
harness cannot run Adobe.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix: address Reviewer A/B consensus findings (round 4)
- web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/dev-server.mdx — tighten the bare `?reload=true` note so it mentions `app/global/functions.cfm` (and any files it `<cfinclude>`s) instead of the looser `app/global/*.cfm`. `$globalIncludesChanged` watches every `*.cfm` under `app/global/` but `$reincludeGlobals` re-evaluates only `functions.cfm` and the files it transitively includes; the previous wording implied a developer could drop a standalone helper file directly and have it bind, which isn't true. Matches the more accurate wording already in `core-concepts/environments-and-configuration.mdx`.
- web/sites/guides/src/content/docs/v4-0-1-snapshot/command-line-tools/wheels-commands/dev-server.mdx — same wording change for the v4-0-1 snapshot copy, keeping the two doc trees in sync.
- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc — drop the dead `mappingPath` local variable; it was only used to compute `absPath`, and the `$reincludeGlobals` calls inside the `notThrows` closures already repeat the path literally (closures can't reach the enclosing `local.mappingPath` on Adobe CF anyway). Inlines the string into `ExpandPath()` directly.
Local verification on Lucee 7 + SQLite:
global directory (wheels.tests.specs.global): 113 pass / 0 fail / 0 error
reloadGlobalsSpec only: 7 pass / 0 fail / 0 error
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* fix(events): address Reviewer A/B consensus findings (round 5)
- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc — add a test
for the DateCompare != 0 branch in $globalIncludesChanged, the
"developer edited an existing helper" path the PR is designed to
serve. Uses the backdated-snapshot approach from Reviewer A's
response (DateAdd seconds -60 on the snapshot entry) rather than
Sleep(1100), so the test is deterministic across filesystems with
different mtime granularities.
- vendor/wheels/events/EventMethods.cfc — append application name to
the wheels_reload_globals lock so concurrent ?reload=true hits from
different apps on a shared Adobe CF server no longer serialize on a
single global lock. Development-only and uncommon in practice, but
the fix is a one-liner.
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs: tighten CHANGELOG entry to distinguish detection from re-evaluation scope
- CHANGELOG.md — round-5 Reviewer A nit. Opening clause said "re-includes
app/global/*.cfm" which overstates the re-evaluation scope: detection
is broad (every *.cfm under app/global/) but $reincludeGlobals only
re-evaluates functions.cfm and the files it transitively <cfinclude>s.
Rewording matches the round-4 dev-server.mdx tightening.
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
* fix(test): keep test-local.sh from silently dying on missing ~/.lucli/express (#2796)
Under `set -euo pipefail`, `find ~/.wheels/express ~/.lucli/express ...`
exits non-zero whenever any path arg is missing (stderr suppressed via
`2>/dev/null`, but the exit status survives), `pipefail` propagates it
through `head -1`, and the command-substitution assignment trips `set -e`.
The cleanup trap then fires with no server to clean up, so the user sees
"Starting Wheels CLI server on port 8080..." with EXIT=1 and no
`/tmp/wheels-test-server.log` produced — broken for every install since
the lucli→wheels rebrand window closed and `~/.lucli/express/` stopped
being created.
Drop the now-dead `~/.lucli/express` fallback (the rename landed in 3.0
and recent CLI releases extract Lucee Express to `~/.wheels/express/`
only) and add `|| true` for defense in depth so a truly fresh install
(before `wheels start` has ever run) leaves `LUCEE_LIB` empty and the
downstream `[ -n "\$LUCEE_LIB" ]` guard skips the JDBC pre-install
cleanly.
Verified: `bash tools/test-local.sh wheels.tests.specs.wheelstest` now
runs the server, produces `/tmp/wheels-test-server.log`, and passes all
137 specs across 38 suites in ~17s.
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci: allow-list APPROVED + CHANGES_REQUESTED in Reviewer A guard (#2797)
* fix(ci): skip COMMENTED reviews in Reviewer A guard
The post-submission guard in bot-review-a.yml scans for "bogus" wheels-bot
reviews on the head SHA and dismisses any with a body shorter than 200
chars or missing the canonical `wheels-bot:review-a` marker. GitHub's
dismiss API only accepts APPROVED or CHANGES_REQUESTED reviews; passing a
COMMENTED review id returns HTTP 422 ("Can not dismiss a commented pull
request review") and crashes the step, red-X'ing the Reviewer A check
even when A's real substantive review landed cleanly seconds earlier.
wheels-bot itself occasionally posts placeholder COMMENTED reviews mid-
cycle as it probes the `gh pr review` CLI before issuing the real one
(observed bodies: "placeholder test - ignore", "placeholder2 - updating",
"test with dollar sign: see \$reincludeGlobals function"). Those leak out
as public COMMENTED reviews, which is what the guard is meant to clean
up — but it can't dismiss them via this API, so it has to skip them.
Add `select(.state != "COMMENTED")` to the jq pipeline that selects
actionable reviews. COMMENTED reviews don't gate merging anyway — only
APPROVED and CHANGES_REQUESTED do — so leaving them in PR history is
acceptable noise. The guard now only acts on what it can actually
dismiss.
Observed on PR #2795 commit 0db188a5a5d27cd80b58939df5e0c8dd7464a00b,
job run 26296842347.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
* ci: allow-list APPROVED + CHANGES_REQUESTED in Reviewer A guard
Tighten the previous COMMENTED-state filter to an explicit allow-list:
APPROVED and CHANGES_REQUESTED are the only review states that (a)
GitHub's dismiss API accepts and (b) gate merging. Switching from
"!= DISMISSED && != COMMENTED" to "state IN (APPROVED, CHANGES_REQUESTED)"
also covers PENDING reviews, which return the same HTTP 422 from the
dismiss endpoint if they ever land on a head SHA.
Follow-up to bd76e53de4 per Reviewer A's allow-list suggestion on #2797.
Reviewer B confirmed PENDING is reachable and the closed-set form is
strictly safer than the deny-list. No behavioral change for the
COMMENTED case fixed in bd76e53de4 — that state is still excluded.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(migrator): handle orphan versions in shared dev databases (#2780) (#2798)
* fix(migrator): handle orphan versions in shared dev databases (#2780)
When wheels_migrator_versions records a version whose migration file is
not in the current checkout (shared dev DB / peer applied a migration
whose file isn't yet in this branch), wheels migrate latest no longer
takes a misleading "down" branch and silently no-ops.
Changes:
* Migrator.$getOrphanVersions() — diffs the tracking table against on-
disk files and returns versions with no matching file, sorted ascending.
* Migrator.migrateTo() — branches on "orphan-at-top" before the existing
direction check. When every DB version above target is an orphan: emits
a warning naming the orphans, then either applies pending local files
(up branch) or prints a clear "Nothing to do" naming current vs target.
When SOME above-target versions are legitimate down candidates and
SOME are orphans, emits the warning but lets the existing down loop
handle the rest (orphan rows skip naturally because the loop iterates
files only).
* Migrator.\$buildInfoOutput() — extracted info rendering so it is unit-
testable without the HTTP dispatcher. Orphan rows render with a [?]
marker and the literal "********** NO FILE **********" (Rails-style),
with a footer explaining the cause.
* public/views/cli.cfm info handler — delegates to \$buildInfoOutput().
* New spec OrphanDetectionSpec covers \$getOrphanVersions + the
directional fix across five scenarios (clean state, single orphan,
multiple orphans, nothing-to-do path, legitimate down still works).
* New spec MigratorInfoSpec covers \$buildInfoOutput rendering.
* Docs: new guides page at basics/shared-development-databases.mdx
walks through what an orphan is, three resolution paths, and the
recommendation to avoid shared dev DBs. AI-side reference at
.ai/wheels/troubleshooting/shared-dev-databases.md.
Note on local verification: a leaked JVM from another worktree is
holding Tomcat's shutdown port (8081), blocking new wheels server
starts on this machine. Tests are queued for CI to run across the full
engine + DB matrix. Local TDD will run on next machine restart.
Refs #2780
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(migrator): address Reviewer A/B consensus findings (round 1)
- Reword "Your latest local migration" to "Your target version" in the
orphan nothing-to-do message. arguments.version is the target, not
necessarily the latest local file (Migrator.cfc).
- Add mixed-case OrphanDetectionSpec covering orphan + legitimate down
candidate above target: warning names the orphan, down branch still
runs, c_o_r_e_dropbears is dropped after rollback.
- Drop docs/superpowers/plans/2026-05-22-orphan-migration-detection.md
(1053-line agentic plan duplicated in PR body / commit message).
- Add CHANGELOG entry under [Unreleased] Fixed describing the orphan
detection, three migrateTo branches, and the [?] info row.
Non-blocking items left for follow-up: double getVersionsPreviouslyMigrated()
fetch in migrateTo() (B disputed scope but underlying concern is valid),
and the deferred local test-local.sh migrator run (sandbox lacks the
wheels CLI binary; compat-matrix CI will validate on the new SHA).
Refs #2780, #2798
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* docs(migrator): fix stale plan reference in shared-dev-databases.md
The 1053-line agentic plan file at
docs/superpowers/plans/2026-05-22-orphan-migration-detection.md was
deleted in commit ddac9690 per Reviewer A/B convergence, but the
AI-side reference doc still pointed to it.
Replaced the dead link with a PR reference and tightened the
follow-up section to describe the work in prose instead of pointing
at plan files that may or may not exist.
Round-2 Reviewer A nit.
Refs #2780, #2798
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: Peter Amiri <peter@alurium.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* feat(migrator): doctor/forget/pretend reconciliation commands (#2780) (#2799)
* feat(migrator): doctor/forget/pretend reconciliation commands (#2780)
Follow-up to #2798. Adds three new `wheels migrate` subcommands for
manual reconciliation against the tracking table — the Flyway
`validate` / `repair` / `SkipExecutingMigrations` analogues for
Wheels.
* `wheels migrate doctor` — single-command health report. Lists
orphans, pending local migrations, and applied count. Pure read;
never mutates. Built on Plan 1's `$getOrphanVersions()`.
* `wheels migrate forget <version> --yes` — removes a single row
from `wheels_migrator_versions` without running `down()`. Refuses
if the version has a matching local file (use `migrate down`
instead) or if the version isn't in the table. Idempotent.
* `wheels migrate pretend <version> --yes` — inserts a row into
`wheels_migrator_versions` without running `up()`. Refuses if
already applied or if no local file matches.
Both `forget` and `pretend` require explicit `--yes` to mutate;
without it they print what would happen and exit. The CLI side wires
through a new `runForgetOrPretend()` helper that handles the gating;
the HTTP dispatcher in `cli.cfm` reads the version from
`request.wheels.params.version`.
New spec `MigratorReconciliationSpec.cfc` covers ~12 scenarios
across the three methods: clean state, unhealthy with orphans,
unhealthy with pending, refusal paths, and the no-mutation guarantee
for invalid inputs.
Docs: extended `basics/shared-development-databases.mdx` with the
new commands (Option 2 reworked, Option 2b added, "Comprehensive
diagnostic" section added). AI-side reference and CHANGELOG entry
follow the same shape.
Note on local verification: a leaked JVM from another worktree
continues to hold Tomcat's shutdown port (8081); CI compat-matrix
will validate across the full engine + DB matrix.
Refs #2780, #2798
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(migrator): address Reviewer A round-1 findings on #2799
- forgetVersion() now delegates to the existing private
$removeVersionAsMigrated() helper instead of running an inline
DELETE. The helper wraps the DELETE in the same request.$wheelsDebugSQL
guard that $setVersionAsMigrated() uses, so forgetVersion() and
pretendVersion() are now symmetric: both are no-ops in debug-mode
request contexts instead of silently corrupting the tracking table.
- runForgetOrPretend() now wraps the version argument with
URLEncodedFormat() when building the reconcile URL. $sanitiseVersion()
on the server side strips non-digits before SQL use (no SQL injection
path), but raw URL-special characters (&, =, %) in the CLI argument
could inject spurious query parameters before the sanitiser ran.
Both items were flagged by Reviewer A on commit b7817459b. No new tests
needed: existing MigratorReconciliationSpec covers the success and
refusal paths for forgetVersion(), and the URL-encoding fix is on the
CLI client side which is not exercised by core unit tests.
Refs #2780, #2799
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(cli): doctor output prints yellow when migrator is unhealthy
Reviewer A flagged that runMigration("doctor") always printed in green
regardless of the report's healthy flag. Green on an unhealthy result
(orphans or pending migrations present) reads as "everything is fine"
when actually attention is needed.
The fix switches color to yellow when:
- action == "doctor"
- response includes a `healthy` key
- healthy is false
Other actions (latest/up/down/info) stay green on success — they
either succeed or throw, so the binary mapping holds. Only `doctor`
returns a structural "succeeded but unhealthy" state.
Non-blocking per A's COMMENTED verdict, but a real UX improvement.
Refs #2780, #2799
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs: cross-reference doctor/forget/pretend in main migration docs
The cumulative state of #2798 (merged) and #2799 (this PR) added
three new migrate subcommands plus the [?] orphan display in
`migrate info`. PR-scope docs already covered the deep-dive page
at basics/shared-development-databases.mdx, but the main migrations
reference still said "four subcommands". Catching up:
- basics/migrations.mdx now lists seven subcommands and points to
shared-development-databases for the reconciliation deep dive.
The `info` description mentions the [?] orphan row format.
- CLAUDE.md's MCP/CLI table row now lists `doctor` under
wheels_migrate and adds a separate "Migrator reconciliation"
row for forget/pretend.
- CLAUDE.md's Migrations & Seeding section gains a "Shared Dev
DB Reconciliation" subsection summarising the new commands and
pointing at the deep-dive docs.
- CLAUDE.md's Reference Docs section adds the
.ai/wheels/troubleshooting/shared-dev-databases.md link.
Refs #2780, #2798, #2799
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs: replace stale schema_migrations references with wheels_migrator_versions
Three carryover Rails table-name references in basics/migrations.mdx
(lines 28, 36, 59) predate the c_o_r_e_* → wheels_* rename and don't
match any code in vendor/wheels/, cli/, or app/. Reviewer A flagged
them on PR 2799 round-4 review (f663b5a55) as worth fixing since the
file was already being touched; the surrounding edits in this PR
already use the correct `wheels_migrator_versions` name (line 46).
Other stale references in basics/seeding.mdx and the
v4-0-1-snapshot/ copies are out of scope for this PR; left for a
follow-up cleanup.
Refs #2780, #2799
Signed-off-by: Peter Amiri <peter@alurium.com>
---------
Signed-off-by: Peter Amiri <peter@alurium.com>
* feat(migrator): enrich wheels_migrator_versions with name + applied_at (#2780) (#2800)
* feat(migrator): enrich wheels_migrator_versions with name + applied_at (#2780)
Final follow-up to #2798 + #2799. Adds two nullable columns to the
tracking table so `wheels migrate info` and `wheels migrate doctor`
can show *what* a peer applied and *when* — even for orphan rows
whose migration file isn't in the local checkout.
## Schema change
- `wheels_migrator_versions.name VARCHAR(255) NULL` — migration name
derived from filename (e.g. `create_users`)
- `wheels_migrator_versions.applied_at TIMESTAMP NULL DEFAULT
CURRENT_TIMESTAMP` — when the migration was applied. SQLite gets
TEXT with CFML-side `Now()` injection on insert because SQLite
can't DEFAULT a column on ADD COLUMN.
Both nullable, additive, backward compatible. Existing rows
(pre-enrichment) stay NULL and display version-only in the info
output. Going-forward-only — no backfill.
## Bootstrap
`Migrator.$ensureTrackingColumns()` probes via `$dbinfo` and ALTERs
the table per-engine when columns are missing. Idempotent (skip
when already present). Per-engine SQL covers MySQL, PostgreSQL,
SQLite, MSSQL, Oracle, H2, and CockroachDB — same pattern as the
existing `renameSystemTables()` machinery at Migrator.cfc:932.
Wrapped by `$maybeEnsureTrackingColumns(appKey)` which caches on
`application[appKey].$trackingColumnsEnsured` so the ALTER runs
once per app process, not on every migrator call. Non-fatal: if
the ALTER fails (locked table, weird permissions), the flag stays
unset and the legacy schema continues to work.
Called from `$getVersionsPreviouslyMigrated()` after both the
existing-table happy path and the bootstrap-creates-table path.
## Population
`$setVersionAsMigrated(version, migrationName)` gains an optional
`migrationName` arg. When non-empty AND the enriched-columns flag
is set, includes `name` (and on SQLite, `applied_at`) in the
INSERT. Other engines rely on the column's CURRENT_TIMESTAMP
default for `applied_at`.
Callers updated to pass the name: both up-loops in `migrateTo()`,
`migrateIndividual()`, and `pretendVersion()` (captures the
migration's name from `getAvailableMigrations()` during the
local-file existence check).
## Display
`$getOrphanVersionsWithMeta()` — new public helper. Returns
`{version, name, appliedAt}` structs for each orphan. Falls back
to bare structs when columns aren't ensured or the SELECT fails.
`$buildInfoOutput()` and `cli.cfm`'s `doctor` case now render
`[?] <version> <name> (applied <timestamp>)` when populated, with
the legacy `[?] <version> ********** NO FILE **********` fallback
for legacy NULL orphans.
`doctor()` adds `orphansWithMeta` to its result struct alongside
the existing `orphans: array of strings` (kept for
backward-compat with `migrateTo()`'s direction logic).
## Docs
- `web/sites/guides/.../basics/shared-development-databases.mdx`:
updated `info` sample output to show the enriched format + a
note explaining the legacy fallback.
- `.ai/wheels/troubleshooting/shared-dev-databases.md`: new
"Schema enrichment" section documenting the helpers and behavior.
- `CLAUDE.md`: updated the `info` format line and added the
tracking-table schema summary in the Shared Dev DB Reconciliation
subsection.
- `CHANGELOG.md`: entry under `[Unreleased] Changed`.
## Tests
`SchemaEnrichmentSpec.cfc` covers three scenarios:
- $ensureTrackingColumns adds both columns on first call
- Idempotent re-run (added=[])
- Name column populated for newly applied migrations
Existing specs (migratorSpec, OrphanDetectionSpec, MigratorInfoSpec,
MigratorReconciliationSpec) all preserve backward compat — the new
name column is opt-in (only written when caller passes it), and
the new display logic falls back to legacy rendering for NULL
metadata.
## Known follow-ups (non-blocking, separate PRs)
- Double `$getVersionsPreviouslyMigrated()` fetch carry-over from
reviewer notes on #2798 and #2799 — still pending. This PR
doesn't worsen the situation; the cached `$trackingColumnsEnsured`
flag means `$ensureTrackingColumns` only probes columns once per
process.
Refs #2780, #2798, #2799
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(migrator): don't call \$dbinfo inside $setVersionAsMigrated transaction
CI on Lucee 7 + SQLite revealed two correctness bugs in #2800:
1. \$setVersionAsMigrated called \$dbinfo(type="version") to detect
SQLite-vs-other-engines before deciding whether to include applied_at
in the INSERT. But \$setVersionAsMigrated runs INSIDE migrateTo's
open JDBC transaction, and SQLite (and possibly other engines under
concurrent load) can't service JDBC metadata calls inside an open
transaction. Result: [SQLITE_ERROR] SQL error or missing database,
the migration's transaction rolled back, and every test that
applied a migration with the enriched schema active failed.
Fix: cache the engine type on application[appKey].\$migratorDbType
in \$ensureTrackingColumns() (which always runs outside transactions),
then read it from the cache in \$setVersionAsMigrated(). No more
\$dbinfo calls during inserts.
2. \$maybeEnsureTrackingColumns set the \$trackingColumnsEnsured cache
flag whenever \$ensureTrackingColumns() didn't throw — even when
the column probe failed (e.g. table didn't exist yet) and no
ALTERs ran. Subsequent \$setVersionAsMigrated calls then saw the
flag set and tried to INSERT with a `name` column that didn't
actually exist on the table → SQL error → rolled back transaction.
Fix: only set the cache flag when BOTH rv.hasName and rv.hasAppliedAt
are true after \$ensureTrackingColumns() returns. Failures, missing
tables, or partial-ALTER states leave the flag unset so subsequent
calls retry.
Together these failures broke six specs across migratorSpec,
OrphanDetectionSpec, and SchemaEnrichmentSpec — all symptoms of the
same two root causes. With the fix in place, \$setVersionAsMigrated
is back to a single $query call with no metadata interrogation, and
the enriched-INSERT path only fires when the schema is genuinely
confirmed.
Refs #2780, #2798, #2799, #2800
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(migrator): address Reviewer A round-1 findings on #2800
C1 (correctness): The shared-dev-databases guide example showed
"(applied ...)" timestamps on [x] rows, but the code in
$buildInfoOutput only populates appliedAt for orphan rows (the
DB isn't re-queried for applied_at on local-file rows). Aligning
the guide example with what the code outputs — keep the timestamp
display on the [?] orphan row where the schema enrichment actually
shows through, drop it from the [x] rows. Surrounding prose already
correctly scopes the enrichment to orphans.
T1 (test quality): SchemaEnrichmentSpec's "populates the name column"
test used expect(Len(rows.name) > 0).toBeTrue() which collapses to
a boolean before the matcher sees it — on failure the error message
is just "Expected [false] to be [true]" with no hint of the actual
value. Replaced with expect(rows.name).notToBeEmpty() so failures
include the actual name (or absence of one).
T2 (coverage): Added a new "populates applied_at for newly applied
migrations" spec that queries the applied_at value and asserts it
parses as a date. Covers both code paths: the column-DEFAULT
CURRENT_TIMESTAMP that fires on MySQL/Postgres/MSSQL/Oracle/H2 AND
the CFML-side Now() that SQLite needs because it can't DEFAULT a
TIMESTAMP on ADD COLUMN.
T3 (lint): Added a comment block above the describe block
explaining why CockroachDB is skipped — mirrors the existing
pattern in migratorSpec, OrphanDetectionSpec, and MigratorInfoSpec
(numeric-version test fixtures don't run cleanly against CockroachDB;
compat-matrix.yml treats CockroachDB as soft-fail). The guard is
intentional, not vestigial.
Also clearing the new $migratorDbType app-scope cache in the
beforeEach/afterEach alongside the existing $trackingColumnsEnsured
cleanup, so each test starts from a clean state.
A's performance note about $dbinfo in $setVersionAsMigrated is
already addressed in commit bb4bd761b (caches the engine type on
$migratorDbType to avoid the per-insert metadata round-trip). A
was reviewing 695f8300c and hadn't seen that commit yet.
Refs #2780, #2800
Signed-off-by: Peter Amiri <peter@alurium.com>
* docs: correct stale [x] timestamp claim in .ai schema-enrichment ref
Reviewer B's round-1 convergence on #2800 caught a missed-by-A
follow-on of the C1 finding: the .ai/wheels/troubleshooting/
shared-dev-databases.md doc's first display bullet read
- `[x] <version> <name> (applied <timestamp>)` when populated
but the code only renders the (applied ...) suffix on orphan rows.
The MDX guide example was corrected in fdbbedc35; this commit
brings the .ai-side reference in line and adds a sentence explaining
why $buildInfoOutput doesn't show applied_at for [x] rows (it would
require re-querying the tracking table for every getAvailableMigrations
result, which is a bigger change than this PR is taking on).
Refs #2780, #2800
Signed-off-by: Peter Amiri <peter@alurium.com>
* fix(migrator): populate \$migratorDbType cache before early-return (#2800 C2)
Reviewer A round-2 caught a latent bug in \$ensureTrackingColumns:
the early-return when both enriched columns are already present
fired BEFORE the \$migratorDbType cache was populated. The cache is
on application scope, so it gets cleared on every app restart /
?reload=true. On the second app start onward, the early-return
fired, \$migratorDbType stayed empty, and \$setVersionAsMigrated's
SQLite branch (which writes an explicit applied_at via CFML Now()
becau…
Summary
Inside
.namespace("foo")(or equivalent.scope()/.package()), writingto="foo/dashboard##index"instead ofto="dashboard##index"silently produced afoo.foo/dashboardcontroller path that downstream got flattened to aFoodashboard-style class lookup with an opaqueWheels.ViewNotFounderror. Users had no way to trace the bizarre class name back to the offending route definition.$match()(invendor/wheels/mapper/matching.cfc) now detects when the parsed controller starts with the scope's package converted to slash form and throwsWheels.MapperArgumentInvalidat registration time. The error message names the namespace, quotes the offending value, and points at the correct shorter form so users can fix the route rather than chase the symptom.Picks option 1 ("Reject the redundant form at route-registration time") from the triage comment's menu of fixes — it surfaces the foot-gun loudly at app boot rather than at first request, and avoids the silent-corruption hazard of option 2 (auto-strip-with-warning).
Related Issue
Closes #2791
Type of Change
Feature Completeness Checklist
Signed-off-by:(usegit commit -s); see CONTRIBUTING.mdvendor/wheels/tests/specs/mapper/MatchingSpec.cfccovering: namespace +to=prefix, namespace +controller=prefix, nested namespaces,.package()helper, near-miss prefix overlap (must NOT reject), and the canonical accepted formbot-update-docs.ymlas a follow-up commitbot-update-docs.ymlbot-update-docs.yml[Unreleased]wheels.tests.specs.mapper76/76,wheels.tests.specs.dispatch96/96,wheels.tests.specs.controller423/423 on Lucee 7 + SQLiteTest Plan
Before this PR (manual repro from the issue):
cfml mapper() .namespace("datapai") .get(name="datapaiDashboard", pattern="dashboard", to="datapai/dashboard##index") .end() .end();GET /datapai/dashboard→Wheels.ViewNotFound — Could not find the view page for the index action in the datapai.Datapaidashboard controller.(opaque)After this PR: app fails to boot with
Wheels.MapperArgumentInvalid — Route inside .namespace('datapai') (or equivalent .scope() / .package()) uses a redundant namespace prefix in its controller path. Got controller="datapai/dashboard" (likely from to="datapai/dashboard##index"). The namespace prefix is added automatically — use controller="dashboard" (or to="dashboard##index") instead.The correct form
to="dashboard##index"continues to route normally toapp/controllers/datapai/Dashboard.cfc#index.A human review is required before merge.