feat(cli): upgrade check --strict + ArgSpec toInputSchema for MCP tooling honesty

CHANGELOG.md

@@ -20,8 +20,14 @@ All historical references to "CFWheels" in this changelog have been preserved fo
 
 ## [Unreleased]
 
+### Added
+
+- `wheels upgrade check --strict` escalates advisory findings (the "Recommended Improvements" section) to the same hard-fail path as breaking findings, throwing `Wheels.UpgradeCheckFailed` and exiting non-zero so CI pipelines can gate on opt-in convention changes. Without `--strict`, advisories continue to report-and-pass. Under `--format=json` the emitted document's `success` field is gated on both breaking findings and the strict-advisory case, and the `strict` flag is echoed back so `jq .success` and `$?` always agree. The flag is documented in `wheels upgrade` help output (#2963).
+- `services/ArgSpec.toInputSchema()` derives a JSON-Schema-compatible `{type:"object", properties, required, additionalProperties:false}` envelope from a command's declared positionals / flags / options. `positional()` / `flag()` / `option()` now accept an optional `description` argument that flows into each emitted property (FastMCP / Symfony JsonDescriptor pattern). Foundation for per-tool MCP input schemas; wiring into `tools/list` is a follow-up (#2963).
+
 ### Fixed
 
+- `mcpHiddenTools()` now structurally appends every `$`-prefixed PUBLIC function discovered via `getMetaData(this)` to the hidden list, in addition to the explicit literal entries. Defense-in-depth: a future `$publicHelper` added without a denylist update can no longer accidentally leak as a callable MCP tool. The literal `$normalizeTestFilter` / `$resolveAppTestDataSource` entries are retained for clarity and the case where LuCLI consults the list before metadata is fully populated; the structural pass de-duplicates and catches additions (#2963).
 - Dispatch now caches resolved route-scoped string middleware as application-scope singletons keyed by component path, so stateful middleware (e.g. an in-memory `RateLimiter` registered on a `.scope(path="/api", middleware=[...])`) accumulates state across requests instead of getting a fresh, empty instance per request. `$copyRouteForRequest` shallow-copies the route's `middleware` array instead of `Duplicate()`-ing it so Adobe CF (which clones CFCs inside arrays) doesn't silently reset the cached instances. The preflight-capability boolean is now computed once at `$init` and stored on the Dispatch instance, replacing the per-OPTIONS-request `IsInstanceOf` scan over the global pipeline. Documents the singleton lifecycle contract: middleware components must be safe to share across concurrent requests, which every built-in middleware already is (#2954)
 - `Job.processQueue()`'s private `$processJob` now guards the claim `UPDATE` with `AND status = 'pending'` and verifies the affected-row count via the `queryExecute` `result` option on the same statement, mirroring the matrix-proven `JobWorker.cfc::$claimJob` idiom (a separate verification `SELECT` breaks on BoxLang + PostgreSQL when the connection pool hands out a different connection that cannot see the uncommitted UPDATE). Pre-fix, two concurrent claimers — overlapping `processQueue()` callers, or `processQueue` racing the CLI worker — could both claim the same job and both run `perform()` (duplicate emails/charges, with `attempts` double-incremented). A lost claim now early-returns `{success = false, skipped = true}` before job instantiation, tenant-context setup, and `perform()`; `processQueue()` counts lost claims under a new additive `skipped` result key (#2899)
 - `onlyProvides()` per-action format restrictions now take effect. `$acceptableFormats()` was reading the top-level `variables.$class.formats` struct instead of the `.actions` sub-struct that `onlyProvides()` writes to, making every per-action restriction a silent no-op since introduction. **Behavior change by design** — apps that relied on the silent no-op will now see restrictions enforced (#2901)

cli/lucli/Module.cfc

@@ -163,9 +163,20 @@ component extends="modules.BaseModule" {
 	 * interactive (console), meta (mcp), alias (d), or don't translate to
 	 * single-call MCP semantics (browser). Read by LuCLI >= 0.3.4 per the
 	 * mcpHiddenTools() convention.
+	 *
+	 * Defense-in-depth (#2963 / wave-2 §5.2): every public function whose
+	 * name starts with `$` is appended structurally via `getMetaData(this)`.
+	 * These are the "public for specs" carve-out helpers documented in
+	 * cli/CLAUDE.md — kept public only so TestBox can reach them — and must
+	 * never appear in MCP `tools/list`. Without this structural sweep, a
+	 * future `$publicHelperFour` added without a denylist update would leak
+	 * as a callable tool. The literal `$normalizeTestFilter` /
+	 * `$resolveAppTestDataSource` entries below are retained for clarity
+	 * and the case where LuCLI consults the list before metadata is fully
+	 * populated; the structural pass de-duplicates and catches additions.
 	 */
 	public array function mcpHiddenTools() {
-		return [
+		var hidden = [
 			"main",     // bare `wheels` no-args dispatch target — not an MCP tool
 			"mcp",      // meta command — prints MCP setup instructions
 			"d",        // alias for destroy
@@ -183,6 +194,32 @@ component extends="modules.BaseModule" {
 			"$normalizeTestFilter",
 			"$resolveAppTestDataSource"
 		];
+
+		// Structural sweep — discover every $-prefixed PUBLIC function on
+		// this module via getMetaData(this).functions and add anything not
+		// already listed. Defense-in-depth so a future $-helper added without
+		// a denylist update can't accidentally leak as an MCP tool.
+		try {
+			var meta = getMetaData(this);
+			if (structKeyExists(meta, "functions") && isArray(meta.functions)) {
+				for (var fn in meta.functions) {
+					if (
+						structKeyExists(fn, "name")
+						&& structKeyExists(fn, "access")
+						&& fn.access == "public"
+						&& left(fn.name, 1) == "$"
+						&& !arrayContainsNoCase(hidden, fn.name)
+					) {
+						arrayAppend(hidden, fn.name);
+					}
+				}
+			}
+		} catch (any e) {
+			// Reflection failure: fall through to the literal denylist.
+			// The hard-coded $-entries above still cover the two known cases.
+		}
+
+		return hidden;
 	}
 
 	// ─────────────────────────────────────────────────
@@ -2733,11 +2770,17 @@ component extends="modules.BaseModule" {
 			.positional(name = "subcommand", default = "")
 			.option(name = "to", default = "")
 			.option(name = "format", default = "")
+			.flag(name = "strict", default = false)
 			.parse(arguments.coll);
 		return {
 			isCheck = lCase(parsed.subcommand) == "check",
 			targetVersion = parsed.to,
 			format = parsed.format,
+			// #2963: --strict escalates advisory findings to a hard failure
+			// (throws Wheels.UpgradeCheckFailed) so CI can gate on opt-in
+			// recommendations, not just breaking changes. Mirrors Django
+			// --fail-level WARNING / Mix --warnings-as-errors.
+			strict = parsed.strict,
 			sawTo = structKeyExists(arguments.coll, "to"),
 			sawDryRun = structKeyExists(arguments.coll, "dry-run")
 		};
@@ -2768,17 +2811,20 @@ component extends="modules.BaseModule" {
 
 		if (!opts.isCheck) {
 			var nl = chr(10);
-			var help = "Usage: wheels upgrade check [--to=<version>]" & nl
+			var help = "Usage: wheels upgrade check [--to=<version>] [--strict] [--format=json]" & nl
 				& nl
 				& "Scans your app for breaking changes between Wheels versions." & nl
 				& "This command is read-only — it does not modify vendor/wheels/." & nl
 				& nl
 				& "Options:" & nl
 				& "  --to=<version>    Target Wheels version (default: latest stable)" & nl
 				& "  --format=json     Emit a machine-readable JSON report" & nl
+				& "  --strict          Treat advisory findings (recommended improvements) as failures" & nl
+				& "                    Useful for CI — opt-in convention changes will gate the build." & nl
 				& nl
 				& "Exit status:" & nl
-				& "  Non-zero when breaking changes are found (advisories never fail the check)." & nl
+				& "  Non-zero when breaking changes are found. With --strict, advisory findings" & nl
+				& "  also fail the check; without --strict, advisories never affect the exit code." & nl
 				& nl
 				& "Unsupported flags:" & nl
 				& "  --dry-run is not supported — the command is already read-only," & nl
@@ -2800,7 +2846,7 @@ component extends="modules.BaseModule" {
 			return help;
 		}
 
-		return runUpgradeCheck(opts.targetVersion, opts.format);
+		return runUpgradeCheck(opts.targetVersion, opts.format, opts.strict);
 	}
 
 	// ─────────────────────────────────────────────────
@@ -4074,7 +4120,7 @@ component extends="modules.BaseModule" {
 	 * the exit code. `format="json"` replaces the human report with a single
 	 * JSON document for pipelines.
 	 */
-	private string function runUpgradeCheck(string targetVersion = "", string format = "") {
+	private string function runUpgradeCheck(string targetVersion = "", string format = "", boolean strict = false) {
 		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
@@ -4518,14 +4564,20 @@ component extends="modules.BaseModule" {
 		var guideUrl = "https://guides.wheels.dev/v4-0-0/upgrading/"
 			& (targetMajor >= 4 ? "3x-to-4x" : "2x-to-3x") & "/";
 
+		// `success` must reflect every condition that produces a non-zero
+		// exit, otherwise `jq .success` and `$?` disagree when --strict is
+		// active with advisory-only findings (#2963 review round 1).
+		var strictAdvisoryFail = arguments.strict && arrayLen(advisories) > 0;
+
 		// 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,
+				"success": arrayLen(issues) == 0 && !strictAdvisoryFail,
+				"strict": arguments.strict,
 				"breaking": issues,
 				"advisories": advisories,
 				"passed": passed,
@@ -4587,6 +4639,19 @@ component extends="modules.BaseModule" {
 			);
 		}
 
+		// #2963: --strict escalates advisory findings to the same hard-fail
+		// path. Reuses Wheels.UpgradeCheckFailed so CI pipelines that already
+		// filter on the breaking-case type pick the strict case up too. The
+		// breaking branch above already returned, so this fires only when
+		// strict mode is on AND at least one advisory matched but no
+		// breaking finding did.
+		if (arguments.strict && arrayLen(advisories)) {
+			throw(
+				type = "Wheels.UpgradeCheckFailed",
+				message = "Upgrade check found #arrayLen(advisories)# advisory finding(s) and --strict is set — see the report above."
+			);
+		}
+
 		return "";
 	}
 

cli/lucli/services/ArgSpec.cfc

@@ -42,36 +42,42 @@ component {
 		required string name,
 		boolean required = false,
 		any default = "",
-		string type = "string"
+		string type = "string",
+		string description = ""
 	) {
 		arrayAppend(variables.positionals, {
 			"name" = arguments.name,
 			"required" = arguments.required,
 			"default" = arguments.default,
-			"type" = arguments.type
+			"type" = arguments.type,
+			"description" = arguments.description
 		});
 		return this;
 	}
 
 	public any function flag(
 		required string name,
-		boolean default = false
+		boolean default = false,
+		string description = ""
 	) {
 		variables.named[arguments.name] = {
 			"default" = arguments.default,
-			"type" = "boolean"
+			"type" = "boolean",
+			"description" = arguments.description
 		};
 		return this;
 	}
 
 	public any function option(
 		required string name,
 		any default = "",
-		string type = "string"
+		string type = "string",
+		string description = ""
 	) {
 		variables.named[arguments.name] = {
 			"default" = arguments.default,
-			"type" = arguments.type
+			"type" = arguments.type,
+			"description" = arguments.description
 		};
 		return this;
 	}
@@ -169,6 +175,84 @@ component {
 		return result;
 	}
 
+	/**
+	 * Emit a JSON-Schema-compatible input schema describing this spec.
+	 *
+	 * The auto-discovered MCP tools in Module.cfc currently advertise empty
+	 * `properties` so clients can't discover parameters (#2963). Per the
+	 * cross-framework research (FastMCP, MCP TypeScript SDK, Symfony
+	 * JsonDescriptor): derive the schema from the same typed declaration
+	 * the command already uses. One source of truth, no hand-written drift.
+	 *
+	 * Result shape (matches MCP `tools/list[].inputSchema`):
+	 *
+	 *     {
+	 *       "type": "object",
+	 *       "properties": {
+	 *         "appName":    {"type": "string",  "description": "...", "default": ""},
+	 *         "sqlite":     {"type": "boolean", "description": "...", "default": true},
+	 *         "datasource": {"type": "string",  "description": "...", "default": ""}
+	 *       },
+	 *       "required": ["appName"],
+	 *       "additionalProperties": false
+	 *     }
+	 *
+	 * Type mapping follows CFML/ArgSpec coercion: positional/option strings
+	 * become JSON Schema "string"; numeric-typed options become "number";
+	 * flags become "boolean". `additionalProperties: false` matches the
+	 * mcpHiddenTools surface convention — unknown keys are rejected at the
+	 * MCP client.
+	 */
+	public struct function toInputSchema() {
+		var properties = {};
+		var required = [];
+
+		for (var p in variables.positionals) {
+			properties[p.name] = $toSchemaProperty(p.type, p["default"], p.description);
+			if (p.required) {
+				arrayAppend(required, p.name);
+			}
+		}
+
+		for (var optName in variables.named) {
+			var spec = variables.named[optName];
+			properties[optName] = $toSchemaProperty(spec.type, spec["default"], spec.description);
+		}
+
+		return {
+			"type" = "object",
+			"properties" = properties,
+			"required" = required,
+			"additionalProperties" = false
+		};
+	}
+
+	private struct function $toSchemaProperty(
+		required string type,
+		required any default,
+		string description = ""
+	) {
+		var prop = {
+			"type" = $toJsonSchemaType(arguments.type),
+			"default" = arguments.default
+		};
+		if (len(arguments.description)) {
+			prop["description"] = arguments.description;
+		}
+		return prop;
+	}
+
+	private string function $toJsonSchemaType(required string cfmlType) {
+		switch (arguments.cfmlType) {
+			case "boolean":
+				return "boolean";
+			case "numeric":
+				return "number";
+			default:
+				return "string";
+		}
+	}
+
 	/**
 	 * Collect the numeric index of every positional (arg<N>) key in the
 	 * collection, sorted ascending. LuCLI numbers positionals by global token

cli/lucli/tests/specs/commands/McpHiddenToolsSpec.cfc

@@ -0,0 +1,86 @@
+/**
+ * Defense-in-depth coverage for `mcpHiddenTools()` — the function LuCLI reads
+ * to filter the MCP `tools/list` surface. Auto-discovered MCP tools include
+ * every public function in Module.cfc; without an exclusion path, a
+ * `$`-prefixed public helper (kept public only so unit tests can reach it —
+ * the cli/CLAUDE.md "public for specs" carve-out) leaks into `tools/list`
+ * verbatim, surfacing as a callable MCP tool.
+ *
+ * Issue #2963 / wave-2 §5.2 (Part 2) flagged this: the static denylist works
+ * for the two known cases (`$normalizeTestFilter`, `$resolveAppTestDataSource`),
+ * but a future `$publicHelperFour` added without a denylist update would leak.
+ * The fix structurally discovers every `$`-prefixed public function via
+ * `getMetaData(this)` so the exclusion is self-maintaining.
+ *
+ * Like every other Module.cfc spec, source-level inspection — Module extends
+ * `modules.BaseModule`, which is only resolvable at LuCLI runtime, not in
+ * TestBox (see UpgradeCommandSpec / MainCommandSpec).
+ */
+component extends="wheels.wheelstest.system.BaseSpec" {
+
+	function beforeAll() {
+		variables.moduleSource = fileRead(expandPath("/cli/lucli/Module.cfc"));
+	}
+
+	function run() {
+
+		describe("mcpHiddenTools() — structural $-prefix exclusion (##2963)", () => {
+
+			it("walks the module's own public functions via getMetaData(this)", () => {
+				// The fix replaces the hard-coded list with reflection over
+				// the module's own metadata, so adding a new $-prefixed public
+				// helper can't accidentally leak as an MCP tool. Source-level:
+				// the function body must call getMetaData(this) (or this.$ ...
+				// → metadata-driven discovery), not just return a static array.
+				var startIdx = reFindNoCase("(?m)^[ \t]*public\s+array\s+function\s+mcpHiddenTools\s*\(", variables.moduleSource);
+				expect(startIdx).toBeGT(0);
+				var body = mid(variables.moduleSource, startIdx, 2500);
+				expect(body).toInclude("getMetaData(this)");
+			});
+
+			it("excludes every $-prefixed public function discovered in the module", () => {
+				// The discovery loop must filter on the leading `$` so future
+				// $publicHelper functions are auto-excluded. Match any of the
+				// idiomatic prefix tests (left, mid, find/findNoCase at position 1).
+				var startIdx = reFindNoCase("(?m)^[ \t]*public\s+array\s+function\s+mcpHiddenTools\s*\(", variables.moduleSource);
+				expect(startIdx).toBeGT(0);
+				var body = mid(variables.moduleSource, startIdx, 2500);
+				// One of these prefix tests must appear inside the function
+				// body. Loose match so an equivalent rewrite still passes.
+				var hasLeftPrefix = reFindNoCase("left\s*\(\s*[a-zA-Z_]+\.name\s*,\s*1\s*\)\s*==\s*""\$""", body) > 0;
+				var hasFindPrefix = reFindNoCase("find\s*\(\s*""\$""\s*,\s*[a-zA-Z_]+\.name\s*\)\s*==\s*1", body) > 0;
+				var hasReFindPrefix = reFindNoCase("reFind(NoCase)?\s*\(\s*""\^\\\$""\s*,\s*[a-zA-Z_]+\.name\s*\)", body) > 0;
+				expect(hasLeftPrefix || hasFindPrefix || hasReFindPrefix).toBeTrue();
+			});
+
+			it("keeps non-$-prefixed CLI-only commands in the hidden list", () => {
+				// The structural prefix filter doesn't cover commands like
+				// `start`, `stop`, `new`, `console`, `browser` — these are
+				// public, non-$-prefixed, and CLI-only. They must remain in
+				// the explicit denylist that the function returns. Source:
+				// the literal strings still appear in the function body.
+				var startIdx = reFindNoCase("(?m)^[ \t]*public\s+array\s+function\s+mcpHiddenTools\s*\(", variables.moduleSource);
+				expect(startIdx).toBeGT(0);
+				var body = mid(variables.moduleSource, startIdx, 2500);
+				expect(body).toInclude("""start""");
+				expect(body).toInclude("""stop""");
+				expect(body).toInclude("""new""");
+				expect(body).toInclude("""console""");
+				expect(body).toInclude("""browser""");
+				expect(body).toInclude("""mcp""");
+			});
+
+			it("returns an array (the LuCLI mcpHiddenTools() contract)", () => {
+				// LuCLI calls mcpHiddenTools() and expects an array of
+				// string names. Source-level: the return type is `array`
+				// and the body returns one. Smoke test against the
+				// declaration only — full integration is covered by LuCLI's
+				// own tools/list invocations end-to-end.
+				expect(reFindNoCase("(?m)^[ \t]*public\s+array\s+function\s+mcpHiddenTools\s*\(", variables.moduleSource)).toBeGT(0);
+			});
+
+		});
+
+	}
+
+}