Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions changelog.d/deploy-numeric-release-dry-run-hang.fixed.md
Original file line number Diff line number Diff line change
@@ -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))
21 changes: 17 additions & 4 deletions cli/lucli/services/ArgSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
46 changes: 29 additions & 17 deletions cli/lucli/services/deploy/cli/DeployArgsParser.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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=") {
Expand All @@ -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 <verb>` 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") {
Expand All @@ -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) != "--";
}
}
24 changes: 24 additions & 0 deletions cli/lucli/tests/specs/deploy/DeployArgsParserSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
14 changes: 14 additions & 0 deletions cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
}

Expand Down
36 changes: 36 additions & 0 deletions cli/lucli/tests/specs/services/ArgSpecSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Loading