diff --git a/CHANGELOG.md b/CHANGELOG.md index db17cb4452..cd554d1a9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ All historical references to "CFWheels" in this changelog have been preserved fo # [Unreleased] +### Changed + +- CLI user-error paths now exit non-zero instead of silently returning success. Several commands printed a red error message and then `return ""`, which LuCLI maps to exit code 0 — so a typo'd subcommand or a failed migration looked like success to CI pipelines, deploy scripts, and pre-commit hooks. The following now throw a typed exception (`Wheels.InvalidArguments` for unknown input; the original error is re-thrown for runtime failures), which LuCLI maps to a non-zero exit while still printing the same friendly diagnostic first: `wheels generate `, `wheels create `, `wheels migrate `, `wheels db `, a failed `wheels migrate latest|up|down|info|doctor|rename-system-tables` (re-throws the underlying `MigrationError` instead of swallowing it), and `wheels routes` when the server returns an unparseable or unsuccessful response (`Wheels.RoutesFailed`). Help/no-args paths (`wheels generate`, `wheels db` with no subcommand) and non-error states (`wheels routes` with zero configured routes, `wheels db reset` without `--force`) are unchanged and still exit 0. Over MCP these surface as proper tool errors instead of empty results. **Scripts that previously relied on these error paths exiting 0 will now see a non-zero exit — that is the intended fix.** + ### Fixed - `wheels reload` and `wheels generate admin` now refuse to attach to a server that isn't bound to the current project, closing the same #2878 gap for two more server-dependent commands that #2879 fixed for the write-side migrators. Both reached `cli.lucli.Module::$requireRunningServer()` without the `requireProjectConfig` flag, so in a project with no `lucee.json` / `.env` port they still fell back to the hardcoded common-port probe (`[8080, 60000, 3000, 8500]`) and could silently attach to a sibling app: `reload` would reset the wrong app's state, and `generate admin` would introspect the wrong schema and scaffold its controller/views into the current project from a sibling's model — wrong-schema output written into the right project. Both now pass `requireProjectConfig = true`; with no project-bound port they throw `Wheels.ServerNotRunning` with a "set 'port' in lucee.json (or PORT in .env), then start with: wheels start" diagnostic instead of proceeding. `generate admin` is gated (rather than left on the read-side fallback alongside `info` / `routes`) precisely because it both reads a schema and writes files into cwd, so a wrong-server attach is a correctness bug, not just a wrong read. Covered by new server-free specs in `cli/lucli/tests/specs/services/ServerDetectionSpec.cfc` that drive `reload()` and `generateAdmin()` in a no-config project and assert the guard refuses to attach (#2878) diff --git a/cli/lucli/Module.cfc b/cli/lucli/Module.cfc index 92f1291fb5..3dcb8eb50f 100644 --- a/cli/lucli/Module.cfc +++ b/cli/lucli/Module.cfc @@ -434,7 +434,8 @@ component extends="modules.BaseModule" { default: out("Unknown generator type: #type#", "red"); out("Run 'wheels generate' for available types."); - return ""; + // throw maps to non-zero exit; return "" would silently succeed. + throw(type = "Wheels.InvalidArguments", message = "Unknown generator type: #type#"); } } @@ -458,14 +459,15 @@ component extends="modules.BaseModule" { return runMigration(action); } catch (MigrationError e) { out("Migration failed: #e.message#", "red"); - return ""; + // rethrow maps to non-zero exit; return "" would silently succeed. + rethrow; } case "doctor": try { return runMigration("doctor"); } catch (MigrationError e) { out("Doctor failed: #e.message#", "red"); - return ""; + rethrow; } case "forget": return runForgetOrPretend("forgetVersion", args); @@ -483,12 +485,12 @@ component extends="modules.BaseModule" { return runRenameSystemTables(dryRun); } catch (MigrationError e) { out("Rename failed: #e.message#", "red"); - return ""; + rethrow; } default: out("Unknown migration action: #action#", "red"); out("Usage: wheels migrate [latest|up|down|info|doctor|forget|pretend|rename-system-tables]"); - return ""; + throw(type = "Wheels.InvalidArguments", message = "Unknown migration action: #action#"); } } @@ -1026,7 +1028,7 @@ component extends="modules.BaseModule" { default: out("Unknown create type: #type#", "red"); out("Run 'wheels create' for available types."); - return ""; + throw(type = "Wheels.InvalidArguments", message = "Unknown create type: #type#"); } } @@ -1054,12 +1056,12 @@ component extends="modules.BaseModule" { } catch (any jsonErr) { out("Failed to parse routes response", "red"); verbose(httpResult); - return ""; + throw(type = "Wheels.RoutesFailed", message = "Failed to parse routes response"); } if (!structKeyExists(result, "success") || !result.success) { out("Failed to fetch routes: #result.message ?: 'unknown error'#", "red"); - return ""; + throw(type = "Wheels.RoutesFailed", message = "Failed to fetch routes: #result.message ?: 'unknown error'#"); } if (!structKeyExists(result, "routes") || !arrayLen(result.routes)) { @@ -1104,7 +1106,11 @@ component extends="modules.BaseModule" { out(""); out("#arrayLen(result.routes)# route(s)", "cyan"); } catch (any e) { - out("Failed to fetch routes: #e.message#", "red"); + // Inner Wheels.RoutesFailed paths already printed a diagnostic; only HTTP/unexpected errors need one here. + if (e.type != "Wheels.RoutesFailed") { + out("Failed to fetch routes: #e.message#", "red"); + } + rethrow; } return ""; } @@ -2663,7 +2669,7 @@ component extends="modules.BaseModule" { default: out("Unknown db command: #subcommand#", "red"); out("Valid commands: reset, status, version"); - return ""; + throw(type = "Wheels.InvalidArguments", message = "Unknown db command: #subcommand#"); } } diff --git a/cli/lucli/tests/specs/commands/CreateCommandSpec.cfc b/cli/lucli/tests/specs/commands/CreateCommandSpec.cfc new file mode 100644 index 0000000000..7585ae624d --- /dev/null +++ b/cli/lucli/tests/specs/commands/CreateCommandSpec.cfc @@ -0,0 +1,39 @@ +/** + * Tests the create command via Module.cfc. + * Verifies argument routing for the unknown-type and no-args paths + * (the only create() paths that don't require an app scaffold). + */ +component extends="wheels.wheelstest.system.BaseSpec" { + + function beforeAll() { + variables.testHelper = new cli.lucli.tests.TestHelper(); + variables.tempRoot = testHelper.scaffoldTempProject(expandPath("/")); + + directoryCreate(tempRoot & "/vendor/wheels", true, true); + + variables.mod = new cli.lucli.Module(cwd = variables.tempRoot); + } + + function afterAll() { + testHelper.cleanupTempProject(variables.tempRoot); + } + + function run() { + + describe("wheels create", () => { + + it("shows help when called with no arguments", () => { + mod.create(); + expect(true).toBeTrue(); + }); + + it("throws Wheels.InvalidArguments for an unknown create type", () => { + // arg1= exercises the callerArgs path; __arguments is only the internal-delegation fallback. + expect(() => mod.create(arg1 = "nonexistent")).toThrow(type = "Wheels.InvalidArguments"); + }); + + }); + + } + +} diff --git a/cli/lucli/tests/specs/commands/DbCommandSpec.cfc b/cli/lucli/tests/specs/commands/DbCommandSpec.cfc index d865916ce7..a09316980c 100644 --- a/cli/lucli/tests/specs/commands/DbCommandSpec.cfc +++ b/cli/lucli/tests/specs/commands/DbCommandSpec.cfc @@ -28,10 +28,9 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(true).toBeTrue(); }); - it("handles unknown subcommand without throwing", () => { - mod.__arguments = ["invalid"]; - mod.db(); - expect(true).toBeTrue(); + it("throws Wheels.InvalidArguments on an unknown subcommand", () => { + // arg1= exercises the callerArgs path; __arguments is only the internal-delegation fallback. + expect(() => mod.db(arg1 = "invalid")).toThrow(type = "Wheels.InvalidArguments"); }); it("accepts status subcommand", () => { diff --git a/cli/lucli/tests/specs/commands/GenerateCommandSpec.cfc b/cli/lucli/tests/specs/commands/GenerateCommandSpec.cfc index d45e8521cd..e97d7cf732 100644 --- a/cli/lucli/tests/specs/commands/GenerateCommandSpec.cfc +++ b/cli/lucli/tests/specs/commands/GenerateCommandSpec.cfc @@ -271,11 +271,8 @@ component extends="wheels.wheelstest.system.BaseSpec" { describe("unknown type handling", () => { - it("does not throw for unknown generator type", () => { - mod.__arguments = ["nonexistent"]; - // Should output error message but not throw - mod.generate(); - expect(true).toBeTrue(); + it("throws Wheels.InvalidArguments for an unknown generator type", () => { + expect(() => mod.generate(arg1 = "nonexistent")).toThrow(type = "Wheels.InvalidArguments"); }); }); diff --git a/cli/lucli/tests/specs/commands/MigrateCommandSpec.cfc b/cli/lucli/tests/specs/commands/MigrateCommandSpec.cfc index aaa4d6cd2f..ce86a58e40 100644 --- a/cli/lucli/tests/specs/commands/MigrateCommandSpec.cfc +++ b/cli/lucli/tests/specs/commands/MigrateCommandSpec.cfc @@ -59,10 +59,8 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(true).toBeTrue(); }); - it("rejects unknown action without throwing", () => { - mod.__arguments = ["invalid"]; - mod.migrate(); - expect(true).toBeTrue(); + it("throws Wheels.InvalidArguments on an unknown action", () => { + expect(() => mod.migrate(arg1 = "invalid")).toThrow(type = "Wheels.InvalidArguments"); }); });