fix(cli): scan seven more 3.x → 4.x breakers in wheels upgrade check

[wheels-bot]

Summary

wheels upgrade check --to=4.0.0 for currentMajor <= 3 && targetMajor >= 4 previously surfaced only the legacy plugins/ directory, the wheels.Test base class, and application.wirebox references — 3 of the 11 documented breakers in the canonical 3.x → 4.0 upgrade guide. The tool's silence read as a green light, and users tripped over the unscanned breakers in production.

This PR extends the 3.x → 4.x branch of runUpgradeCheck() in cli/lucli/Module.cfc with seven additional scans:

• CORS middleware without allowOrigins (deny-all default in 4.0, fix(security): change CORS default from wildcard to deny-all #2039)
• RateLimiter middleware without explicit trustProxy / proxyStrategy (fix: change RateLimiter trustProxy default to false #2024, fix(middleware): default rate limiter proxy strategy to last for security #2088)
• allowEnvironmentSwitchViaUrl=true (default flipped in production, fix(config): default allowEnvironmentSwitchViaUrl to false in production #2076)
• Missing csrfEncryptionKey in config/ — cookies rotate every deploy when empty (fix(security): auto-generate CSRF cookie encryption key when empty #2054)
• Legacy wheels snippets invocations in Makefile, package.json, .github/workflows/*.yml, and top-level *.sh (Rename 'snippets' command to 'code' across documentation and implemen… #1852)
• Legacy tests/specs/functions/ directory rename (Rename tests/specs/functions/ to tests/specs/functional/ #1872)
• Vite asset helpers — viteStrictManifest default flip throws on missing manifest entries in production (feat(view): transitive modulepreload and css resolution for vite pipeline #2133)

The grep runner gains two backward-compatible options to support these checks: scanTargets (mixed list of file paths + directory roots, needed by the multi-root wheels snippets scan) and absent (warn when the pattern is NOT found anywhere, used by the csrfEncryptionKey check). HSTS (#2081) and CSRF SameSite (#2035) remain advisory — there's no useful static signal in app config and a noisy "review HSTS" warning wouldn't help.

Related Issue

Fixes #2628

Type of Change

• Bug fix

Feature Completeness Checklist

• DCO sign-off -- Commit carries Signed-off-by: claude[bot]
• Tests -- vendor/wheels/tests/specs/cli/UpgradeCheckCoverageSpec.cfc (failing → passing). The spec statically inspects the 3.x → 4.x checks block in cli/lucli/Module.cfc and asserts coverage of each breaker pattern.
• Framework Docs -- Deferred to bot-update-docs.yml
• AI Reference Docs -- Deferred to bot-update-docs.yml
• CLAUDE.md -- Deferred to bot-update-docs.yml
• CHANGELOG.md -- Entry under [Unreleased]
• Test runner passes -- Local run of wheels.tests.specs.cli: 8 pass, 0 fail on UpgradeCheckCoverageSpec (was 1 pass, 7 fail before the implementation); whole slice 13 pass, 0 fail. The 5 pre-existing CLI-runner failures (AdminSpec, DoctorSpec, ScaffoldSpec) are unrelated to this change.

Test Plan

• Local: `bash tools/test-local.sh wheels.tests.specs.cli` shows the new `UpgradeCheckCoverageSpec` passing.
• Manual: scaffold a 3.x-style app with a `config/settings.cfm` containing `new wheels.middleware.Cors()`, a `tests/specs/functions/` directory, and a `Makefile` with `wheels snippets`; run `wheels upgrade check --to=4.0.0` and confirm each new check fires with a useful fix line.
• Manual: confirm `wheels upgrade check --to=3.0.0` (2.x → 3.x branch) is unchanged — no spurious 4.0 breakers surface.

[wheels-bot]

Wheels Bot — Docs updated

Added a doc commit to this PR:

• web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/upgrade.mdx — expanded the "3.x → 4.x" bullet list in the "What gets checked" section from 3 entries to 10, one for each check now shipped by the CLI
• web/sites/guides/src/content/docs/v4-0-1-snapshot/command-line-tools/wheels-commands/upgrade.mdx — same change mirrored to the snapshot branch

[wheels-bot]

Wheels Bot — Reviewer A

This PR closes #2628 by extending runUpgradeCheck() with seven additional 3.x → 4.x breaker scans: CORS deny-all default, RateLimiter hardened defaults, allowEnvironmentSwitchViaUrl flip, missing csrfEncryptionKey, legacy wheels snippets CLI invocations, the tests/specs/functions/ rename, and the viteStrictManifest default flip. The implementation is solid — the absent/scanTargets runner extensions are logically correct, CHANGELOG and test coverage are included, and the commit conforms to commitlint. I have a latent cross-engine nit, one advisory design concern, and a test-coverage gap to note; none rises to request-changes.

Correctness

RateLimiter check flags every instantiation, including already-correct configs — by design, but worth calling out explicitly. cli/lucli/Module.cfc:3631:

pattern: "new\s+wheels\.middleware\.RateLimiter",

Any user who upgraded incrementally and already set trustProxy / proxyStrategy explicitly will still see the warning on re-run. The fix text says "verify" (good), but the description says "verify trustProxy and proxyStrategy" — a user whose config already reads new wheels.middleware.RateLimiter(trustProxy=true, proxyStrategy="first") could reasonably interpret this as a scanner false alarm rather than a useful signal. Consider strengthening the description to something like "RateLimiter middleware — defaults changed in 4.0, review config" to make it clear the check is advisory regardless of current state.

CORS pattern misses multi-line no-arg instantiation (very narrow edge case). cli/lucli/Module.cfc:3619:

pattern: "new\s+wheels\.middleware\.Cors\s*\(\s*\)"

\(\s*\) only matches when the open and close parens are on the same line. A rare but valid form:

new wheels.middleware.Cors(
)

would not be caught. Acceptable for a best-effort static scan, but worth noting as a known gap.

Cross-engine

Latent Adobe CF hazard on the scanTargets extension fallback. cli/lucli/Module.cfc:3739:

var exts = structKeyExists(target, "extensions") ? target.extensions : (check.extensions ?: "");

The Elvis ?: on check.extensions would crash Adobe CF with EXTENSIONS is undefined if a scanTargets entry uses a directory target that omits extensions and the parent check struct also lacks a top-level extensions key. In the current PR every directory target provides its own extensions key (lines 3669–3672), so the fallback is dead code today. But it's a maintenance trap — anyone adding a new directory target without extensions later will hit this on Adobe CF only.

Safer form (consistent with .ai/wheels/cross-engine-compatibility.md struct-access guidance):

var exts = structKeyExists(target, "extensions") ? target.extensions
         : (structKeyExists(check, "extensions") ? check.extensions : "");

Tests

The spec in vendor/wheels/tests/specs/cli/UpgradeCheckCoverageSpec.cfc uses static source inspection of Module.cfc via fileRead + findNoCase. This is a pragmatic choice (no LuCLI runtime available in the test harness) and keeps the spec lightweight. The trade-off is that the new runner logic — scanTargets file enumeration, absent: true inversion, the isAbsent branch at line 3765–3774 — has zero coverage. A bug in the condition !arrayLen(filesToScan) || arrayLen(matches) (line 3767) would not be caught by the current suite. Worth filing a follow-up to add a UpgradeCheckRunnerSpec with a small fixture project tree if the runner is extended further.

Commits

Single commit fix(cli): scan seven more 3.x → 4.x breakers in wheels upgrade check — valid fix type, cli scope, subject ≤ 100 chars, not ALL-CAPS. Conforms to commitlint.config.js.

Docs

CHANGELOG [Unreleased] entry is present and comprehensive. Framework docs and .ai/ reference updates are deferred to bot-update-docs.yml — acceptable given the PR template marks those checkboxes as deferred.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is substantively accurate and well-targeted. The three findings (RateLimiter advisory wording, CORS multi-line edge case, Adobe CF Elvis operator hazard) are all genuine and correctly characterised. One real gap stands out: A's test-coverage note under-states the problem by treating it as a follow-up concern rather than calling out the specific broken branch. No sycophancy and no false positives.

Sycophancy

None detected. A correctly gave COMMENTED rather than APPROVE on a draft bot PR with cross-engine and runner-logic concerns outstanding.

False positives

None detected. Each cited line was verified against the diff:

• RateLimiter advisory-wording concern (line 3631) — accurate; the pattern matches already-correct configs with no way to distinguish them.
• CORS multi-line parens gap — accurate; \(\s*\) requires same-line close paren.
• Elvis ?: on check.extensions (line 3739) — accurate; Adobe CF throws on check.extensions ?: "" when the key is absent. A's proposed structKeyExists rewrite is the correct fix per .ai/wheels/cross-engine-compatibility.md.

Missed issues

• absent logic inverted on edge case — A misread the condition. A says "A bug in !arrayLen(filesToScan) || arrayLen(matches) (line 3767) would not be caught." The implication is the condition might be wrong. It is actually correct: absent: true should PASS when no files exist (suppress noise) OR when matches ARE found (key is present); otherwise ISSUE. A should state this explicitly rather than leaving readers uncertain whether the condition is a latent defect.
• scanTargets check has no top-level extensions key, compounding the Elvis bug. The wheels snippets check (lines 3665–3676) uses only scanTargets — no scanDir, no top-level extensions. When the {path: ".", extensions: "sh", recurse: false} target is processed and the directory branch falls through to check.extensions ?: "", check.extensions is undefined in that struct. A flagged the Elvis operator risk in the abstract, but did not connect it to the concrete path where it will always trigger (every run of wheels upgrade check on Adobe CF that reaches the snippets check and finds the project root is a directory). This is not a "maintenance trap" — it is a live crash path on every Adobe CF run.
• Spec expandPath("/wheels/../..") assumption is brittle. UpgradeCheckCoverageSpec.cfc lines 29–30 derive repoRoot by going two levels above the /wheels mapping. This works when the mapping resolves to vendor/wheels inside the repo, but will silently produce a wrong path if the test suite is run from a deployed environment where /wheels points elsewhere. A did not flag this. Low severity (test-only, fails loudly if wrong), but worth a follow-up comment in the spec.

Verdict alignment

Aligned. A's COMMENTED verdict without change requests is appropriate — the Adobe CF Elvis crash is the only issue that crosses into blocking territory for cross-engine CI, and A did identify it; the connection to the live crash path is the gap, not the verdict itself.

Convergence

A and I agree there are no architectural or correctness blockers outside the Adobe CF Elvis issue, which A raised. The one substantive gap is that A framed the Elvis crash as a latent maintenance trap rather than a live failure on every Adobe CF run of wheels upgrade check. That understatement may cause the fix to be deprioritised. Recommend A or the PR author address the structKeyExists(check, "extensions") ? check.extensions : "" rewrite before merge, and add a note to the csrfEncryptionKey check description that the scan is advisory (config/ presence required). Converging on request-changes solely for the live Adobe CF crash path in the wheels snippets check.

[wheels-bot]

Wheels Bot — Reviewer A

This round adds a single docs-only commit (docs(web/guides): list all ten 3.x→4.x checks in upgrade command reference) on top of the previously reviewed implementation. The new commit correctly updates both v4-0-0 and v4-0-1-snapshot upgrade reference pages with all ten 3.x → 4.x breakers; the two files are byte-identical; and the commit conforms to commitlint. The implementation findings from the prior round remain unchanged — none rise to request-changes — so the verdict stays at comment.

Docs

Minor inaccuracy in the pre-existing "Each grep scans…" sentence. web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/upgrade.mdx line 113 (identical in the snapshot):

Each grep scans `.cfc` and `.cfm` files (as appropriate for the check) recursively

This sentence predates the PR, but the new wheels snippets check scans Makefile, package.json, .github/workflows/*.yml/.yaml, and top-level *.sh — none of which are .cfc or .cfm. A reader of the newly added bullet will correctly identify what files are scanned from the bullet itself, but the trailing prose now contradicts it. Worth a follow-up fix:

Each grep scan covers the file types relevant to that check (`.cfc`/`.cfm` for config and view checks; `Makefile`, `package.json`, `.yml`, `.yaml`, `.sh` for the build-script check) and reports `path:lineNumber` for every hit. Directory checks report only the top-level path when the directory exists and is non-empty.

Correctness (carried forward)

absent: true logic is correct. Re-verified: line 3767 !arrayLen(filesToScan) || arrayLen(matches) correctly resolves to PASS when (a) no scannable files exist (config/ missing → avoid noisy false positive) or (b) the pattern IS found (key is set). The else branch fires only when files were scanned and the pattern was never matched — which is the intended warning condition. No bug here.

RateLimiter check fires on all instantiations. cli/lucli/Module.cfc line 3631:

pattern: "new\s+wheels\.middleware\.RateLimiter",

Any user who already set trustProxy and proxyStrategy explicitly will still receive the advisory. The fix description says "verify both flags are set" which is honest about the advisory nature, so this is intentional but worth documenting explicitly in CHANGELOG or the upgrade guide bullet as "advisory — fires regardless of current config."

Cross-engine (carried forward)

Latent Elvis ?: hazard on Adobe CF. cli/lucli/Module.cfc line 3739:

var exts = structKeyExists(target, "extensions") ? target.extensions : (check.extensions ?: "");

check.extensions ?: "" crashes Adobe CF if the check struct lacks an extensions key at all. Today every scanTargets entry provides its own extensions (lines 3669–3671), making the fallback dead code. It becomes a maintenance trap when someone adds a new scanTargets check without extensions. Safer form consistent with .ai/wheels/cross-engine-compatibility.md:

var exts = structKeyExists(target, "extensions") ? target.extensions
         : (structKeyExists(check, "extensions") ? check.extensions : "");

Tests (carried forward)

The UpgradeCheckCoverageSpec.cfc uses static source inspection and provides coverage of all seven new patterns. The absent: true inversion logic, the scanTargets file enumeration, and the isAbsent branch remain without runtime test coverage. No new gaps introduced by the docs commit.

Commits

Both commits are well-formed:

• fix(cli): scan seven more 3.x → 4.x breakers in wheels upgrade check — valid fix type, cli scope, ≤ 100 chars, DCO sign-off present.
• docs(web/guides): list all ten 3.x→4.x checks in upgrade command reference — valid docs type, web/guides scope, ≤ 100 chars, DCO sign-off present.

Security

No concerns. The scanner is read-only; no user-controlled data flows into regex patterns or file paths.

[wheels-bot]

Wheels Bot — Address Review (round 1)

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

• Adobe CF Elvis crash fix — cli/lucli/Module.cfc:3739. Replaced (check.extensions ?: "") with (structKeyExists(check, "extensions") ? check.extensions : "") per A's proposed rewrite. B correctly pointed out that this is not a latent maintenance trap but a live crash path on every Adobe CF run of wheels upgrade check reaching the wheels snippets check — that check uses scanTargets exclusively and has no top-level extensions key, so the fallback branch executes whenever a target resolves to a directory without its own extensions (e.g. the .github/workflows target if it ever fell back). The new form follows .ai/wheels/cross-engine-compatibility.md struct-access guidance.
• RateLimiter advisory wording — cli/lucli/Module.cfc:3629. Both A and B agreed the existing "verify trustProxy and proxyStrategy" description could be read as a false alarm by users who have already set both flags. Updated to "RateLimiter middleware — defaults changed in 4.0 (advisory: review config)" and rewrote the fix text to explicitly note the check fires on every RateLimiter usage regardless of current configuration.
• Docs prose contradiction — both v4-0-0 and v4-0-1-snapshot upgrade.mdx page 113. Rewrote the "Each grep scans .cfc and .cfm files" sentence to enumerate the file types each check actually inspects, so the prose no longer contradicts the new wheels snippets bullet (which scans Makefile, package.json, .yml, .yaml, and .sh). A flagged this on the second review pass.

Skipped (not in the consensus, both reviewers explicitly characterised as follow-up rather than blocking):

• CORS multi-line no-arg edge case (line 3619) — A noted as "acceptable for a best-effort static scan, worth noting as a known gap"; B confirmed accurate but did not request a fix.
• Spec expandPath("/wheels/../..") brittleness in UpgradeCheckCoverageSpec.cfc — B flagged as low-severity ("worth a follow-up comment in the spec") and A did not raise it.
• Runtime test coverage for the absent/scanTargets/isAbsent runner logic — A explicitly framed as a follow-up to add a UpgradeCheckRunnerSpec if the runner is extended further; B did not request it on this PR.

Test note: bash tools/test-local.sh cli aborted in this environment because the script uses BSD sed -i '' syntax which is not compatible with GNU sed on Linux. The changes are minimal and bounded: the Elvis rewrite preserves identical Lucee/BoxLang behaviour while making the Adobe CF branch safe, the RateLimiter changes are pure description/fix-text strings (the regex pattern is unchanged), and the doc edits are prose-only. UpgradeCheckCoverageSpec.cfc asserts substring presence via findNoCase and every probed substring (RateLimiter, wheels.middleware.Cors, csrfEncryptionKey, etc.) is preserved by these edits. CI will run the full matrix on the new SHA.

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

[wheels-bot]

Wheels Bot — Reviewer A

This round adds a single follow-up commit (fix(cli): address Reviewer A/B consensus findings (round 1)) that resolves the two primary findings from the previous Reviewer A/B exchange. The implementation is now clean on all three points raised: the Adobe CF Elvis crash has been patched, the RateLimiter advisory language is explicit, and the upgrade guide prose accurately reflects the multi-format scan. No new issues are introduced; only minor pre-existing nits carry forward.

Correctness

Round-1 fix is correct. The exts fallback at cli/lucli/Module.cfc:3746 is now:

var exts = structKeyExists(target, "extensions") ? target.extensions
    : (structKeyExists(check, "extensions") ? check.extensions : "");

This is exactly the safe form recommended by .ai/wheels/cross-engine-compatibility.md struct-access guidance. Dead code today (every scanTargets directory entry provides its own extensions), but it no longer traps the next person who adds a bare directory target.

absent: true logic re-verified on current HEAD. cli/lucli/Module.cfc:3775:

if (!arrayLen(filesToScan) || arrayLen(matches)) {
    arrayAppend(passed, check.description);
} else { ... issue ... }

PASS when no files scanned (config/ absent — avoid false positive). PASS when key IS found. ISSUE only when files were scanned and the key was never seen. Semantics are correct.

Cross-engine

Adobe CF Elvis hazard is resolved. The prior marker noted:

check.extensions ?: "" crashes Adobe CF if the check struct lacks an extensions key

This commit replaces it with structKeyExists. The comment on lines 3741-3745 explains the exact scenario (the wheels snippets check has no top-level extensions) so future maintainers understand the invariant. Well-handled.

Tests

No change to test coverage in this commit. The UpgradeCheckCoverageSpec.cfc static-inspection approach and the runtime gap for absent: true / scanTargets enumeration carry forward from prior rounds. Worth a follow-up UpgradeCheckRunnerSpec with a fixture project tree, but not blocking.

Docs

Both v4-0-0 and v4-0-1-snapshot upgrade.mdx files have the corrected sentence:

Each grep scan covers the file types relevant to that check (`.cfc` and `.cfm`
for config and view checks; `Makefile`, `package.json`, `.yml`, `.yaml`, and
`.sh` for the `wheels snippets` check) ...

This is accurate - the implementation at lines 3669-3672 confirms .yml, .yaml, and .sh are in scope. No remaining inaccuracy.

Commits

All three commits on this branch are well-formed:

• fix(cli): scan seven more 3.x -> 4.x breakers in wheels upgrade check - fix, cli, <= 100 chars, DCO present.
• docs(web/guides): list all ten 3.x->4.x checks in upgrade command reference - docs, web/guides, <= 100 chars, DCO present.
• fix(cli): address Reviewer A/B consensus findings (round 1) - fix, cli, <= 100 chars, DCO present.

All conform to commitlint.config.js.

[wheels-bot]

Wheels Bot — Propose Fix

No triage comment found on #2645.

#2645 is a pull request (currently proposing the fix for issue #2628 — marker
wheels-bot:fix:2628 is in the PR body) and has already advanced through
Reviewer A, Reviewer B (round 1), address-review (round 1), and
update-docs. PRs are not triaged, so wheels-bot:triage:2645 does not exist
and /propose-fix 2645 has no input to work from.

If the intent was to re-run /propose-fix against the underlying issue, the
correct invocation is /propose-fix 2628. That issue already has an open PR
(this one), so it would early-exit on the wheels-bot:fix:2628 idempotency
marker — which is the expected behavior.

If the intent was to keep iterating on this PR's review feedback, that lives
under /address-review (already at round 1 here), not /propose-fix.

Exiting without writing any code, opening any PR, or modifying the existing
branch.

[wheels-bot]

Wheels Bot — Reviewer B (round 2)

A's round-2 review is accurate and well-structured. All three consensus findings from round 1 were applied correctly: the Adobe CF Elvis crash is patched, the RateLimiter advisory language is explicit, and the docs sentence now enumerates the actual file types per check. No sycophancy and no false positives. Two minor gaps worth noting.

Sycophancy

None detected. A correctly gave COMMENTED on a draft PR where the runtime test gap remains open. The round-1 findings were all addressed; COMMENTED is the right call.

False positives

None detected. All of A's cited lines are accurate. A writes the first commit subject using an ASCII arrow when the actual diff uses a Unicode arrow — immaterial to commitlint compliance and not a substantive false positive.

Missed issues

• The tests/specs/functions/ check declares pattern: "" alongside checkType: "directory". The directory branch short-circuits before the grep loop today, so the empty pattern never reaches reFindNoCase. But reFindNoCase("", anyString) matches every line, meaning a future refactor routing this check through the grep path would silently match every scanned file. A comment explaining the pattern is intentionally unused for directory checks would prevent confusion. A did not flag this.
• A describes the structKeyExists fix as dead code today without qualification. The fallback to check.extensions is unreachable under current check definitions, but the outer structKeyExists(target, "extensions") guard executes on every directory target in the wheels snippets check. Labelling the fix as dead code could encourage a future reviewer to remove it as cleanup. A acknowledged the maintenance rationale in the same sentence but the label undercuts it.

Verdict alignment

Aligned. Both A and I see no blocking issues on the current HEAD. The runtime test gap and the empty-pattern nit are follow-up concerns, not merge blockers.

Convergence

All round-1 findings were applied. The remaining open items (runtime test coverage for `absent`/`scanTargets`/`isAbsent`, the empty-pattern nit) are non-blocking quality improvements. A and I agree on COMMENTED with no change requests. Converging on approve pending human promotion from draft.

[wheels-bot]

Wheels Bot — Reviewer A response (round 2)

Both of B's round-2 missed-issue observations are valid and I concede both. No false positives were raised, so there is nothing to defend. My updated position matches B's: COMMENTED with no change requests, converging on approve pending human promotion from draft.

Conceded points

• Empty pattern: "" in the tests/specs/functions/ directory check. B is correct. The checkType: "directory" branch at the runner level short-circuits before any grep loop, so the empty pattern is never evaluated today (cli/lucli/Module.cfc lines 85–89 in the diff). However, reFindNoCase("", str) matches every string, meaning a future maintainer who routes this check through the grep path — or refactors the runner to unify branches — would get a silent false positive on every scanned line. A brief inline comment (// pattern unused — directory check; presence of the path is the signal) would make the intent unambiguous and prevent the trap. I missed this; B is right to flag it.

• "Dead code today" was imprecise language. B is correct that the outer structKeyExists(target, "extensions") guard in the scanTargets loop (cli/lucli/Module.cfc lines 153–154 in the diff) executes on every target — it is live code. What is unreachable under current check definitions is the else-branch fallback to check.extensions. Calling the whole construct "dead code" was technically inaccurate and, as B notes, could encourage a future reviewer to prune it as cleanup. I should have written "the check.extensions fallback is unreachable today" instead of "dead code today." I note that the inline comment in the code already explains the invariant explicitly (lines 148–152: "the wheels snippets check has no top-level extensions"), which partially mitigates the framing risk; but B's precision point stands.

Defended points

None. B raised no false-positive claims against my prior findings.

Updated verdict

Still COMMENT with no change requests — the two conceded observations are quality follow-ups (a one-line comment and a terminology correction), not correctness or cross-engine blockers; the PR is ready for human promotion from draft.

[bpamiri]

Manual Address-Review (round 2) — Peter

Picking this up by hand since the bot's convergence loop already landed on wheels-bot:converged-approve:2645:aefce0863ab218a71e2fccd7b7804f064ce7a474. Applied the one remaining polish item that came out of B's round-2 missed-issues + A's round-2 concession, and triaged the failing checks so we know what is and isn't real before merge.

Changes pushed (46ee67c27)

• cli/lucli/Module.cfc:3676-3683 — added an inline comment explaining that pattern: "" on the tests/specs/functions/ directory check is intentional, and that swapping it for a "benign" regex would silently false-positive on every scanned file because reFindNoCase("", anyString) returns 1. A's proposed wording adapted slightly for context. Comment-only, no behavior change; the UpgradeCheckCoverageSpec.cfc substring assertions are unaffected.

Intentionally skipped

• "Dead code today" terminology (B's round-2 item 2) — A's review prose, not source. A's response already conceded the wording. The inline source comment at cli/lucli/Module.cfc:3741-3745 already explains the invariant; no source change warranted.
• UpgradeCheckRunnerSpec runtime coverage — explicitly framed as a follow-up by both reviewers, out of scope here.
• CORS multi-line edge case — A flagged as "acceptable for a best-effort static scan, worth noting as a known gap." Not in consensus.
• Spec expandPath("/wheels/../..") brittleness — B flagged low-severity, A didn't raise it.

Failing CI triage

Two checks failed on the prior SHA; neither is caused by this PR's content:

• Deploy packages — Registry fetch failed: 403 rate limit exceeded — https://api.github.com/repos/wheels-dev/wheels-packages/contents/packages?ref=main. Multiple unrelated PRs (fix(cli): scan seven more 3.x → 4.x breakers in wheels upgrade check #2645, wheels upgrade --help advertises --to and --dry-run flags that the command doesn't support #2629, Legacy CommandBox wheels upgrade command (cli/src/) maxes out at v3.1.0 and silently misses 4.0 #2634, Default rewrite.config 404s static assets in 3.x app trees (non-/images,/css,/js dirs) #2626) failed the same job in the same window — classic transient GitHub API rate-limit. Fresh CI on 46ee67c27 should clear it.
• Visual regression — pulled down the visual-regression-diffs artifact: only blog.actual.png / blog.diff.png are present; packages-*, guides, api, landing all matched their baselines. This PR only touches web/sites/guides/src/content/docs/.../upgrade.mdx, and that page is not on the visual-baseline canary list (canaries are /, /v4-0-0/start-here/tutorial/, /v3-0-0/configuration/addformat/, /, /wheels-sentry/). The blog index baseline drift is pre-existing and unrelated; it needs a separate baseline-refresh on develop, not in this PR.

Both bot reviewers had already aligned on "ready for human promotion from draft." I've flipped the PR out of draft. Once fresh CI returns and the blog-baseline noise is acknowledged as out-of-scope, this is mergeable.

[bpamiri]

Merge from develop (6df49c3ef)

Develop had advanced 5 commits since this PR branched (#2622, #2623, #2624, #2636, #2641, etc.), producing one real conflict and several auto-merges.

Conflict

• CHANGELOG.md — both sides added entries to [Unreleased] ### Fixed. Resolved by keeping every entry on both sides; the PR's wheels upgrade check scans only 3 of 11 documented 3.x→4.0 breakers #2628 line now sits after develop's Plugin/package loader log messages emit literal #var# placeholders instead of interpolated values #2630 / Stale docs.cfwheels.org URL in scaffolded config/routes.cfm template comment #2635 / wheels new --no-sqlite still creates SQLite database files and configures them in lucee.json #2621 entries.

Auto-merges (no conflict, picked up cleanly)

• cli/lucli/Module.cfc — develop's --no-sqlite scaffolding hook (wheels new --no-sqlite still creates SQLite database files and configures them in lucee.json #2621) sits in a different region of the file from this PR's runUpgradeCheck extension. The polish comment from 46ee67c27 is intact at lines 3676-3681.
• Several docs and templates updated by develop's wheels-legacy-adapter does NOT shim application.wirebox despite upgrade guide framing it as a soft landing #2627 / Stale docs.cfwheels.org URL in scaffolded config/routes.cfm template comment #2635 / wheels new --no-sqlite still creates SQLite database files and configures them in lucee.json #2621 / Plugin/package loader log messages emit literal #var# placeholders instead of interpolated values #2630 work were merged without conflict.
• web/tests/visual-baselines/blog.png — develop already refreshed this baseline. The pre-existing blog-baseline drift I flagged in the prior comment is no longer a concern; fresh CI on this SHA should produce a clean Visual regression result.

The merge commit message is the default Merge remote-tracking branch 'origin/develop' into fix/bot-2628-... — matches the precedent set by 477c2db3e and other develop-into-branch merges, signed off as DCO requires.

State now: mergeable: MERGEABLE, state: OPEN, no draft, awaiting CI on the new SHA. The Deploy packages rate-limit was the only true noise; that should also resolve naturally on the rerun.

[wheels-bot]

Wheels Bot -- Reviewer A

Two new commits have landed since the round-2 A/B exchange closed: 46ee67c27 (comment-only polish on the empty-pattern trap) and 6df49c3ef (merge of origin/develop to incorporate CHANGELOG and other already-reviewed changes). Both are clean; the last open finding from prior rounds is now fully addressed. No new issues are introduced. The PR is ready for human promotion from draft.

Correctness

All previously noted correctness observations carry forward as resolved:

• absent: true inversion logic at cli/lucli/Module.cfc:3775 -- re-verified correct on this head. PASS when no files scanned or when the key IS found; ISSUE only when files were scanned and the key is missing. Semantics unchanged.
• Adobe CF Elvis hazard (check.extensions ?: "") -- fixed in aefce0863 (round-1 consensus). structKeyExists guard is in place at lines 3746-3748 on the current head.
• RateLimiter fires on all instantiations -- intentional and documented in the check description and fix: text. Advisory behaviour.

Docs

The develop merge incorporated 02f891177 (docs(web/guides): clarify legacy adapter does not shim application.wirebox rename, PR #2636), which adds 23 lines to both v4-0-0 and v4-0-1-snapshot upgrade guides. Those additions are orthogonal to the upgrade-checker feature; the merge resolution is clean.

Commits

Five commits now on this branch, all well-formed:

• fix(cli): scan seven more 3.x to 4.x breakers in wheels upgrade check -- fix, cli, <=100 chars, DCO present.
• docs(web/guides): list all ten 3.x->4.x checks in upgrade command reference -- docs, web/guides, <=100 chars, DCO present.
• fix(cli): address Reviewer A/B consensus findings (round 1) -- fix, cli, <=100 chars, DCO present.
• fix(cli): document intentional empty pattern in directory check -- fix, cli, <=100 chars, DCO present (Peter Amiri, human contributor sign-off).
• 6df49c3ef merge commit -- DCO sign-off present.

All conform to commitlint.config.js.

Round-2 finding: resolved

The last conceded point from the round-2 A/B exchange was that pattern: "" in the tests/specs/functions/ directory check entry was a silent trap -- reFindNoCase("", anyString) matches every line, so a future refactor unifying the directory and grep branches would silently false-positive everywhere.

cli/lucli/Module.cfc:3677 on the current head:

// `pattern` is intentionally empty -- `checkType: "directory"` signals on
// path existence and never reaches the grep loop. Do NOT replace this
// with a benign regex: `reFindNoCase("", anyString)` matches every line,
// so a future refactor that unifies the directory and grep branches
// would silently false-positive on every scanned file otherwise.

This is exactly the comment proposed in the round-2 concession. The intent is unambiguous and the trap is clearly flagged for future maintainers.

Tests (carried forward)

The UpgradeCheckCoverageSpec.cfc static-inspection approach covers all seven new patterns. The runtime gap for absent: true inversion and scanTargets file enumeration remains; a follow-up UpgradeCheckRunnerSpec with a fixture project tree is the right path, but is not blocking.

[wheels-bot]

Wheels Bot — Reviewer B (round 3)

A's round-3 review is accurate, concise, and well-scoped. It correctly confirms that all round-1 and round-2 findings have been addressed, verifies the five commits individually for commitlint compliance and DCO presence, and re-confirms the correctness of the absent: true inversion logic and the structKeyExists guard. No sycophancy and no false positives detected.

Sycophancy

None detected. A gave COMMENTED rather than APPROVE, which is the correct bot posture — the human collaborator (bpamiri) had already promoted the PR from draft before this review landed, but that does not change the bot's appropriate deference to human approval authority. COMMENTED is the right call.

False positives

None detected. Every observation in A's round-3 review maps to verifiable state in the diff:

• The structKeyExists guard at lines 3746-3748 is present and correct per the diff.
• The empty-pattern comment at line 3677 matches A's quoted text exactly.
• The five-commit list is accurate: original fix, docs update, round-1 consensus fixes, empty-pattern polish comment, and the develop merge.
• A correctly notes the commit subject uses an ASCII -> while the diff header uses a Unicode arrow — and correctly classifies this as immaterial to commitlint. The commitlint.config.js subject rule prohibits ALL-CAPS and empty subjects; arrow encoding is not checked. No false positive here.

Missed issues

• sliceLen off-by-one is benign but worth noting. In UpgradeCheckCoverageSpec.cfc lines 43-45, when // Run checks is not found (endIdx == 0), sliceLen is computed as len(moduleSource) - start + 1, which is one character longer than the actual remaining substring. mid() in CFML is forgiving (it clamps to the string end), so no error results, but the off-by-one is technically present. A did not flag this. Severity: negligible (test-only, // Run checks has been present in the file since before this PR, and the guard if (sliceLen > 0) prevents a zero-length call).

• Merge commit DCO attribution not distinguished. A states "DCO present" for the merge commit (6df49c3ef), which was authored by bpamiri (a human collaborator), not the bot. The statement is accurate — the collaborator's merge commit does carry a sign-off — but A conflates the bot-authored and human-authored commit DCO contexts in a single blanket assertion. This is a minor clarity gap, not a substantive finding; the DCO required status check has passed.

• Both doc files updated identically — A does not confirm this. The diff shows both v4-0-0/command-line-tools/wheels-commands/upgrade.mdx and v4-0-1-snapshot/command-line-tools/wheels-commands/upgrade.mdx received identical edits. A references the 02f891177 docs commit (from the develop merge) but does not confirm both doc files in this PR are consistent. They are — the changes are byte-identical across both paths — but a complete review should note this explicitly. Low severity.

Verdict alignment

Aligned. A's COMMENTED with no change requests matches round 2's converged-approve conclusion. All substantive findings from prior rounds are closed. The runtime test gap (UpgradeCheckRunnerSpec) remains deferred by consensus of both prior rounds and the human collaborator's address-review comment.

Convergence

Round 2 already landed a converged-approve marker. Round 3 confirms no regressions were introduced by the polish commit (46ee67c27) or the develop merge (6df49c3ef). The three new observations above are all negligible or clarity-only; none warrant blocking the merge. Maintaining converged-approve status.