diff --git a/changelog.d/deploy-numeric-release-dry-run-hang.fixed.md b/changelog.d/deploy-numeric-release-dry-run-hang.fixed.md new file mode 100644 index 0000000000..764c8ccbdf --- /dev/null +++ b/changelog.d/deploy-numeric-release-dry-run-hang.fixed.md @@ -0,0 +1 @@ +- **CLI**: `wheels deploy ... --release=1 --dry-run` no longer hangs ~76 seconds with `Operation timed out`. CFML's coercing `==` treated the value `1` as the boolean string `true` during the argv round-trip, dropping the release value and letting the deploy flag parser swallow `--dry-run` as the version — so a documented dry run opened real SSH connections. Values are now compared exactly (`1`, `0`, `yes`, `no` stay values), and a value-taking deploy flag never consumes a following `--` flag as its value ([#3111](https://github.com/wheels-dev/wheels/issues/3111)) diff --git a/cli/lucli/services/ArgSpec.cfc b/cli/lucli/services/ArgSpec.cfc index abfefa6af8..f918dfeada 100644 --- a/cli/lucli/services/ArgSpec.cfc +++ b/cli/lucli/services/ArgSpec.cfc @@ -158,17 +158,30 @@ component { } // Named keys, re-prefixed. --no-X for false preserves the negation. + // Flag detection MUST use an exact string compare: CFML `==` coerces + // both operands, so "1" == "true" and "0" == "false" evaluate TRUE. + // That coercion turned `--release=1` into a bare --release flag (value + // dropped) and the downstream deploy parser then swallowed --dry-run + // as the version — a documented dry run dispatched live SSH and hung + // ~76s against the config stub's placeholder host (issue #3111). + // LuCLI normalizes flags to the literal strings "true"/"false"; MCP + // argCollections may carry native booleans, which toString() renders + // as "true"/"false" — both shapes match the exact compare. for (var key in arguments.coll) { if (reFindNoCase("^arg\d+$", key)) { continue; } var value = arguments.coll[key]; - if (isSimpleValue(value) && value == "true") { + if (!isSimpleValue(value)) { + continue; + } + var stringValue = toString(value); + if (compareNoCase(stringValue, "true") == 0) { arrayAppend(result, "--" & key); - } else if (isSimpleValue(value) && value == "false") { + } else if (compareNoCase(stringValue, "false") == 0) { arrayAppend(result, "--no-" & key); - } else if (isSimpleValue(value)) { - arrayAppend(result, "--" & key & "=" & value); + } else { + arrayAppend(result, "--" & key & "=" & stringValue); } } diff --git a/cli/lucli/services/deploy/cli/DeployArgsParser.cfc b/cli/lucli/services/deploy/cli/DeployArgsParser.cfc index b31e1ecea3..353203b1ce 100644 --- a/cli/lucli/services/deploy/cli/DeployArgsParser.cfc +++ b/cli/lucli/services/deploy/cli/DeployArgsParser.cfc @@ -24,7 +24,7 @@ component { opts.dryRun = true; } else if (left(a, 14) == "--destination=") { opts.destination = mid(a, 15, 99999); - } else if (a == "--destination" && i < n) { + } else if (a == "--destination" && $nextIsValue(arguments.args, i, n)) { opts.destination = arguments.args[i+1]; i++; } else if (left(a, 10) == "--version=") { @@ -33,89 +33,89 @@ component { // has rewritten --version -> --release first, or when arrays are // constructed programmatically (tests). opts.version = mid(a, 11, 99999); - } else if (a == "--version" && i < n) { + } else if (a == "--version" && $nextIsValue(arguments.args, i, n)) { opts.version = arguments.args[i+1]; i++; } else if (left(a, 10) == "--release=") { // picocli-safe alias for --version (issue #2674). opts.version = mid(a, 11, 99999); - } else if (a == "--release" && i < n) { + } else if (a == "--release" && $nextIsValue(arguments.args, i, n)) { opts.version = arguments.args[i+1]; i++; } else if (left(a, 13) == "--configPath=") { opts.configPath = mid(a, 14, 99999); - } else if (a == "--configPath" && i < n) { + } else if (a == "--configPath" && $nextIsValue(arguments.args, i, n)) { opts.configPath = arguments.args[i+1]; i++; } else if (left(a, 9) == "--config=") { // Alias for --configPath — the deploy guides document --config. CLI audit H9. opts.configPath = mid(a, 10, 99999); - } else if (a == "--config" && i < n) { + } else if (a == "--config" && $nextIsValue(arguments.args, i, n)) { opts.configPath = arguments.args[i+1]; i++; } else if (a == "--force") { opts.force = true; } else if (left(a, 10) == "--service=") { opts.service = mid(a, 11, 99999); - } else if (a == "--service" && i < n) { + } else if (a == "--service" && $nextIsValue(arguments.args, i, n)) { opts.service = arguments.args[i+1]; i++; } else if (left(a, 8) == "--image=") { opts.image = mid(a, 9, 99999); - } else if (a == "--image" && i < n) { + } else if (a == "--image" && $nextIsValue(arguments.args, i, n)) { opts.image = arguments.args[i+1]; i++; } else if (left(a, 20) == "--registry-username=") { opts.registryUsername = mid(a, 21, 99999); - } else if (a == "--registry-username" && i < n) { + } else if (a == "--registry-username" && $nextIsValue(arguments.args, i, n)) { opts.registryUsername = arguments.args[i+1]; i++; } else if (left(a, 7) == "--host=") { opts.host = mid(a, 8, 99999); - } else if (a == "--host" && i < n) { + } else if (a == "--host" && $nextIsValue(arguments.args, i, n)) { opts.host = arguments.args[i+1]; i++; } else if (left(a, 7) == "--keep=") { opts.keep = mid(a, 8, 99999); - } else if (a == "--keep" && i < n) { + } else if (a == "--keep" && $nextIsValue(arguments.args, i, n)) { opts.keep = arguments.args[i+1]; i++; } else if (left(a, 10) == "--message=") { opts.message = mid(a, 11, 99999); - } else if (a == "--message" && i < n) { + } else if (a == "--message" && $nextIsValue(arguments.args, i, n)) { opts.message = arguments.args[i+1]; i++; } else if (left(a, 10) == "--adapter=") { opts.adapter = mid(a, 11, 99999); - } else if (a == "--adapter" && i < n) { + } else if (a == "--adapter" && $nextIsValue(arguments.args, i, n)) { opts.adapter = arguments.args[i+1]; i++; } else if (left(a, 10) == "--account=") { opts.account = mid(a, 11, 99999); - } else if (a == "--account" && i < n) { + } else if (a == "--account" && $nextIsValue(arguments.args, i, n)) { opts.account = arguments.args[i+1]; i++; } else if (left(a, 7) == "--from=") { opts.from = mid(a, 8, 99999); - } else if (a == "--from" && i < n) { + } else if (a == "--from" && $nextIsValue(arguments.args, i, n)) { opts.from = arguments.args[i+1]; i++; } else if (a == "--confirm") { opts.confirm = true; } else if (left(a, 7) == "--tail=") { opts.tail = mid(a, 8, 99999); - } else if (a == "--tail" && i < n) { + } else if (a == "--tail" && $nextIsValue(arguments.args, i, n)) { opts.tail = arguments.args[i+1]; i++; } else if (left(a, 7) == "--role=") { // `deploy app ` role filter; DeployAppCli reads opts.role. CLI audit H9. opts.role = mid(a, 8, 99999); - } else if (a == "--role" && i < n) { + } else if (a == "--role" && $nextIsValue(arguments.args, i, n)) { opts.role = arguments.args[i+1]; i++; } else if (left(a, 12) == "--container=") { opts.container = mid(a, 13, 99999); - } else if (a == "--container" && i < n) { + } else if (a == "--container" && $nextIsValue(arguments.args, i, n)) { opts.container = arguments.args[i+1]; i++; } else if (a == "--follow") { @@ -125,4 +125,16 @@ component { } return opts; } + + /** + * True when the token after position i exists and is a plain value, not + * another `--` flag. Guards every space-separated `--flag value` arm so a + * value-taking flag with a missing value can never swallow the flag that + * follows it — before this, `--release --dry-run` consumed --dry-run as + * the version and a documented dry run dispatched live SSH (issue #3111). + * Mirrors the identical rule in Module.cfc::$deployStripFlags. + */ + private boolean function $nextIsValue(required array args, required numeric i, required numeric n) { + return arguments.i < arguments.n && left(arguments.args[arguments.i + 1], 2) != "--"; + } } diff --git a/cli/lucli/tests/specs/deploy/DeployArgsParserSpec.cfc b/cli/lucli/tests/specs/deploy/DeployArgsParserSpec.cfc index 178ab52e12..cfd2c90a8d 100644 --- a/cli/lucli/tests/specs/deploy/DeployArgsParserSpec.cfc +++ b/cli/lucli/tests/specs/deploy/DeployArgsParserSpec.cfc @@ -68,6 +68,30 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(structKeyExists(opts, "version")).toBeFalse(); }); + // Issue #3111: a bare --release followed by another flag must NOT + // swallow that flag as its value. Before the guard, --release ate + // --dry-run (version became the literal string "--dry-run" and + // dryRun was never set), so a documented dry run dispatched real + // SSH connections. $deployStripFlags in Module.cfc already applies + // the same `left(next, 2) != "--"` rule; the parser now matches. + it("does not swallow a following flag as the value of '--release' (issue ##3111)", () => { + var opts = parser.parse(["app", "boot", "--release", "--dry-run"]); + expect(structKeyExists(opts, "version")).toBeFalse(); + expect(opts.dryRun).toBeTrue(); + }); + + it("does not swallow a following flag as the value of '--version' (issue ##3111)", () => { + var opts = parser.parse(["app", "boot", "--version", "--dry-run"]); + expect(structKeyExists(opts, "version")).toBeFalse(); + expect(opts.dryRun).toBeTrue(); + }); + + it("does not swallow a following flag as the value of '--destination'", () => { + var opts = parser.parse(["--destination", "--dry-run"]); + expect(structKeyExists(opts, "destination")).toBeFalse(); + expect(opts.dryRun).toBeTrue(); + }); + // CLI audit H9: --config aliases --configPath; the deploy guides // document --config but only --configPath was parsed. it("parses --config=value as an alias for configPath", () => { diff --git a/cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc b/cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc index c67e7a2cdc..d056559d28 100644 --- a/cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc +++ b/cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc @@ -96,6 +96,20 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(len(out)).toBeGT(0); expect(out).toInclude("docker stop"); }); + + // Issue #3111: `--release=1` hung ~76s under --dry-run because the + // argv round-trip dropped the numeric value and the parser then + // swallowed --dry-run. Pin the contract at this layer too: a + // dry-run boot with a purely numeric version must never touch the + // SshPool (strict fake throws on ANY unexpected command). + it("dry-run with a numeric version never touches the SshPool (issue ##3111)", () => { + var fake = new cli.lucli.services.deploy.lib.FakeSshPool({strict: true}); + var cli = new cli.lucli.services.deploy.cli.DeployAppCli(fake); + var out = cli.boot({configPath: variables.fixture, version: "1", dryRun: true}); + expect(arrayLen(fake.calls())).toBe(0); + expect(out).toInclude("docker run"); + expect(out).toInclude("demo-web-1"); + }); }); } diff --git a/cli/lucli/tests/specs/services/ArgSpecSpec.cfc b/cli/lucli/tests/specs/services/ArgSpecSpec.cfc index e776869b23..59b972bb52 100644 --- a/cli/lucli/tests/specs/services/ArgSpecSpec.cfc +++ b/cli/lucli/tests/specs/services/ArgSpecSpec.cfc @@ -227,6 +227,42 @@ component extends="wheels.wheelstest.system.BaseSpec" { expect(argv).toInclude("--setup-h2"); }); + // Issue #3111: CFML `==` boolean-coerces both operands, so + // "1" == "true" and "0" == "false" are TRUE. `--release=1` + // arrived as {release: "1"}, got re-emitted as a bare + // --release flag (value dropped), and the downstream deploy + // parser then swallowed the next token — --dry-run — as the + // version, turning a dry run into a live SSH dispatch that + // hung ~76s against the config stub's placeholder host. + it("preserves a value of '1' as --key=1, not a bare flag (issue ##3111)", () => { + var argv = new cli.lucli.services.ArgSpec().toArgv({ + "arg1": "app", + "arg2": "boot", + "release": "1", + "dry-run": "true" + }); + expect(argv).toInclude("--release=1"); + expect(argv).toInclude("--dry-run"); + }); + + it("preserves a value of '0' as --key=0, not a --no- negation (issue ##3111)", () => { + var argv = new cli.lucli.services.ArgSpec().toArgv({"keep": "0"}); + expect(argv).toInclude("--keep=0"); + }); + + it("preserves boolean-castable words (yes/no) as values, not flags (issue ##3111)", () => { + var argv = new cli.lucli.services.ArgSpec().toArgv({"release": "yes", "follow": "no"}); + expect(argv).toInclude("--release=yes"); + expect(argv).toInclude("--follow=no"); + }); + + it("still emits a bare flag for a native boolean true (MCP argCollection)", () => { + var coll = {"arg1": "app"}; + coll["dry-run"] = javaCast("boolean", true); + var argv = new cli.lucli.services.ArgSpec().toArgv(coll); + expect(argv).toInclude("--dry-run"); + }); + }); describe("toInputSchema() — typed MCP tool input schema", () => {