fix(cli): upgrade-check exit code, breaker-scan coverage, ArgSpec positional gaps (#2941)

U1: wheels upgrade check now throws Wheels.UpgradeCheckFailed after the
report flushes when breaking findings exist (mirrors validate()'s
Wheels.ValidationFailed), so it exits non-zero and can gate CI. Adds
--format=json for a machine-readable report; advisories never affect
the exit code.

U4: breaker scan covers the canonical eleven — the WireBox check now
matches wirebox.system.ioc and scans the root Application.cfc + config/
in addition to app/; new breaking grep for removed renderPage()/
renderPageToString(); new advisories for the HSTS default flip
(SecurityHeaders, #2081) and the CSRF SameSite cookie attribute
(protectsFromForgery, #2035). UpgradeCheckCoverageSpec pins the full
set plus the exit-code throw and --format=json plumbing.

U5: wheels.Testbox shim now documented with a 5.0 removal target (CFC
docblock + 3x-to-4x guide), mapperModernSpec migrated off it to
wheels.WheelsTest, and the upgrade-check test-base-class grep matches
wheels.Testbox and single-quoted extends forms.

U7: renamed the scanTargets loop var that shadowed the target-version
local; fixed the dead 2.x->3.x app/plugins check to the real root
plugins/ path (skipped on 2.x->4.x jumps where the 4.x entry covers
it); breaking-finding output now links the version-appropriate upgrade
guide and the wheels-legacy-adapter soft landing; new in-repo
regression spec for wheels-legacy-adapter registry resolution.

CLI-D5: structuredArgs() now consumes __arguments once (cleared on
every call), so a delegation stash (create/generate app -> new) can
never replay stale argv into a later zero-arg tool call under the
persistent stdio MCP server.

CLI-D1: ArgSpec.parse()/toArgv() bind positionals by collecting and
numerically sorting arg<N> keys (parseTestArgs' proven pattern) instead
of fixed-index probing / stop-at-gap loops. LuCLI numbers positionals
by global token index, so a flag between positionals leaves a gap —
'wheels new --port=3000 blog' previously ignored the app name and
threw, and 'wheels g scaffold Post --force title:string body:text'
silently dropped every column after the flag.

Verified locally (Lucee 7 + SQLite, worktree docker recipe): core cli
specs 78/78, mapperModernSpec 27/27, CLI suite 774 pass with only the
pre-existing environment-dependent SSH/live-server failures.

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: bpamiri

cli/lucli/Module.cfc

@@ -98,12 +98,20 @@ component extends="modules.BaseModule" {
 	 * `create` → `new`, and unit tests). That array is reconstructed into the
 	 * same structured shape LuCLI would have produced, so a command behaves
 	 * identically whether LuCLI dispatched it or another command delegated to it.
+	 *
+	 * `__arguments` is consume-once: it is cleared on every call so a stale
+	 * stash can never replay. One-shot CLI runs hid the leak, but the stdio
+	 * MCP server (`wheels mcp wheels`) is a persistent process — after any
+	 * delegating call (create/generate app → new) a later zero-arg tool call
+	 * (e.g. wheels_test()) would otherwise re-parse the stale argv and turn
+	 * into a completely different invocation.
 	 */
 	private struct function structuredArgs(struct callerArgs = {}) {
+		var raw = __arguments ?: [];
+		__arguments = [];
 		if (!structIsEmpty(arguments.callerArgs)) {
 			return arguments.callerArgs;
 		}
-		var raw = __arguments ?: [];
 		return argvToCollection(isArray(raw) ? raw : []);
 	}
 
@@ -2724,10 +2732,12 @@ component extends="modules.BaseModule" {
 		var parsed = new services.ArgSpec()
 			.positional(name = "subcommand", default = "")
 			.option(name = "to", default = "")
+			.option(name = "format", default = "")
 			.parse(arguments.coll);
 		return {
 			isCheck = lCase(parsed.subcommand) == "check",
 			targetVersion = parsed.to,
+			format = parsed.format,
 			sawTo = structKeyExists(arguments.coll, "to"),
 			sawDryRun = structKeyExists(arguments.coll, "dry-run")
 		};
@@ -2744,9 +2754,14 @@ component extends="modules.BaseModule" {
 	 * Despite occasional appearances in older help output, `--dry-run` is not
 	 * supported — the command is already read-only by design.
 	 *
+	 * Breaking findings throw Wheels.UpgradeCheckFailed after the report is
+	 * printed, so the command exits non-zero and can gate CI. Advisory
+	 * (opt-in recommendation) findings never affect the exit code.
+	 *
 	 * Examples:
-	 *   wheels upgrade check                 - scan against the latest stable release
-	 *   wheels upgrade check --to=4.0.0      - scan against a specific target version
+	 *   wheels upgrade check                       - scan against the latest stable release
+	 *   wheels upgrade check --to=4.0.0            - scan against a specific target version
+	 *   wheels upgrade check --format=json         - machine-readable report (CI pipelines)
 	 */
 	public string function upgrade() {
 		var opts = parseUpgradeArgs(structuredArgs(arguments));
@@ -2760,6 +2775,10 @@ component extends="modules.BaseModule" {
 				& nl
 				& "Options:" & nl
 				& "  --to=<version>    Target Wheels version (default: latest stable)" & nl
+				& "  --format=json     Emit a machine-readable JSON report" & nl
+				& nl
+				& "Exit status:" & nl
+				& "  Non-zero when breaking changes are found (advisories never fail the check)." & nl
 				& nl
 				& "Unsupported flags:" & nl
 				& "  --dry-run is not supported — the command is already read-only," & nl
@@ -2781,7 +2800,7 @@ component extends="modules.BaseModule" {
 			return help;
 		}
 
-		return runUpgradeCheck(opts.targetVersion);
+		return runUpgradeCheck(opts.targetVersion, opts.format);
 	}
 
 	// ─────────────────────────────────────────────────
@@ -4039,8 +4058,15 @@ component extends="modules.BaseModule" {
 
 	/**
 	 * Scan app for breaking changes between current and target version.
+	 *
+	 * Breaking findings throw Wheels.UpgradeCheckFailed after the report is
+	 * flushed (same pattern as validate()'s Wheels.ValidationFailed), so the
+	 * command exits non-zero and can gate CI. Advisory findings never affect
+	 * the exit code. `format="json"` replaces the human report with a single
+	 * JSON document for pipelines.
 	 */
-	private string function runUpgradeCheck(string targetVersion = "") {
+	private string function runUpgradeCheck(string targetVersion = "", string format = "") {
+		var jsonMode = lCase(arguments.format) == "json";
 		// Detect current version. Prefer wheels.json (post-rename) and fall back
 		// to box.json so apps with pre-rename vendor/wheels/ committed in their
 		// repo still work. The fallback can be removed two releases after the
@@ -4066,21 +4092,24 @@ component extends="modules.BaseModule" {
 				var releaseData = deserializeJSON(response);
 				target = replace(releaseData.tag_name, "v", "");
 			} catch (any e) {
-				out("Could not fetch latest version. Use --to=<version> to specify.", "yellow");
+				var fetchMsg = "Could not fetch latest version. Use --to=<version> to specify.";
+				out(jsonMode ? serializeJSON({"error": fetchMsg}) : fetchMsg, "yellow");
 				return "";
 			}
 		}
 
-		out("Current version: #currentVersion#", "bold");
-		out("Target version:  #target#", "bold");
-		out("");
+		if (!jsonMode) {
+			out("Current version: #currentVersion#", "bold");
+			out("Target version:  #target#", "bold");
+			out("");
+		}
 
 		// Compare major versions
 		var currentMajor = val(listFirst(currentVersion, "."));
 		var targetMajor = val(listFirst(target, "."));
 		var sameMajor = (currentMajor == targetMajor);
 
-		if (sameMajor) {
+		if (sameMajor && !jsonMode) {
 			out("Same major version — no known breaking changes.", "green");
 			out("Scanning for opt-in recommendations...", "green");
 			out("");
@@ -4094,16 +4123,23 @@ component extends="modules.BaseModule" {
 
 		// 2.x -> 3.x
 		if (currentMajor <= 2 && targetMajor >= 3) {
+			// 2.x plugins lived at the webroot's /plugins (the previous
+			// `app/plugins` path never existed in any Wheels layout, so the
+			// check was dead). The 3.x -> 4.x block adds an identical root
+			// /plugins check, so skip this one on a 2.x -> 4.x jump to avoid
+			// reporting the same directory twice.
+			if (targetMajor < 4) {
+				arrayAppend(checks, {
+					description: "Legacy plugin directory",
+					pattern: "",
+					checkType: "directory",
+					path: "plugins",
+					fix: "Migrate to packages/ + vendor/ activation model"
+				});
+			}
 			arrayAppend(checks, {
-				description: "Legacy plugin directory",
-				pattern: "",
-				checkType: "directory",
-				path: "app/plugins",
-				fix: "Migrate to packages/ + vendor/ activation model"
-			});
-			arrayAppend(checks, {
-				description: "Old test base class (wheels.Test)",
-				pattern: 'extends\s*=\s*"wheels\.Test"',
+				description: "Old test base class (wheels.Test / wheels.Testbox)",
+				pattern: 'extends\s*=\s*["'']wheels\.Test(box)?["'']',
 				checkType: "grep",
 				scanDir: "tests",
 				extensions: "cfc",
@@ -4120,21 +4156,68 @@ component extends="modules.BaseModule" {
 				path: "plugins",
 				fix: "Migrate to packages/ + vendor/ system"
 			});
+			// Matches both quote styles and the silent wheels.Testbox shim
+			// (deprecated alias of wheels.WheelsTest, removal target 5.0) —
+			// the previous double-quote-only wheels.Test pattern missed both.
 			arrayAppend(checks, {
-				description: "Old test base class (wheels.Test)",
-				pattern: 'extends\s*=\s*"wheels\.Test"',
+				description: "Old test base class (wheels.Test / wheels.Testbox)",
+				pattern: 'extends\s*=\s*["'']wheels\.Test(box)?["'']',
 				checkType: "grep",
 				scanDir: "tests",
 				extensions: "cfc",
 				fix: 'Change to extends="wheels.WheelsTest"'
 			});
+			// application.wirebox → application.wheelsdi (guide item 10). The
+			// hardest real-world case is a root Application.cfc bootstrap that
+			// calls `new wirebox.system.ioc.Injector(...)` — the WireBox
+			// package no longer ships in vendor/wheels/ — so scan the root
+			// Application.cfc and config/ in addition to app/.
+			arrayAppend(checks, {
+				description: "Direct WireBox references (application.wirebox / wirebox.system.ioc)",
+				pattern: "application\.wirebox|wirebox\.system\.ioc",
+				checkType: "grep",
+				scanDir: "app",
+				extensions: "cfc,cfm",
+				scanTargets: [
+					{path: "Application.cfc"},
+					{path: "config", extensions: "cfm,cfc", recurse: true}
+				],
+				fix: "Use service() / application.wheelsdi instead of application.wirebox; replace `new wirebox.system.ioc.Injector(...)` bootstraps with `new wheels.Injector()`. The legacy adapter does NOT shim this item."
+			});
+			// renderPage()/renderPageToString() removed in 4.0 — shimmed by
+			// the optional wheels-legacy-adapter package, but unshimmed apps
+			// throw at first render.
 			arrayAppend(checks, {
-				description: "Direct WireBox references",
-				pattern: "application\.wirebox",
+				description: "Removed renderPage()/renderPageToString() helpers",
+				pattern: "renderPage(ToString)?\s*\(",
 				checkType: "grep",
 				scanDir: "app",
 				extensions: "cfc,cfm",
-				fix: "Use service() or inject() from the DI container instead"
+				fix: 'Replace renderPage() with renderView() and renderPageToString() with renderView(returnAs="string"), or install the soft-landing shim: wheels packages add wheels-legacy-adapter'
+			});
+			// HSTS defaults on in production (guide item 2, ##2081). Advisory:
+			// fires on SecurityHeaders usage so proxied/LB setups know the
+			// header now emits by default.
+			arrayAppend(checks, {
+				description: "SecurityHeaders middleware — HSTS defaults on in production in 4.0 (advisory)",
+				severity: "advisory",
+				pattern: "new\s+wheels\.middleware\.SecurityHeaders",
+				checkType: "grep",
+				scanDir: "config",
+				extensions: "cfm,cfc",
+				fix: "4.0 emits Strict-Transport-Security (max-age=31536000; includeSubDomains) by default in production. Pass hsts=false to the middleware if your load balancer already sets it."
+			});
+			// CSRF cookie now sets SameSite (guide item 6, ##2035). Advisory:
+			// fires on CSRF protection usage — same-site flows are unaffected,
+			// but cross-site POSTs from third-party frames will break.
+			arrayAppend(checks, {
+				description: "CSRF cookie sets SameSite in 4.0 (advisory: review cross-site POST flows)",
+				severity: "advisory",
+				pattern: "protectsFromForgery",
+				checkType: "grep",
+				scanDir: "app",
+				extensions: "cfc",
+				fix: "The CSRF cookie now sets the SameSite attribute. Cross-site POSTs from third-party frames that relied on the missing attribute will break; same-site app flows are unaffected."
 			});
 			// CORS default flip — wildcard "*" → deny-all (#2039). A bare
 			// `new wheels.middleware.Cors()` accepts no requests in 4.0.
@@ -4344,18 +4427,21 @@ component extends="modules.BaseModule" {
 				}
 
 				if (structKeyExists(check, "scanTargets") && isArray(check.scanTargets)) {
-					for (var target in check.scanTargets) {
-						var targetPath = variables.projectRoot & "/" & target.path;
+					// `scanTarget`, not `target` — the function-level `target`
+					// above holds the target VERSION string and a same-named
+					// loop var would shadow (then clobber) it.
+					for (var scanTarget in check.scanTargets) {
+						var targetPath = variables.projectRoot & "/" & scanTarget.path;
 						if (fileExists(targetPath)) {
 							arrayAppend(filesToScan, targetPath);
 						} else if (directoryExists(targetPath)) {
-							var recurse = structKeyExists(target, "recurse") ? target.recurse : true;
+							var recurse = structKeyExists(scanTarget, "recurse") ? scanTarget.recurse : true;
 							// Avoid Elvis `?:` on `check.extensions` — Adobe CF
 							// throws when the key is absent. The `wheels snippets`
 							// check has no top-level `extensions`, so this branch
 							// is reached on every Adobe CF run when a target is a
 							// directory without its own `extensions` key.
-							var exts = structKeyExists(target, "extensions") ? target.extensions
+							var exts = structKeyExists(scanTarget, "extensions") ? scanTarget.extensions
 								: (structKeyExists(check, "extensions") ? check.extensions : "");
 							for (var ext in listToArray(exts)) {
 								var dirFiles2 = directoryList(targetPath, recurse, "path", "*." & ext);
@@ -4418,44 +4504,79 @@ component extends="modules.BaseModule" {
 			}
 		}
 
-		// Output — three sections in priority order: Breaking → Recommended → All Clear
-		if (arrayLen(issues)) {
-			out("Breaking Changes (#arrayLen(issues)# found):", "yellow");
-			for (var issue in issues) {
-				out("  ! #issue.description#", "yellow");
-				for (var match in issue.matches) {
-					out("    #match#");
+		// The version-appropriate guide + the soft-landing adapter, surfaced
+		// whenever breaking findings are reported (and always in JSON output).
+		var guideUrl = "https://guides.wheels.dev/v4-0-0/upgrading/"
+			& (targetMajor >= 4 ? "3x-to-4x" : "2x-to-3x") & "/";
+
+		// JSON mode — one machine-readable document, no human report. The
+		// breaking-findings throw below still fires so pipelines can gate on
+		// the exit code without parsing stdout.
+		if (jsonMode) {
+			out(serializeJSON({
+				"currentVersion": currentVersion,
+				"targetVersion": target,
+				"success": arrayLen(issues) == 0,
+				"breaking": issues,
+				"advisories": advisories,
+				"passed": passed,
+				"guide": guideUrl
+			}));
+		} else {
+			// Output — three sections in priority order: Breaking → Recommended → All Clear
+			if (arrayLen(issues)) {
+				out("Breaking Changes (#arrayLen(issues)# found):", "yellow");
+				for (var issue in issues) {
+					out("  ! #issue.description#", "yellow");
+					for (var match in issue.matches) {
+						out("    #match#");
+					}
+					out("    -> #issue.fix#", "cyan");
+					out("");
+				}
+				out("Upgrade guide: #guideUrl#", "cyan");
+				if (targetMajor >= 4) {
+					out("Soft landing: wheels packages add wheels-legacy-adapter (shims renderPage()/renderPageToString() while you migrate)", "cyan");
 				}
-				out("    -> #issue.fix#", "cyan");
 				out("");
 			}
-		}
 
-		if (arrayLen(advisories)) {
-			out("Recommended Improvements (#arrayLen(advisories)# found):", "cyan");
-			for (var advisory in advisories) {
-				out("  ~ #advisory.description#", "cyan");
-				for (var match in advisory.matches) {
-					out("    #match#");
+			if (arrayLen(advisories)) {
+				out("Recommended Improvements (#arrayLen(advisories)# found):", "cyan");
+				for (var advisory in advisories) {
+					out("  ~ #advisory.description#", "cyan");
+					for (var match in advisory.matches) {
+						out("    #match#");
+					}
+					// Advisory fix lines are intentionally uncolored so the
+					// section header and description carry the cyan accent and
+					// opt-in items read lighter than breaking-change fixes
+					// (which use cyan on the fix line for stronger emphasis).
+					out("    -> #advisory.fix#");
+					out("");
 				}
-				// Advisory fix lines are intentionally uncolored so the
-				// section header and description carry the cyan accent and
-				// opt-in items read lighter than breaking-change fixes
-				// (which use cyan on the fix line for stronger emphasis).
-				out("    -> #advisory.fix#");
-				out("");
 			}
-		}
 
-		if (arrayLen(passed)) {
-			out("All Clear (#arrayLen(passed)# checks):", "green");
-			for (var p in passed) {
-				out("  + #p#", "green");
+			if (arrayLen(passed)) {
+				out("All Clear (#arrayLen(passed)# checks):", "green");
+				for (var p in passed) {
+					out("  + #p#", "green");
+				}
 			}
+
+			out("");
+			out("Upgrade with: brew upgrade wheels");
 		}
 
-		out("");
-		out("Upgrade with: brew upgrade wheels");
+		// Throw after the full report flushes — breaking findings exit
+		// non-zero (CI gate), advisories and all-clear exit 0. Mirrors
+		// validate()'s Wheels.ValidationFailed convention.
+		if (arrayLen(issues)) {
+			throw(
+				type = "Wheels.UpgradeCheckFailed",
+				message = "Upgrade check found #arrayLen(issues)# breaking change(s) — see the report above."
+			);
+		}
 
 		return "";
 	}