Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
- `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)
- Oracle and Microsoft SQL Server adapters now resolve the new primary key after INSERT from the JDBC driver-supplied generated key instead of session-wide / location-based heuristics, closing a silent data-integrity race. `wheels.databaseAdapters.Oracle.OracleModel::$identitySelect` previously fell back to `WHERE ROWID = (SELECT MAX(ROWID) FROM <tbl>)`, but ROWID is the row's *physical location*, not insertion order — `MAX(ROWID)` can point at another concurrent session's row and the caller reads back the wrong PK under load. `MicrosoftSQLServerModel::$identitySelect` used `SELECT @@IDENTITY`, which returns the last identity generated in *any* scope on the connection — including AFTER INSERT triggers that insert into their own identity-keyed log/audit tables, in which case the trigger's identity is returned instead of the row's. Both adapters now read the driver-supplied generated key first — `result.generatedKey` on Lucee, `result.rowid` on Adobe CF (the surfaces CFML engines expose when `Statement.RETURN_GENERATED_KEYS` is set, which Wheels does on every INSERT via `$bulkInsertSQL`) — and either return it directly when it's numeric or use it as an exact-row lookup. The Oracle path treats the 18-character base-64 extended ROWID as a row pointer (`WHERE ROWID = CHARTOROWID('<rowid>')`), targeting the just-inserted row race-free; the regex `^[A-Za-z0-9/+]{18}$` strictly gates before interpolating, and UROWIDs / anything unexpected fall through to the legacy `MAX(ROWID)` query, preserved as a last resort for engines that surface no generated key (current BoxLang). The SQL Server path uses `SCOPE_IDENTITY()` only as a fallback after the driver-supplied key is exhausted, sidestepping `@@IDENTITY`'s trigger-unsafety. The `endPar > 0` guard added around the column-list extraction also fixes a latent Adobe CF crash where `Mid(sql, N, -N)` ran when the closing parenthesis was missing. Mirrors the pattern already in `CockroachDBModel.cfc`. Covered by new server-free specs in `OracleUnitSpec.cfc` and `MicrosoftSQLServerUnitSpec.cfc` (#2908)
Expand Down
77 changes: 71 additions & 6 deletions cli/lucli/Module.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

// ─────────────────────────────────────────────────
Expand Down Expand Up @@ -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")
};
Expand Down Expand Up @@ -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
Expand All @@ -2800,7 +2846,7 @@ component extends="modules.BaseModule" {
return help;
}

return runUpgradeCheck(opts.targetVersion, opts.format);
return runUpgradeCheck(opts.targetVersion, opts.format, opts.strict);
}

// ─────────────────────────────────────────────────
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 "";
}

Expand Down
96 changes: 90 additions & 6 deletions cli/lucli/services/ArgSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading