fix(cli): point scaffolded routes.cfm comment at canonical guides.wheels.dev URL#2640
Conversation
…els.dev URL Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — No doc updatesReviewed this PR's diff and found no docs that need updating (purely a comment URL fix in two CLI scaffolding templates — no user-visible framework feature surface, API, or convention changed; CHANGELOG.md was already updated as part of the PR). |
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a minimal, correct fix — two one-line changes replacing a dead guides.wheels.dev/docs/routing URL with the canonical v4-0-0-snapshot path in the two remaining stale templates, backed by a regression spec that mirrors the established pattern from ApplicationCfcInjectorAssignmentSpec.cfc. Everything compiles, the assertions are logically sound, the commit is compliant, and the CHANGELOG entry is present. One nit worth addressing: the regression spec's targets array omits the third template that was already corrected, leaving a blind spot if that file ever regresses. No correctness, cross-engine, or security concerns.
Conventions
Nit — regression spec omits the already-corrected third template
vendor/wheels/tests/specs/cli/ConfigRoutesStaleDocUrlSpec.cfc, line 32:
var targets = [
"cli/src/templates/ConfigRoutes.txt",
"cli/lucli/templates/app/app/snippets/ConfigRoutes.txt"
];The PR description notes that cli/lucli/templates/app/config/routes.cfm was corrected in a prior pass and is the canonical reference. Excluding it from the regression guard means a future edit could silently reintroduce the stale URL there without a failing test. Suggested fix — add it to targets:
var targets = [
"cli/src/templates/ConfigRoutes.txt",
"cli/lucli/templates/app/app/snippets/ConfigRoutes.txt",
"cli/lucli/templates/app/config/routes.cfm"
];The spec's assertions (canonical URL present, stale URL absent, no cfwheels.org/cfwheels.com) apply equally to that file — no other changes needed.
Tests
The new spec correctly follows the ApplicationCfcInjectorAssignmentSpec.cfc IIFE-captures-loop-var pattern:
for (var rel in targets) {
(function(relPath) {
it("...", () => { ... });
})(rel);
}reFindNoCase("cfwheels\.(org|com)", content) > 0 returns a position (> 0) on a match, so .toBeFalse() correctly rejects any occurrence. The separate docs.cfwheels.org string-contains check is redundant (the regex subsumes it) but harmless and produces a more specific failure message — keeping it is fine.
The ##2635 in the block comment (/** ... */) is cosmetically double-escaped (block comments don't interpret #) but consistent with the pattern in ApplicationCfcInjectorAssignmentSpec.cfc (##2622), so no change needed.
Commits
Single commit fix(cli): point scaffolded routes.cfm comment at canonical guides.wheels.dev URL — type fix, scope cli, subject ≤ 100 chars, not ALL-CAPS, DCO Signed-off-by trailer present. Clean.
Docs
CHANGELOG [Unreleased] entry added. Framework/AI-reference docs and CLAUDE.md marked as handled by bot-update-docs.yml, which is appropriate for a template-comment URL change.
Wheels Bot — Reviewer B (round 1)A's review is accurate and well-grounded. The single nit — adding the already-corrected third template to the regression spec's SycophancyNone detected. A's positive framing is backed by specific diff evidence: commit compliance, CHANGELOG entry, CFML idiom analysis (IIFE closure capture, False positivesNone detected. I verified each claim A made:
Missed issuesNone detected. I searched the full Verdict alignmentA's state is ConvergenceAligned. A's one finding is valid and I have no additional findings to add. The nit (add |
…argets (round 1) Address Reviewer A/B consensus: add cli/lucli/templates/app/config/routes.cfm to the targets array in ConfigRoutesStaleDocUrlSpec so the canonical-URL regression guard also covers the already-corrected third template. Without this, a future edit could silently reintroduce the stale /docs/routing path in that file. All four spec assertions already hold against the file (canonical URL present, stale path / docs.cfwheels.org / cfwheels.(org|com) all absent). Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Test runner note: 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 PR correctly fixes a dead doc-URL comment in two scaffolding templates and adds a regression spec that now covers all three affected files. The previous review's sole nit — the third template (cli/lucli/templates/app/config/routes.cfm) missing from the spec's targets array — was addressed in the follow-up commit. The diff is clean across all dimensions: correctness, conventions, cross-engine compatibility, tests, docs, and commits. No objections to merging; a human approver is required because GitHub prevents self-approval.
What changed
cli/src/templates/ConfigRoutes.txtandcli/lucli/templates/app/app/snippets/ConfigRoutes.txt: one-line URL replacement from dead/docs/routingto the canonical/v4-0-0-snapshot/handling-requests-with-controllers/routingpath.vendor/wheels/tests/specs/cli/ConfigRoutesStaleDocUrlSpec.cfc(new): regression spec guarding all three templates against the stale URL and any retiredcfwheels.(org|com)/docs.cfwheels.orgreferences.- The round-1 follow-up commit added
cli/lucli/templates/app/config/routes.cfmtotargets, closing the blind spot noted in the prior review.
Tests
Spec mirrors the established ApplicationCfcInjectorAssignmentSpec.cfc pattern exactly:
expandPath("/wheels/../..")for repo-root resolution — consistent with the companion spec.- IIFE loop-var capture:
(function(relPath) { it(...) })(rel)— correct; without it everyitclosure would bind the final iteration's value. - All four assertions (
canonical present,stale path absent,docs.cfwheels.org absent,reFindNoCase("cfwheels\.(org|com)")) are logically sound and confirmed bygrepagainst the working tree. - No RocketUnit / legacy
wheels.Testpatterns — correctly extendswheels.WheelsTest.
Commits
Both commits conform to commitlint.config.js: valid types (fix), unrestricted scopes (cli, test), subjects under 100 chars, not ALL-CAPS, DCO Signed-off-by trailers present on both.
74789a1—fix(cli): point scaffolded routes.cfm comment at canonical guides.wheels.dev URL133b863—fix(test): add scaffolded routes.cfm to ConfigRoutesStaleDocUrlSpec targets (round 1)
Docs
CHANGELOG [Unreleased] ### Fixed entry is present and accurate. AI-reference and CLAUDE.md updates correctly deferred to bot-update-docs.yml — a URL comment fix in scaffolding templates does not change framework conventions.
Wheels Bot — Reviewer B (round 1)A's review on the updated SHA is accurate and well-grounded. The address-review commit correctly added SycophancyNone detected. A's "no objections to merging" is backed by specific evidence: diff verification of both URL replacements, IIFE closure analysis, False positivesNone detected. I verified each claim:
Missed issuesNone detected. Cross-engine check: Verdict alignmentA's state is COMMENTED rather than APPROVED, correctly noting that GitHub prevents the bot from self-approving. A's substantive verdict — "no objections to merging" after verifying correctness, tests, commits, and docs — is internally consistent with the findings. ConvergenceAligned. A has no remaining findings and neither do I. The PR is clean on this SHA: two correct one-line URL fixes, a sound regression spec covering all three templates, conformant commits, and an accurate CHANGELOG entry. Joint recommendation: approve and merge once a human reviewer signs off. |
…in-scaffolded-config-r
bpamiri
deleted the
fix/bot-2635-stale-docs-cfwheels-org-url-in-scaffolded-config-r
branch
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The head SHA for this review is the merge-from-develop sync commit (44891e6f) — Merge branch 'develop' into fix/bot-2635-stale-docs-cfwheels-org-url-in-scaffolded-config-r. It introduces no new PR content; the two substantive commits (74789a147 and 133b863fe) were reviewed in the prior pass at 133b863fe. The PR has since been merged. This review records a clean pass against the final tree for completeness.
What the PR did
- Replaced the dead
https://guides.wheels.dev/docs/routingcomment URL with the canonicalhttps://guides.wheels.dev/v4-0-0-snapshot/handling-requests-with-controllers/routingURL in two scaffolding templates. - Added
vendor/wheels/tests/specs/cli/ConfigRoutesStaleDocUrlSpec.cfc: a BDD regression spec covering all three affected template files, asserting the canonical URL is present and all retiredcfwheels.org/cfwheels.com/docs.cfwheels.orgreferences are absent.
Head-SHA delta
The merge commit brought in 1e7dfa491 from develop (fix(cli): honor --no-sqlite when scaffolding lucee.json) — orthogonal to this PR and already reviewed independently (PR 2624). No new code paths in this PR's files were introduced.
Correctness
Template changes: single-line substitution in two identical files — confirmed correct. The canonical URL matches the existing cli/lucli/templates/app/config/routes.cfm reference.
Spec: expandPath("/wheels/../..") correctly resolves the repo root from the Lucee /wheels mapping. The IIFE loop-var-capture pattern is correct and matches the companion spec in ApplicationCfcInjectorAssignmentSpec.cfc. All four assertions are logically sound. The reFindNoCase("cfwheels\.(org|com)", content) > 0 expression returns a position integer — .toBeFalse() correctly fires when a match is found.
Tests
Spec extends wheels.WheelsTest (BDD) — no legacy RocketUnit patterns. All three template files are now guarded in targets, closing the blind spot noted in the prior review round. The ##2635 in the block comment is consistent with the ## escape convention used elsewhere in the spec suite.
Commits
Both PR commits conform to commitlint.config.js — valid types (fix), unrestricted scopes (cli, test), subjects under 100 chars, not ALL-CAPS, DCO Signed-off-by trailers present on both. The merge commit (44891e6f) was authored by a human maintainer doing the develop sync — outside the scope of this review.
Docs
CHANGELOG [Unreleased] ### Fixed entry present and accurate. AI reference and CLAUDE.md updates correctly deferred to bot-update-docs.yml — a template comment URL fix does not alter framework conventions.
No security concerns; no cross-engine issues (spec uses only core CFML built-ins: expandPath, fileExists, fileRead, contains, reFindNoCase).
Wheels Bot — Reviewer B (round 1)A's review on SHA SycophancyNone detected. A's "clean pass for completeness" framing is backed by specific evidence: confirmed that the merge commit only brought in False positivesNone detected. I verified each claim:
Missed issuesNone detected. Cross-engine check: Verdict alignmentA's state is COMMENTED. Given the PR is already merged and A cannot self-approve, COMMENTED is the only available verdict. The substantive judgment — "no objections, prior review stands" — is internally consistent with the findings and with the prior ConvergenceAligned. A has no findings and neither do I. The PR landed cleanly: two correct one-line URL fixes, a sound regression spec covering all three templates, conformant commits with DCO trailers, and an accurate CHANGELOG entry. The develop sync commit introduced nothing new in scope. Joint recommendation: already merged; no further action needed. |
1 participant
Summary
Two CLI scaffolding templates emit a
// See https://...comment in the new app'sconfig/routes.cfmthat points athttps://guides.wheels.dev/docs/routing— a path that doesn't exist on the current docs site. The active 4.0 lucliconfig/routes.cfmtemplate was already corrected to the canonical/v4-0-0-snapshot/handling-requests-with-controllers/routingURL, but two sibling templates that produce the same comment for older code paths still shipped the broken link:cli/src/templates/ConfigRoutes.txt(legacy CommandBox CLI)cli/lucli/templates/app/app/snippets/ConfigRoutes.txt(lucli runtime snippet copied into the generated app'sapp/snippets/)This PR replaces the stale URL in both files with the canonical one and adds a regression spec under
vendor/wheels/tests/specs/cli/that asserts each template references the canonical guides.wheels.dev path and guards against any reintroduction of retireddocs.cfwheels.org/cfwheels.org/cfwheels.comreferences.Related Issue
Closes #2635
Type of Change
Feature Completeness Checklist
git commit -sused; trailer present on the commitvendor/wheels/tests/specs/cli/ConfigRoutesStaleDocUrlSpec.cfc(failing → passing)bot-update-docs.ymlbot-update-docs.ymlbot-update-docs.yml[Unreleased]bash tools/test-local.shcouldn't drive the runner end-to-end here. The new spec is purefileExists/fileRead/contains/reFindNoCaseover two known paths (mirroringvendor/wheels/tests/specs/cli/ApplicationCfcInjectorAssignmentSpec.cfc), and I verified the assertions directly withgrep: pre-fix, each file matched the stale/docs/routingpath 1× and the canonical URL 0× (would failtoBeTrue+toBeFalse); post-fix, each file matches the canonical URL 1× and the stale path 0×, with nodocs.cfwheels.org/cfwheels.org/cfwheels.comhits. CI will exercise the spec on every supported engine × database matrix.Test Plan
https://guides.wheels.dev/v4-0-0-snapshot/handling-requests-with-controllers/routingguides.wheels.dev/docs/routing,docs.cfwheels.org,cfwheels.org, orcfwheels.comvendor/wheels/tests/specs/cli/ConfigRoutesStaleDocUrlSpec.cfcexercises every assertion above against both filesbash tools/test-local.sh wheels.tests.specs.clilocally and confirm the new spec passes alongside the existing CLI specs