diff --git a/CHANGELOG.md b/CHANGELOG.md index 122f4af0f4..f96daf2443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,12 +20,18 @@ 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). + ### Performance - `URLFor()` controller/action route lookup is now memoized in application scope with negative caching, instead of a per-request memo that only cached matches. The previous memo was rebuilt on every request and was never written on a miss, so wildcard-`[controller]` apps — where `$addRoute` strips the `controller` key, guaranteeing no match — re-scanned the entire route table for every `linkTo` / `urlFor` / `redirectTo` call, on every request. The new `application.wheels.urlForCache` survives across requests and caches both hits and misses (empty-string sentinel) for O(1) lookup. Invalidation is plumbed through both `$lockedLoadRoutes` (route reload) and `$addRoute` (any mutation, including test-suite manipulation), so a previously negative-cached `(controller, action)` pair that a newly-added route now matches can never serve a stale miss (#2955) ### 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) diff --git a/cli/lucli/Module.cfc b/cli/lucli/Module.cfc index a63a9fc4bc..ec836caf7e 100644 --- a/cli/lucli/Module.cfc +++ b/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,7 +2811,7 @@ component extends="modules.BaseModule" { if (!opts.isCheck) { var nl = chr(10); - var help = "Usage: wheels upgrade check [--to=]" & nl + var help = "Usage: wheels upgrade check [--to=] [--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 @@ -2776,9 +2819,12 @@ component extends="modules.BaseModule" { & "Options:" & nl & " --to= 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,6 +4564,11 @@ 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. @@ -4525,7 +4576,8 @@ component extends="modules.BaseModule" { 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 ""; } diff --git a/cli/lucli/services/ArgSpec.cfc b/cli/lucli/services/ArgSpec.cfc index 34c4383450..abfefa6af8 100644 --- a/cli/lucli/services/ArgSpec.cfc +++ b/cli/lucli/services/ArgSpec.cfc @@ -42,24 +42,28 @@ 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; } @@ -67,11 +71,13 @@ component { 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) key in the * collection, sorted ascending. LuCLI numbers positionals by global token diff --git a/cli/lucli/tests/specs/commands/McpHiddenToolsSpec.cfc b/cli/lucli/tests/specs/commands/McpHiddenToolsSpec.cfc new file mode 100644 index 0000000000..2fb6d3755c --- /dev/null +++ b/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); + }); + + }); + + } + +} diff --git a/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc b/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc index 790bb527d6..87d6ce5db8 100644 --- a/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc +++ b/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc @@ -59,6 +59,95 @@ component extends="wheels.wheelstest.system.BaseSpec" { }); + describe("wheels upgrade — --strict CI gate (##2963)", () => { + + // #2963 / wave-2 §5.2 (Part 1): `wheels upgrade check` already + // throws Wheels.UpgradeCheckFailed when breaking findings exist, + // but advisories never gate CI. `--strict` escalates advisory + // findings to a hard failure so projects can opt into "treat + // recommended improvements as breaking" for CI runs. Mirrors + // Django's `--fail-level WARNING` / Mix's --warnings-as-errors. + + it("declares the --strict flag in parseUpgradeArgs", () => { + // Source-level: parseUpgradeArgs must declare a `strict` flag + // alongside `to` and `format` so LuCLI surfaces it. + var startIdx = reFindNoCase("(?m)^[ \t]*private\s+struct\s+function\s+parseUpgradeArgs\s*\(", variables.moduleSource); + expect(startIdx).toBeGT(0); + var body = mid(variables.moduleSource, startIdx, 800); + expect(body).toInclude("strict"); + expect(body).toInclude(".flag"); + }); + + it("threads strict mode through to runUpgradeCheck", () => { + // upgrade() must forward the parsed strict flag into the runner. + // Window the dispatch line so we don't false-match an unrelated + // strict reference elsewhere in the module. + var dispatchIdx = reFindNoCase("runUpgradeCheck\s*\(", variables.moduleSource); + expect(dispatchIdx).toBeGT(0); + var callsite = mid(variables.moduleSource, dispatchIdx, 200); + expect(callsite).toInclude("opts.strict"); + }); + + it("runUpgradeCheck accepts a strict argument", () => { + var startIdx = reFindNoCase("(?m)^[ \t]*private\s+string\s+function\s+runUpgradeCheck\s*\(", variables.moduleSource); + expect(startIdx).toBeGT(0); + var sigEnd = find(")", variables.moduleSource, startIdx); + expect(sigEnd).toBeGT(startIdx); + var signature = mid(variables.moduleSource, startIdx, sigEnd - startIdx + 1); + expect(signature).toInclude("strict"); + }); + + it("throws Wheels.UpgradeCheckFailed when strict mode finds advisories (no breaking)", () => { + // The strict gate must throw with a distinct, parseable error + // type so pipelines can distinguish "breaking" from + // "strict-mode advisory" exits. Reuse the existing + // UpgradeCheckFailed type so CI scripts that already filter on + // it pick the strict case up automatically. + expect(variables.moduleSource).toInclude("Wheels.UpgradeCheckFailed"); + // The strict gate fires when (a) strict mode is on AND (b) at + // least one advisory was matched. Match the conjunction loosely + // so an equivalent rewrite (e.g. `strict && arrayLen(advisories)`) + // still satisfies the spec. + expect(reFindNoCase("strict\s*(&&|and)\s*arrayLen\s*\(\s*advisories", variables.moduleSource)).toBeGT(0); + }); + + it("documents --strict in the upgrade() help banner", () => { + // The help text users read when running bare `wheels upgrade` + // must surface the new flag — otherwise it's discoverable only + // by reading the source. + expect(variables.moduleSource).toInclude("--strict"); + }); + + it("gates the JSON `success` field on strict + advisories, not just breaking issues", () => { + // Round-1 review finding (#2963): with `--strict --format=json` + // on an app with advisory-only findings, JSON stdout reported + // `success: true` while the process exited non-zero — `jq .success` + // and `$?` disagreed. The fix routes `success` through a + // `strictAdvisoryFail` precomputation. Pin both the precomp and + // its consumption inside the `serializeJSON({` block so a future + // rewrite that forgets the gate fails the spec. + expect(variables.moduleSource).toInclude("strictAdvisoryFail"); + + var serializeIdx = reFindNoCase("out\s*\(\s*serializeJSON\s*\(\s*\{", variables.moduleSource); + expect(serializeIdx).toBeGT(0); + // Window only the JSON literal — the `}));` that closes the + // serializeJSON call sits within ~400 chars of its opening. + var jsonBlock = mid(variables.moduleSource, serializeIdx, 600); + expect(reFindNoCase("success.{0,80}strictAdvisoryFail", jsonBlock)).toBeGT(0); + }); + + it("includes the `strict` flag in the JSON document so consumers can explain a non-zero exit", () => { + // Without surfacing `strict` in the JSON body, a `success: false` + // document with empty `breaking[]` looks like a data inconsistency + // to anyone parsing stdout instead of reading the error message. + var serializeIdx = reFindNoCase("out\s*\(\s*serializeJSON\s*\(\s*\{", variables.moduleSource); + expect(serializeIdx).toBeGT(0); + var jsonBlock = mid(variables.moduleSource, serializeIdx, 600); + expect(reFindNoCase("""strict""\s*:\s*arguments\.strict", jsonBlock)).toBeGT(0); + }); + + }); + } } diff --git a/cli/lucli/tests/specs/services/ArgSpecSpec.cfc b/cli/lucli/tests/specs/services/ArgSpecSpec.cfc index 11b4512c4b..e776869b23 100644 --- a/cli/lucli/tests/specs/services/ArgSpecSpec.cfc +++ b/cli/lucli/tests/specs/services/ArgSpecSpec.cfc @@ -229,6 +229,95 @@ component extends="wheels.wheelstest.system.BaseSpec" { }); + describe("toInputSchema() — typed MCP tool input schema", () => { + + // #2963 / wave-2 §5.2: MCP tool input schemas. Auto-discovered + // tools in Module.cfc advertise empty `properties` so MCP + // clients can't discover parameters. The fix derives the + // per-tool schema from the same ArgSpec the command already + // declares (FastMCP / Symfony JsonDescriptor pattern) — one + // source of truth, no hand-written drift. + + it("returns a JSON-Schema-compatible object envelope", () => { + var schema = new cli.lucli.services.ArgSpec().toInputSchema(); + expect(schema.type).toBe("object"); + expect(structKeyExists(schema, "properties")).toBeTrue(); + expect(structKeyExists(schema, "required")).toBeTrue(); + // Hostile clients sending an unknown key shouldn't be + // silently tolerated — match the existing hidden-tool + // pattern (additionalProperties:false). + expect(schema.additionalProperties).toBeFalse(); + }); + + it("emits one property per declared positional, flag, and option", () => { + var schema = new cli.lucli.services.ArgSpec() + .positional(name = "appName", required = true, description = "App folder name") + .flag(name = "sqlite", default = true, description = "Use SQLite datasource") + .option(name = "datasource", default = "", description = "Datasource name") + .toInputSchema(); + expect(structKeyExists(schema.properties, "appName")).toBeTrue(); + expect(structKeyExists(schema.properties, "sqlite")).toBeTrue(); + expect(structKeyExists(schema.properties, "datasource")).toBeTrue(); + }); + + it("lists required positionals in the required array", () => { + var schema = new cli.lucli.services.ArgSpec() + .positional(name = "appName", required = true) + .positional(name = "templateName", required = false, default = "default") + .toInputSchema(); + expect(schema.required).toInclude("appName"); + expect(schema.required).notToInclude("templateName"); + }); + + it("maps positional type=string to JSON Schema type 'string'", () => { + var schema = new cli.lucli.services.ArgSpec() + .positional(name = "appName", required = true) + .toInputSchema(); + expect(schema.properties.appName.type).toBe("string"); + }); + + it("maps option type=numeric to JSON Schema type 'number'", () => { + var schema = new cli.lucli.services.ArgSpec() + .option(name = "port", default = 3000, type = "numeric") + .toInputSchema(); + expect(schema.properties.port.type).toBe("number"); + }); + + it("maps flag to JSON Schema type 'boolean'", () => { + var schema = new cli.lucli.services.ArgSpec() + .flag(name = "sqlite", default = true) + .toInputSchema(); + expect(schema.properties.sqlite.type).toBe("boolean"); + }); + + it("includes the description on each property when supplied", () => { + var schema = new cli.lucli.services.ArgSpec() + .positional(name = "appName", required = true, description = "App folder name") + .flag(name = "sqlite", default = true, description = "Use SQLite datasource") + .option(name = "datasource", default = "", description = "Datasource name") + .toInputSchema(); + expect(schema.properties.appName.description).toBe("App folder name"); + expect(schema.properties.sqlite.description).toBe("Use SQLite datasource"); + expect(schema.properties.datasource.description).toBe("Datasource name"); + }); + + it("includes the declared default in each property", () => { + var schema = new cli.lucli.services.ArgSpec() + .flag(name = "sqlite", default = true) + .option(name = "datasource", default = "wheelsapp") + .toInputSchema(); + expect(schema.properties.sqlite.default).toBeTrue(); + expect(schema.properties.datasource.default).toBe("wheelsapp"); + }); + + it("returns an empty schema (no properties, no required) when nothing is declared", () => { + var schema = new cli.lucli.services.ArgSpec().toInputSchema(); + expect(structIsEmpty(schema.properties)).toBeTrue(); + expect(arrayLen(schema.required)).toBe(0); + }); + + }); + }); } diff --git a/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/mcp-integration.mdx b/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/mcp-integration.mdx index 2108622f62..15a8e83d3e 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/mcp-integration.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/mcp-integration.mdx @@ -86,6 +86,7 @@ A handful of public CLI commands are excluded from MCP discovery via `mcpHiddenT - `browser` — multi-step browser testing flow with its own subcommand tree. - `mcp` — the meta command itself; exposing it would let an agent recursively launch MCP servers. - `d` — short alias for `destroy`, excluded to avoid a duplicate tool entry (the canonical `wheels_destroy` is exposed). +- Any function whose name begins with `$` — internal helpers kept `public` only so unit tests can reach them (a CFML testing carve-out). `mcpHiddenTools()` discovers these structurally via `getMetaData(this)`, so a future `$helper` added to the module cannot accidentally surface as a callable MCP tool without a manual denylist update. If you need one of these behaviours from an AI IDE, invoke it via a shell tool — don't try to expose it as an MCP tool. diff --git a/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/upgrade.mdx b/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/upgrade.mdx index 306aa230c6..5e7b1f1581 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/upgrade.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/upgrade.mdx @@ -19,7 +19,7 @@ import { Aside, CardGrid, LinkCard } from '@astrojs/starlight/components'; ### Synopsis ``` title="Synopsis" -wheels upgrade check [--to=] [--format=json] +wheels upgrade check [--to=] [--format=json] [--strict] ``` Calling `wheels upgrade` with no subcommand (or any subcommand other than `check`) prints usage and exits. The only verb the command currently understands is `check`. @@ -33,7 +33,7 @@ Calling `wheels upgrade` with no subcommand (or any subcommand other than `check 3. **Compares major versions.** If current and target share the same major, the command notes "Same major version — no known breaking changes" for the major-transition checks, then still runs its advisory scan for the same-major patterns below. 4. **Runs breaking-change checks.** Each check is either a directory existence test or a regex grep across a sub-tree of the project. Major-version transitions add their own checks on top of the advisory scan. Hits are printed as issues (yellow); absences are printed as passed checks (green). -**Exit status:** when breaking findings exist the command throws `Wheels.UpgradeCheckFailed` after the report flushes, exiting non-zero so it can gate CI. Advisory findings never affect the exit code. With `--format=json` the human report is replaced by a single JSON document (`currentVersion`, `targetVersion`, `success`, `breaking`, `advisories`, `passed`, `guide`) — the non-zero exit on breaking findings still applies. +**Exit status:** when breaking findings exist the command throws `Wheels.UpgradeCheckFailed` after the report flushes, exiting non-zero so it can gate CI. Without `--strict`, advisory findings never affect the exit code. With `--strict`, advisory findings escalate to the same hard-fail path — useful for CI pipelines that want to gate on opt-in convention changes, not just breaking ones. With `--format=json` the human report is replaced by a single JSON document (`currentVersion`, `targetVersion`, `success`, `strict`, `breaking`, `advisories`, `passed`, `guide`) — the non-zero exit on breaking or strict-mode advisory findings still applies. The document's `success` field tracks the exit code precisely: it is `false` whenever the process exits non-zero, including the `--strict` + advisory-only case, and the `strict` field is echoed back so consumers can tell `success: false` with empty `breaking[]` apart from a data inconsistency. Nothing in the project is modified. No files are written. The command does not stage, commit, or touch git state. @@ -50,7 +50,8 @@ None that the command enforces — but in practice: | Flag | Description | |---|---| | `--to=` | Target version to scan against (e.g. `--to=4.0.0`). When omitted, the command queries GitHub for the latest release tag. If the GitHub call fails and no `--to=` is given, the command aborts. | -| `--format=json` | Emit a single machine-readable JSON report instead of the human output — for CI pipelines. Breaking findings still exit non-zero. | +| `--format=json` | Emit a single machine-readable JSON report instead of the human output — for CI pipelines. Breaking findings (and advisory findings when `--strict` is set) still exit non-zero. | +| `--strict` | Escalate advisory findings (the "Recommended Improvements" section) to the same hard-fail path as breaking findings. The command throws `Wheels.UpgradeCheckFailed` and exits non-zero so CI can gate on opt-in convention changes. Without this flag, advisories are reported but never fail the check. Mirrors Django's `--fail-level WARNING` / Mix's `--warnings-as-errors`. | That is the complete flag surface. The command does not accept `--force`, `--dry-run`, `--check`, `--backup`, or any apply-style switch — there is nothing to apply.