Repository navigation
fix(cli): rename consoleExec url param so /models /routes /version /datasource work - #2591
Conversation
Wheels Bot — No doc updatesReviewed this PR's diff and found no docs that need updating (purely internal CLI bugfix — the |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a minimal, correct fix for a CFML reserved-scope collision bug in the wheels console slash commands. Renaming the url parameter to requestUrl in consoleExec unambiguously resolves the shadowing. Tests, CHANGELOG, and commit message are all clean. Would approve if not self-review restricted.
Correctness
The root cause is straightforward: CFML treats url as a reserved struct scope (the URL query-string bag). When consoleExec declared required string url, the call makeHttpPost(url, body) resolved url to the URL scope struct rather than the function parameter, producing the Cannot cast Object type [url] to a value of type [string] error.
The fix is correct and complete. Every call site already passes evalUrl (a local var), so only the function signature and internal reference needed updating. The three HTTP helper functions (consoleExec, makeHttpRequest, makeHttpPost) now all use requestUrl consistently — verified by grepping Module.cfc. No residual url parameter names remain.
Tests
ConsoleCommandSpec.cfc uses the same source-inspection pattern established by ReloadCommandSpec.cfc: consoleExec is private and makes a live HTTP call, so checking the source text is the right trade-off. Both assertions are correctly written:
expect(reFind(pattern, moduleSource) > 0).toBeFalse(message)—toBeFalse(message)is a valid WheelsTest BDD matcher signature.expect(moduleSource).notToInclude(needle, message)— dispatched throughonMissingMethod→toIncludewithisNot=true→notIncludes(target, needle, message), all with matching signatures.notToIncludewith a second message argument works correctly."##2582"in the failure message escapes to#2582at runtime — correct.
One small note for future maintainers: the first test's regex anchors url as the first parameter (function consoleExec\(\s*required\s+string\s+url\b). If a url parameter were ever added in a later position it would not fire. The second test (makeHttpPost(url,) is the more robust guard since it catches the actual call site regardless of signature position. Both together provide adequate regression coverage.
Commits
fix(cli): rename consoleExec url param so /models /routes /version /datasource work — valid type, valid scope, 82 chars, not ALL-CAPS. Passes commitlint.
Docs
CHANGELOG entry under [Unreleased] > Fixed is accurate and appropriately detailed. No .ai/wheels/ or guide doc changes are needed for an internal CLI helper rename.
Wheels Bot — Reviewer B (round 1)A's review is accurate and appropriately scoped. No sycophancy, no false positives, no missed issues. The technical claims are verified against the source. SycophancyNone detected. A backs every positive claim with a specific reference to the diff or source. "Would approve if not self-review restricted" is an honest acknowledgement of the constraint, not a lazy LGTM. False positivesNone detected.
Missed issuesNone detected. The diff is a 2-line rename in Verdict alignmentA submitted COMMENTED with an explicit "would approve" rationale. That is the appropriate state when the bot cannot self-approve; it conveys the intent without bypassing the restriction. Consistent with the findings. ConvergenceA and I are aligned. The fix is correct and complete, the tests adequately cover the regression, and the CHANGELOG entry is accurate. No changes are needed. |
…nds-fail-with-makehttp
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a minimal, correct fix for a CFML reserved-scope collision in wheels console slash commands. The parameter rename from url to requestUrl in consoleExec is the right fix, all call sites pass a local var evalUrl so no call site changes were needed, and the source-level regression spec follows the established ReloadCommandSpec.cfc pattern. The head SHA changed due to a merge-with-develop commit; the PR's three-file diff is substantively identical to the previous review pass. No new issues found.
Correctness
The fix is precise and complete. CFML's implicit URL scope shadowed the required string url parameter inside consoleExec, causing makeHttpPost(url, body) to receive a struct rather than the string URL. The rename to requestUrl (matching makeHttpPost's own parameter name at line 5185 of Module.cfc) eliminates the collision at the only call site inside the function (line 1195).
All six callers of consoleExec (lines 1144, 1165, 1169, 1173, 1178, 1183) pass evalUrl, which is a local var defined at line 1073. No caller changes were needed. The remaining makeHttpPost call at line 1075 passes evalUrl directly and was never affected.
Test coverage
ConsoleCommandSpec.cfc extends wheels.wheelstest.system.BaseSpec (correct — BDD, not RocketUnit). The two source-inspection assertions are appropriate for a private function that makes a live HTTP call:
- Line 23:
reFind(pattern, moduleSource) > 0).toBeFalse(...)— guards the function signature. - Line 30:
.notToInclude("makeHttpPost(url,", ...)— guards the call site.
The ##2582 escape on line 24 correctly emits #2582 at runtime. The toBeFalse(message) signature is valid under WheelsTest's BDD matcher interface.
One minor note for future maintainers: the first regex anchors only the first-parameter position; if url were added as a later parameter it would not fire. The second assertion (call site guard) provides the more robust coverage. Together they are adequate regression protection.
Commits
fix(cli): rename consoleExec url param so /models /routes /version /datasource work — valid type, valid scope, 82 characters, not ALL-CAPS. Passes commitlint.
CHANGELOG
Entry under [Unreleased] > Fixed accurately describes the root cause, the affected commands, and the rename. No guide or .ai/wheels/ doc updates are needed for an internal CLI helper rename.
Cross-engine compatibility
No cross-engine concerns: serializeJSON, deserializeJSON, fileRead, expandPath, and reFind are all standard CFML built-ins available across Lucee, Adobe CF, and BoxLang. No struct member functions on CFCs, no client scope, no bracket-notation function calls.
Wheels Bot — Reviewer B (round 1)A's review is accurate, well-evidenced, and appropriately scoped to the diff. No sycophancy, no false positives, no missed issues. The prior B review on SHA SycophancyNone detected. Every positive claim is backed by a specific line reference. The COMMENTED verdict with explicit "would approve" rationale is an honest acknowledgement of the self-review restriction, not a lazy LGTM. False positivesNone detected.
Missed issuesNone detected. The diff is a 2-line rename in Verdict alignmentCOMMENTED with explicit "would approve" is the correct verdict under a self-review restriction. Consistent with the findings. ConvergenceAligned, no changes needed. A and I agree: the fix is minimal and correct, the tests adequately cover the regression, and the CHANGELOG entry is accurate. This matches the converged-approve outcome already reached on the previous SHA. |
…ti-patterns #11-14 (#2740) CLAUDE.md was 1133 lines, with 10 of 17 listed reference subdirectories missing and several broken doc paths (config/services.cfm, docs/src/...). Reorders, dedups, and trims to 714 lines (-37%) with all internal links now resolving. Top-of-file restructured for high-leverage content first: - Code Map: where framework/demo/CLI live and how they relate - Before-Done checklist: which test suite to run for which change type - Cross-Engine Invariants: promoted from buried Docker / Browser-Testing locations - Anti-Patterns extended Top 10 -> Top 14, all new entries sourced from recent PRs: - #11 CFML reserved scopes shadow parameters (#2591) - #12 Empty array in whereIn / whereNotIn (#2736) - #13 Comma-list config != single-value HTTP header (#2725) - #14 Strip CFML comments before source-scanning (#2595) Extracted sections, loaded only when relevant: - .ai/wheels/deploy.md (92 lines) - wheels deploy Kamal port reference - .ai/wheels/wheels-bot.md (34 lines) - bot architecture - .ai/wheels/testing/browser-testing.md (68 lines) - browser DSL Other dedups: t.timestamps() 3-column rule, mixed-argument-style rule, and the WheelsTest-only-for-new-tests reminder each appear in one canonical location now. Reference Docs list at bottom lists only verified-to-exist files. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
wheels consoleslash commands/models,/routes,/version, and/datasourceroute throughconsoleExecincli/lucli/Module.cfc, whose first parameter was namedurl. CFML's reserved URL scope shadowed the parameter inside the function body, so the callmakeHttpPost(url, body)received the URL scope struct in place of the request URL and threwCannot cast Object type [url] to a value of type [string]. Renamed the parameter torequestUrlto mirrormakeHttpPost's own signature and updated the single call site.Related Issue
Closes #2582
Type of Change
Feature Completeness Checklist
cli/lucli/tests/specs/commands/ConsoleCommandSpec.cfcasserts the parameter name no longer collides with the URL scope (failing → passing).bot-update-docs.ymlif needed).bot-update-docs.ymlif needed).bot-update-docs.ymlif needed).[Unreleased] > Fixed.cli.lucli.tests.specs.commands.ConsoleCommandSpecreportstotalPass: 2, totalFail: 0after the fix; sibling specs incli.lucli.tests.specs.commands.*still pass.Test Plan
curl "http://localhost:60007/wheels/cli/tests?format=json&directory=cli.lucli.tests.specs.commands"→ConsoleCommandSpecpasses; no regressions in adjacent command specs.wheels console→/models,/routes,/version,/datasourceeach return their expected output withoutmakeHttpPosterrors.