feat(cli): wheels upgrade applies the framework swap from the CLI bundle

[bpamiri]

Closes #3035. PR1 of the two-PR apply-mode plan (PR2 = network download of arbitrary --to= targets via ReleaseChannel; not in scope here).

The story

Today wheels upgrade is check-only: the bare verb prints usage, and the help points at brew upgrade wheels — which upgrades the CLI binary but never the app's vendored framework copy. The actual swap is a documented manual zip dance. This PR closes that gap with the smallest trustworthy source: the framework already bundled inside the installed CLI.

• wheels upgrade apply — replaces the app's vendor/wheels/ with the CLI's bundled framework. The old copy is parked at vendor/wheels.bak-<yyyymmdd>-<HHmmss>/ (atomic Java renameTo, collision counter, hard error if the rename fails) so recovery is a single mv. --nobackup opts out. Reports old -> new versions, the backup path, and the exact (quoted) recovery command.
• wheels upgrade (bare) — prints concise usage steering at the two verbs and exits 0, same as before this PR. Contract decision from review: destructive commands deserve an explicit verb, and MCP clients calling wheels_upgrade with {} must never mutate — requiring apply fixes that transport-independently, and exit-0 bare matches the pre-PR behavior so existing CI invocations see usage text, not a surprise swap or a new failure.
• wheels upgrade check — unchanged read-only scan, including --strict, --format=json, and the Wheels.UpgradeCheckFailed exit-code contract. Only its closing hint changes: Apply with: wheels upgrade apply instead of the misleading brew upgrade wheels. Note: wheels upgrade check --help now prints usage instead of running the scan (help wins over any verb).

This unlocks the bleeding-edge flow:

brew uninstall wheels && brew install wheels-be
wheels upgrade apply      # BE framework into the app, stable parked in vendor/wheels.bak-*

Failure-mode UX (review fix)

The plan is announced before any mutation, with the exact reserved backup destination and the recovery one-liner:

Backing up vendor/wheels -> vendor/wheels.bak-20260611-141502
If this is interrupted, restore with:
  rm -rf "/path/to/app/vendor/wheels" && mv "/path/to/app/vendor/wheels.bak-20260611-141502" "/path/to/app/vendor/wheels"

The backup path is reserved up front (FrameworkUpgrader.reserveBackupPath(), now public) and passed back into applyUpgrade(), so the announcement and the actual backup can never disagree. The create+copy step is wrapped: a mid-copy failure throws Wheels.FrameworkUpgrader.CopyFailed whose message names the partial-state target and the backup to restore from (quoted paths) — or, with --nobackup, says the old tree is gone and points at re-running wheels upgrade apply after fixing the cause. Module-level dispatch catches it and rethrows Wheels.UpgradeApplyFailed after printing, mirroring validate()'s print-then-throw convention (#2941).

Safety rails (all fire before any mutation)

• Source and target must sniff as a Wheels framework dir — and the sniff is hardened (review fix): a bare box.json is no longer evidence (every CommandBox-era project has one). Required: wheels.json with a non-empty version, or box.json whose version is non-empty and whose name/slug (when present) identifies a wheels artifact. A generic app box.json ({"name":"myapp", ...}) is refused on both sides.
• Identity/containment guard: running inside the wheels repo checkout itself resolves the bundled source to the very directory being replaced — the backup rename (or --nobackup delete) would destroy the source mid-swap. Refused, along with either direction of nesting.
• Refuses outside a Wheels app (no vendor/wheels/).
• --to=<version> is an assertion: must equal the bundled framework version or the command errors with the brew/scoop pointer (PR2 lifts this).
• Typo'd subcommands (wheels upgrade chekc) and unknown flags hard-stop (Wheels.InvalidArguments); check-only flags on the apply verb (--strict, --format, --dry-run) nudge toward wheels upgrade check and refuse.

Design notes

• The swap lives in cli/lucli/services/FrameworkUpgrader.cfc, isolated from Module.cfc (mirrors FrameworkInstaller.cfc) so file-level behavior is spec'd without the LuCLI runtime.
• Bundled-source resolution: WHEELS_FRAMEWORK_PATH override first (invalid path hard-fails, same semantics as resolveFrameworkSource() / wheels new: explicit WHEELS_FRAMEWORK_PATH should hard-fail when it doesn't exist #2215), else walk up from the module's own install location. The project-root candidate resolveFrameworkSource() prefers is deliberately skipped — it's the swap target.
• parseUpgradeArgs accepts subcommand as a named key as well as positionally: the MCP inputSchema (roadmap: high-impact CLI/MCP tooling-honesty gaps — failure exit codes (incl. upgrade check) + MCP tool input schemas #2963) advertises it as a named property; {subcommand: "apply"} is the only MCP shape that may mutate, {} prints usage.
• upgradeArgSpec() documents the explicit-verb contract, so CLI parsing, MCP tools/list schema, and help can't drift.

Tests

RED→GREEN against the local CLI harness (tools/test-cli-local.sh, Lucee 7 + SQLite):

• GREEN (this branch): 902 pass / 0 fail / 0 error.
• RED (review-fix specs against the pre-fix implementation): 881 pass / 14 fail / 7 error — exactly the 21 new/flipped specs (hardened sniff, reserveBackupPath, CopyFailed contract incl. a real mid-copy failure simulated via an unreadable source file, bare-verb usage steer, apply-verb dispatch, pre-swap announcement).
• Core cli area (tools/test-local.sh wheels.tests.specs.cli): 79 pass (was 78; +1 pin that the help advertises wheels upgrade apply).

New/updated specs:

• FrameworkUpgraderSpec (34) — hardened sniff matrix (generic box.json refused, version-only legacy box.json accepted, malformed/empty-version refused), version reads, swap, backup naming/collision, caller-reserved backup path honored, --nobackup, identity/containment refusals, fresh-install path, missing-parent refusal, and the two CopyFailed contracts (with-backup names the backup to restore; --nobackup says the tree is gone). Fixture cleanup now lives in afterEach hooks.
• UpgradeApplyCommandSpec (19) — help paths, bare-verb usage steer (with and without stray apply flags, vendor untouched), every pre-mutation refusal on the apply verb, the real swap + backup, the pre-swap announcement (exact backup dir + quoted recovery command, ordered before the swap summary in the output), --to assert-match, --nobackup, MCP named-subcommand dispatch for both verbs.
• InfoCommandSpec — bare verb pins the usage steer; apply over the empty stub pins the sniff refusal.
• Core UpgradeCommandHelpSpec — pins the explicit-verb surface (summary, hint mentions both verbs, wheels upgrade apply advertised, --dry-run gap named, backup convention referenced).

Follow-ups

• PR2 (wheels upgrade: apply mode — swap the app's vendored framework from the CLI bundle #3035): download arbitrary --to= targets via ReleaseChannel.
• Guides refresh: command-line-tools/wheels-commands/upgrade/ and the upgrading guides still describe the check-only command and the manual zip swap.
• Cap the bundled-source walk-up depth and surface @build.version@ more nicely in repo checkouts (noted on wheels upgrade: apply mode — swap the app's vendored framework from the CLI bundle #3035).

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: This PR turns the bare wheels upgrade verb into an actual framework swap from the CLI's bundled copy (PR1 of #3035), with a backup-by-default rename, a thorough set of pre-mutation refusals, and 37 new specs that exercise both the file-level service and the dispatch layer. The guard ordering is correct (identity/containment check fires before any rename or delete — verified at cli/lucli/services/FrameworkUpgrader.cfc:101-117 against the spec at FrameworkUpgraderSpec.cfc "refuses when source and target resolve to the same directory", which also asserts no backup was created), the MCP named-subcommand dispatch hole is closed, and the commit and changelog fragment both conform. Verdict: comment — no blocking findings, five non-blocking notes below.

Correctness

1. A copy failure after the backup rename bypasses the print-then-throw guidance — in exactly the scenario where the user most needs it. In applyUpgrade (cli/lucli/services/FrameworkUpgrader.cfc:129-139), once $renameDirectory(arguments.vendorDir, result.backupDir) has run, the subsequent directoryCreate / directoryCopy are not wrapped in try/catch. If the copy throws (disk full, permissions), the raw exception propagates and runUpgradeApply (cli/lucli/Module.cfc:4894) never reaches its if (!result.success) branch — so the Backup: ... path and the Recover with: rm -rf ... && mv ... command are never printed, even though vendor/wheels/ is now missing and the backup is the only copy. The process still exits non-zero (good), but the #2941 print-guidance-then-throw convention is lost on this one path. Suggestion: wrap steps 4–5 in try/catch inside applyUpgrade and return result.success = false with an error that includes result.backupDir when it's set. Non-blocking because the backup directory is discoverable with ls vendor/, and the window is narrow.

2. The framework sniff accepts any directory containing a box.json (cli/lucli/services/FrameworkUpgrader.cfc:25). box.json is the generic CommandBox manifest — an app root, a package, or most CFML projects carry one, so WHEELS_FRAMEWORK_PATH pointed at a non-framework directory with a box.json would pass the source sniff and get copied over vendor/wheels/. The docblock honestly calls it a "quick sniff" and the backup default bounds the damage, so this is a nit — but readFrameworkVersion already parses the manifest, so checking name == "wheels" when the field is present would be nearly free and would tighten both the source and target guards.

Conventions

3. wheels_upgrade stays MCP-exposed while its no-arg shape became destructive — worth a deliberate confirmation that this is intended. mcpHiddenTools() hides new explicitly because it is "destructive (new scaffolds a whole project)" (cli/lucli/Module.cfc:162), and upgrade is not in that list, so an MCP client calling wheels_upgrade with no arguments now swaps vendor/wheels/ where it previously printed usage. The PR clearly thought about this surface (the named-subcommand fallback in parseUpgradeArgs, the schema regenerated from upgradeArgSpec().toInputSchema() at cli/lucli/Module.cfc:252, and the new positional description warning that omitting the subcommand applies the swap), and backup-by-default plus the sniff rails make it recoverable — so I'm not asking for a change, just flagging that the new-is-hidden precedent points the other way. If you want to keep upgrade check reachable over MCP while de-fanging the bare call, one option is requiring an explicit subcommand: "apply" on the MCP path only; happy for this to be a wontfix with rationale.

Tests

4. The --no-backup normalization branch has no spec. parseUpgradeArgs (cli/lucli/Module.cfc, the doBackup block) honors LuCLI's --no-backup → backup = "false" normalization alongside the documented --nobackup, but UpgradeApplyCommandSpec.cfc only drives nobackup = true ("accepts --to= matching the bundled version and skips the backup with --nobackup"). A one-liner spec passing backup = "false" would pin the second spelling. Nit.

Otherwise the coverage is genuinely strong: every refusal asserts the vendor manifest is untouched as a side-effect check, the identity-guard spec asserts no .bak- sibling exists (proving the guard fired before the rename), and the InfoCommandSpec rewrite correctly converts the old silently-empty-dispatch specs into a pinned sniff-refusal.

Docs

5. The PR body already names it, so just confirming it's tracked: web/sites/guides/src/content/docs/.../command-line-tools/wheels-commands/upgrade/ and the upgrading guides still describe the check-only command and the manual zip swap, which is now actively wrong user-facing documentation for a destructive verb. Please make sure the follow-up has an issue so it doesn't ride only on the PR description. The changelog fragment (changelog.d/upgrade-apply-mode.added.md) is present and correctly uses the fragment system rather than editing CHANGELOG.md.

Commits

Single commit feat(cli): wheels upgrade applies the framework swap from the CLI bundle — valid type, valid scope, subject under 100 chars. Conforms to commitlint.config.js.

---

Cross-engine: the only file in the all-engines core suite is vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc, and its changes are plain string assertions with correctly escaped ##3035 literals inside string concatenation — no new closure/scope/tag patterns. The CLI code itself runs only on the bundled Lucee per cli/CLAUDE.md, and the left(srcCanonical & separator, ...) containment comparison can't hit the Left(str, 0) Lucee 7 trap since the separator guarantees a non-zero length.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: Re-review at the review-fix head (7f82d2a). The three substantive findings from the prior review are genuinely addressed: the explicit apply verb makes bare wheels upgrade (and an MCP wheels_upgrade call with {}) inert, looksLikeWheelsFramework no longer accepts a bare box.json (generic-app manifests are refused on both sides, spec'd in the new sniff matrix), and a mid-copy failure now throws Wheels.FrameworkUpgrader.CopyFailed naming the restore path, exercised by a real unreadable-source-file simulation. Coverage is strong (RED→GREEN documented; every refusal spec asserts vendor is untouched as a side-effect check). However, the fix commit introduces one correctness issue of its own: the pre-swap announcement — including the destructive rm -rf … && mv … restore one-liner — is printed before the service-level pre-mutation validation runs, so every service refusal path tells the user to restore from a backup that was never created. Verdict: request changes for that one finding; the other two notes are non-blocking.

Correctness

1. [blocking] Refusal paths print a restore command for a backup that never existed — running it deletes the intact vendor/wheels/. runUpgradeApply prints the plan at cli/lucli/Module.cfc:4941-4951:

   Backing up vendor/wheels -> vendor/wheels.bak-<ts>
   If this is interrupted, restore with:
     rm -rf "<vendorDir>" && mv "<backupPath>" "<vendorDir>"
   

   — before applyUpgrade() runs its own pre-mutation refusals: source sniff (cli/lucli/services/FrameworkUpgrader.cfc:141), identity (:165), containment (:173), and target sniff (:181) all return result.error, which Module prints only afterwards at Module.cfc:4965-4967. On those paths no backup is created and vendor/wheels/ is untouched — but the user is now holding a one-liner whose first half is rm -rf "<vendorDir>" and whose second half (mv) will fail because the backup does not exist. Paste it after a refusal and you have deleted a perfectly intact vendor/wheels/ with no backup. These paths are reachable, not theoretical: the identity refusal is the PR's own headline scenario (its message at FrameworkUpgrader.cfc:166 literally asks whether you are running inside the wheels repo checkout), and the target-sniff refusal is exactly what InfoCommandSpec.cfc's "apply verb refuses over the empty vendor/wheels stub" spec drives. It also contradicts the PR body's claim that "the announcement and the actual backup can never disagree" — they disagree on every refusal path (announced, never made). Suggested fix: extract the step 1–4 checks (everything before the rename/delete) into a public validateSwap(sourceDir, vendorDir) on FrameworkUpgrader, call it in runUpgradeApply before printing the plan (print-then-throw on error, per the #2941 convention this PR already follows elsewhere), and have applyUpgrade() keep re-running it — the checks are idempotent reads, so there is no drift risk. Then extend the existing refusal specs with an output assertion that the rm -rf line is absent.

2. [non-blocking] RenameFailed bypasses the documented exit contract. The runUpgradeApply docblock (cli/lucli/Module.cfc:4885-4887) promises "Every refusal throws Wheels.UpgradeApplyFailed AFTER printing the guidance", but $renameDirectory's Wheels.FrameworkUpgrader.RenameFailed (cli/lucli/services/FrameworkUpgrader.cfc:250-255) propagates uncaught — the catch at Module.cfc:4956 matches only the CopyFailed leaf type. No data risk (a false renameTo leaves vendor/wheels/ in place), so this is contract consistency only. Cheapest fix: broaden the catch to the parent prefix catch (Wheels.FrameworkUpgrader e) — CFML's hierarchical exception-type matching catches both leaf types — so any service-thrown failure gets the same print-then-rethrow treatment.

Tests

3. [nit, carried over] The --no-backup normalization branch is still unspec'd. parseUpgradeArgs honors LuCLI's --no-backup → backup=false normalization at cli/lucli/Module.cfc:2860-2862, but the specs only drive nobackup = true (UpgradeApplyCommandSpec.cfc:110 and :213). This was finding 4 of the prior review; a one-liner spec passing backup = false alongside arg1 = "apply" would pin the second spelling.

Docs

changelog.d/upgrade-apply-mode.added.md correctly uses the fragment system, and the guides refresh the prior review asked to be tracked now exists as a checklist item on #3035 (with bot-update-docs.yml covering the .ai//guides sweep separately). No action needed.

Commits

Both commits (feat(cli): wheels upgrade applies the framework swap from the CLI bundle, fix(cli): explicit apply verb, pre-swap recovery guidance, hardened source sniff) conform to commitlint.config.js — valid types, cli scope, subjects under 100 chars, DCO sign-off present. The fix commit message is an unusually good "why" record.

---

Cross-engine: the only all-engines file is vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc; its changes are plain string/contains assertions with correctly escaped ##3035/##3039 literals and no new closure/scope/tag patterns — the contains operator and \b regexes are pre-existing idioms in that same file. Security: no findings — no SQL/XSS surface, the CLI never executes the printed recovery command itself, and WHEELS_FRAMEWORK_PATH is operator-controlled with the hardened sniff bounding what it can inject.

[bpamiri]

7f82d2a (pushed before the review posted) addresses notes 1–3:

1. Copy failures now caught with pre-swap recovery guidance — applyUpgrade wraps the file-copy in a try/catch; on failure it prints the reserved backup path (which was announced before any mutation started) alongside the exact restore command, then exits non-zero via Wheels.UpgradeApplyFailed. Users are never left with a half-swapped vendor/wheels/ without a recovery path.

2. Source sniff hardened — the Wheels-identity check now requires wheels.json to be present and to contain a "name": "wheels" (or "cfwheels") field. A bare directory with no manifest, or a manifest without the identity field, is rejected before the swap begins.

3. Resolved MORE conservatively than suggested — bare wheels upgrade never mutates on any transport, not just MCP. The suggestion was to guard MCP callers; instead the apply verb is required everywhere. wheels_upgrade({}) and wheels upgrade both print usage steering and exit 0 — destructive commands deserve an explicit opt-in.

Note 4 is pinned in 0b4b668 (test(cli): pin the --no-backup normalization spelling on upgrade apply).

Note 5 is now tracked in #3045.

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: Re-review at head 0b4b668. The only delta since the prior changes-requested review (at 7f82d2a) is commit 0b4b6680d, a 12-line spec that resolves the prior review's test nit — the --no-backup → backup="false" normalization branch at cli/lucli/Module.cfc:2860-2862 is now pinned by a sibling spec (UpgradeApplyCommandSpec.cfc:219-229) that drives the exact LuCLI-normalized named-arg shape and asserts no .bak- sibling is created. That spec is accurate and well-placed. However, the blocking finding from the prior review is unaddressed: the pre-swap plan — including the rm -rf … && mv … restore one-liner — still prints before the service-level pre-mutation refusals run, so every refusal path hands the user a restore command for a backup that was never created. The non-blocking RenameFailed contract gap also remains. Both were re-verified against the code at this head, not carried over on trust. Verdict: request changes — same blocking finding, unchanged.

Correctness

1. [blocking, carried over — unaddressed] Refusal paths print a restore command for a backup that never existed; running it deletes the intact vendor/wheels/. runUpgradeApply still prints the plan at cli/lucli/Module.cfc:4941-4951:

   Backing up vendor/wheels -> vendor/wheels.bak-<ts>
   If this is interrupted, restore with:
     rm -rf "<vendorDir>" && mv "<backupPath>" "<vendorDir>"
   

   — before applyUpgrade() runs at Module.cfc:4955. The service's pre-mutation refusals — source sniff (cli/lucli/services/FrameworkUpgrader.cfc:141-144), missing parent (:151-154), identity (:165-168), containment (:173-177), target sniff (:181-184) — all return result.error, which Module prints only afterwards at Module.cfc:4965-4967. On those paths no backup exists and vendor/wheels/ is intact, but the user is holding a one-liner whose first half is rm -rf "<vendorDir>" and whose second half (mv) will fail because the backup was never made. These paths are reachable, not theoretical: the identity refusal is the PR's own headline scenario (FrameworkUpgrader.cfc:166 literally asks "Are you running wheels upgrade inside the wheels repo checkout?"), and the target-sniff refusal is exactly what InfoCommandSpec.cfc's "apply verb refuses over the empty vendor/wheels stub" spec drives. It also contradicts the PR body's claim that "the announcement and the actual backup can never disagree" — they disagree on every refusal path (announced, never made).

   Suggested fix (unchanged from the prior review): extract the step 1–4 checks in applyUpgrade() (everything before the rename/delete at FrameworkUpgrader.cfc:187-192) into a public validateSwap(sourceDir, vendorDir), call it in runUpgradeApply before printing the plan (print-then-throw on error, per the #2941 convention this PR already follows elsewhere), and have applyUpgrade() keep re-running it — the checks are idempotent reads, so there is no drift risk. Then extend the existing refusal specs with an output assertion that the rm -rf line is absent.

2. [non-blocking, carried over — unaddressed] RenameFailed bypasses the documented exit contract. The runUpgradeApply docblock (cli/lucli/Module.cfc:4886-4889) promises every refusal throws Wheels.UpgradeApplyFailed after printing the guidance, but $renameDirectory's Wheels.FrameworkUpgrader.RenameFailed (cli/lucli/services/FrameworkUpgrader.cfc:250-255, thrown from applyUpgrade() at :189) propagates uncaught — the catch at Module.cfc:4956 matches only the CopyFailed leaf type. No data risk (a false renameTo leaves vendor/wheels/ in place), so this is contract consistency only. Cheapest fix: broaden the catch to the parent prefix catch (Wheels.FrameworkUpgrader e) — CFML's hierarchical exception-type matching catches both leaf types — so any service-thrown failure gets the same print-then-rethrow treatment.

Tests

Resolved: the prior review's nit on the unspec'd --no-backup normalization branch is fixed by 0b4b6680d. The new spec (cli/lucli/tests/specs/commands/UpgradeApplyCommandSpec.cfc:219-229) drives mod.upgrade(argumentCollection = {"arg1": "apply", "backup": "false"}) — the exact shape LuCLI's --no-backup negation normalizes to — and asserts the swap completes with zero .bak- siblings, mirroring the adjacent --nobackup spec at :211-217. It exercises the real branch (Module.cfc:2860-2862 flips doBackup only when coll.backup == "false"), and backup is in the apply verb's knownKeys allowlist (Module.cfc:2978) so it doesn't trip the unknown-flag hard-stop. No further test asks.

Commits

The new commit test(cli): pin the --no-backup normalization spelling on upgrade apply conforms to commitlint.config.js — valid test type, cli scope, subject under 100 chars, not ALL-CAPS, DCO sign-off present, and the body records the "why" (bot note 4, PR #3039). The two earlier commits were reviewed and passed previously; unchanged.

---

Cross-engine and security posture are unchanged from the prior review at 7f82d2a — the only all-engines file remains vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc (plain string/contains assertions, correctly escaped ## literals), and the new spec runs only under the Lucee-hosted CLI harness. No new findings in either category.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: Re-review at head d87e24c. The delta since the prior changes-requested review (at 0b4b668) is a clean merge of origin/develop (combined diff is empty — no conflict resolutions) plus commit d87e24c35, which resolves both outstanding findings exactly along the lines the prior review suggested: the pre-mutation refusal checks are extracted into a public validateSwap() that runUpgradeApply now calls before printing the plan, so no refusal path ever shows the rm -rf … && mv … restore one-liner for a backup that was never made; and the Module-level catch is broadened to the Wheels.FrameworkUpgrader parent prefix so RenameFailed gets the documented print-then-throw exit contract. Each fix was re-verified against the code at this head and is pinned by new RED-first specs. Verdict: approve — this supersedes the bot's earlier CHANGES_REQUESTED reviews (issue #3048), whose findings are all resolved per the audit trail below.

Previously blocking — resolved

1. Refusal paths printed a restore command for a backup that never existed (blocking at 7f82d2a and 0b4b668). Fixed:

   • cli/lucli/services/FrameworkUpgrader.cfc:110-153 — the four pre-mutation checks (source sniff, missing parent, identity/containment, target sniff) now live in a public validateSwap(sourceDir, vendorDir) that is pure reads and returns "" or the refusal text.
   • cli/lucli/Module.cfc:4941-4945 — runUpgradeApply calls validateSwap() before the Source:/Target: lines and the plan block (Module.cfc:4947-4967); a refusal prints the error and throws Wheels.UpgradeApplyFailed (print-then-throw, the #2941 convention) without ever reaching the restore one-liner.
   • cli/lucli/services/FrameworkUpgrader.cfc:199-202 — applyUpgrade() re-runs validateSwap() first, so direct service callers keep the identical refusal behavior; the checks are idempotent reads, no drift risk.
   • Ordering verified at this head: every Module-level early refusal (no vendor/wheels at Module.cfc:4901, missing bundled source at :4911, --to mismatch at :4923) and the validateSwap gate all fire before any plan output. The only remaining reserveBackupPath() call before the plan (:4960) is a pure read (FrameworkUpgrader.cfc:250-259 only loops directoryExists and returns a string).
   • Pinned by RED-first specs: the new ModuleOutputCapture fixture (cli/lucli/tests/_fixtures/commands/ModuleOutputCapture.cfc, mirroring the existing ModuleArgvProbe fixture pattern, with an out() override that exactly matches the test double's signature in cli/lucli/tests/_modules/BaseModule.cfc:27) lets specs assert printed output. UpgradeApplyCommandSpec.cfc:174-198 drives a real service-level refusal and asserts rm -rf and the backup announcement are absent (commit message records it failing pre-fix); the other refusal specs add absence assertions at :163, :171, :193; the success path pins presence at :246; and FrameworkUpgraderSpec.cfc:214-259 pins the validateSwap contract directly (six specs: valid pair, fresh install, source sniff, identity, target sniff, missing parent with no side effects).

2. RenameFailed bypassed the documented exit contract (non-blocking at 0b4b668). Fixed at cli/lucli/Module.cfc:4972 — the catch is broadened from the CopyFailed leaf to catch (Wheels.FrameworkUpgrader e); CFML's hierarchical exception matching now covers both service-thrown leaf types (CopyFailed at FrameworkUpgrader.cfc:228/233, RenameFailed at :274), so every service failure gets the same print-then-rethrow Wheels.UpgradeApplyFailed treatment the runUpgradeApply docblock promises.

3. Earlier rounds (resolved at 7f82d2a/0b4b668, re-confirmed unchanged at this head): explicit apply verb (bare wheels upgrade and MCP {} are inert), hardened looksLikeWheelsFramework sniff (generic app box.json refused on both sides), pre-swap announcement with caller-reserved backup path, and the --no-backup normalization spec (UpgradeApplyCommandSpec.cfc:219-229-region, added in 0b4b668).

Tests

The new coverage is well-aimed: the output-capture fixture closes a real observability gap (the test double's out() is a no-op, so output-ordering bugs were previously invisible to specs), and the refusal specs assert both the explanation's presence and the restore line's absence, plus no-mutation side-effect checks (UpgradeApplyCommandSpec.cfc:196-198). The validateSwap describe block exercises every refusal branch the extraction moved. No further test asks.

Commits

d87e24c35 — fix(cli): validate the swap before announcing the restore one-liner on upgrade apply — conforms to commitlint.config.js: valid fix type, header 86 chars, not ALL-CAPS, DCO sign-off matches the author email, and the body records the why (the #3039 review finding and the RED-first test evidence). The merge commit e0df666e5 is a standard merge (exempt from the squash-merge gate; the PR title itself is a valid conventional-commit header). Earlier commits reviewed previously; unchanged.

Non-blocking polish (optional, no action required to merge)

The announce-before-mutate design necessarily leaves one narrow window: if the backup rename itself fails (RenameFailed), the plan's conditional restore one-liner is already on screen while no backup exists — though vendor/wheels/ is intact, the failure prints an explicit red error, and the plan's wording ("If this is interrupted") doesn't apply to a clean failure. If you want belt-and-braces, append to the RenameFailed message at FrameworkUpgrader.cfc:275 something like "No backup was created and vendor/wheels/ is untouched — disregard the restore command above." Fine as a follow-up or never.

Cross-engine and security posture are unchanged from the prior rounds: the only all-engines file remains vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc (untouched in this delta); the new fixture and specs run only under the Lucee-hosted CLI harness. No new findings.

This approval supersedes the bot's CHANGES_REQUESTED reviews at 7f82d2a and 0b4b668 (issue #3048 — a comment-state re-review would leave the stale merge block active).