Skip to content

Commit 2ea1064

Browse files
bpamiribpamiri
authored
fix(cli): stop numeric --release values from defeating deploy --dry-run (#3146)
A purely numeric release label (--release=1) hung ~76s with 'Operation timed out' even under --dry-run. Two stacked defects: 1. ArgSpec.toArgv() detected boolean flags with CFML's coercing == operator, and "1" == "true" / "0" == "false" evaluate TRUE in CFML. The value was dropped and re-emitted as a bare --release flag (likewise "0" became --no-release, and yes/no coerced too). 2. DeployArgsParser's space-separated '--release <value>' arm then swallowed the next token — --dry-run — as the version, so dryRun was never set and the dispatch layer opened real SSH connections to the config stub's placeholder host (192.168.0.1) until the TCP timeout. Fix: toArgv() now uses an exact string compare (native booleans from MCP argCollections still stringify to true/false and keep working), and every value-taking deploy flag refuses to consume a following '--' token as its value — mirroring the rule Module.cfc's $deployStripFlags already applied, so the two tokenizers agree. Specs pin all three layers: the argv round-trip preserves 1/0/yes/no as values, the parser never swallows a flag, and a strict FakeSshPool proves a dry-run boot with a numeric version performs zero SSH calls. Fixes #3111 Signed-off-by: Peter Amiri <peter@alurium.com>
1 parent 024c4dd commit 2ea1064

6 files changed

Lines changed: 121 additions & 21 deletions

File tree

‎changelog.d/deploy-numeric-release-dry-run-hang.fixed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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))

‎cli/lucli/services/ArgSpec.cfc‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,17 +158,30 @@ component {
158158
}
159159

160160
// Named keys, re-prefixed. --no-X for false preserves the negation.
161+
// Flag detection MUST use an exact string compare: CFML `==` coerces
162+
// both operands, so "1" == "true" and "0" == "false" evaluate TRUE.
163+
// That coercion turned `--release=1` into a bare --release flag (value
164+
// dropped) and the downstream deploy parser then swallowed --dry-run
165+
// as the version — a documented dry run dispatched live SSH and hung
166+
// ~76s against the config stub's placeholder host (issue #3111).
167+
// LuCLI normalizes flags to the literal strings "true"/"false"; MCP
168+
// argCollections may carry native booleans, which toString() renders
169+
// as "true"/"false" — both shapes match the exact compare.
161170
for (var key in arguments.coll) {
162171
if (reFindNoCase("^arg\d+$", key)) {
163172
continue;
164173
}
165174
var value = arguments.coll[key];
166-
if (isSimpleValue(value) && value == "true") {
175+
if (!isSimpleValue(value)) {
176+
continue;
177+
}
178+
var stringValue = toString(value);
179+
if (compareNoCase(stringValue, "true") == 0) {
167180
arrayAppend(result, "--" & key);
168-
} else if (isSimpleValue(value) && value == "false") {
181+
} else if (compareNoCase(stringValue, "false") == 0) {
169182
arrayAppend(result, "--no-" & key);
170-
} else if (isSimpleValue(value)) {
171-
arrayAppend(result, "--" & key & "=" & value);
183+
} else {
184+
arrayAppend(result, "--" & key & "=" & stringValue);
172185
}
173186
}
174187

‎cli/lucli/services/deploy/cli/DeployArgsParser.cfc‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ component {
2424
opts.dryRun = true;
2525
} else if (left(a, 14) == "--destination=") {
2626
opts.destination = mid(a, 15, 99999);
27-
} else if (a == "--destination" && i < n) {
27+
} else if (a == "--destination" && $nextIsValue(arguments.args, i, n)) {
2828
opts.destination = arguments.args[i+1];
2929
i++;
3030
} else if (left(a, 10) == "--version=") {
@@ -33,89 +33,89 @@ component {
3333
// has rewritten --version -> --release first, or when arrays are
3434
// constructed programmatically (tests).
3535
opts.version = mid(a, 11, 99999);
36-
} else if (a == "--version" && i < n) {
36+
} else if (a == "--version" && $nextIsValue(arguments.args, i, n)) {
3737
opts.version = arguments.args[i+1];
3838
i++;
3939
} else if (left(a, 10) == "--release=") {
4040
// picocli-safe alias for --version (issue #2674).
4141
opts.version = mid(a, 11, 99999);
42-
} else if (a == "--release" && i < n) {
42+
} else if (a == "--release" && $nextIsValue(arguments.args, i, n)) {
4343
opts.version = arguments.args[i+1];
4444
i++;
4545
} else if (left(a, 13) == "--configPath=") {
4646
opts.configPath = mid(a, 14, 99999);
47-
} else if (a == "--configPath" && i < n) {
47+
} else if (a == "--configPath" && $nextIsValue(arguments.args, i, n)) {
4848
opts.configPath = arguments.args[i+1];
4949
i++;
5050
} else if (left(a, 9) == "--config=") {
5151
// Alias for --configPath — the deploy guides document --config. CLI audit H9.
5252
opts.configPath = mid(a, 10, 99999);
53-
} else if (a == "--config" && i < n) {
53+
} else if (a == "--config" && $nextIsValue(arguments.args, i, n)) {
5454
opts.configPath = arguments.args[i+1];
5555
i++;
5656
} else if (a == "--force") {
5757
opts.force = true;
5858
} else if (left(a, 10) == "--service=") {
5959
opts.service = mid(a, 11, 99999);
60-
} else if (a == "--service" && i < n) {
60+
} else if (a == "--service" && $nextIsValue(arguments.args, i, n)) {
6161
opts.service = arguments.args[i+1];
6262
i++;
6363
} else if (left(a, 8) == "--image=") {
6464
opts.image = mid(a, 9, 99999);
65-
} else if (a == "--image" && i < n) {
65+
} else if (a == "--image" && $nextIsValue(arguments.args, i, n)) {
6666
opts.image = arguments.args[i+1];
6767
i++;
6868
} else if (left(a, 20) == "--registry-username=") {
6969
opts.registryUsername = mid(a, 21, 99999);
70-
} else if (a == "--registry-username" && i < n) {
70+
} else if (a == "--registry-username" && $nextIsValue(arguments.args, i, n)) {
7171
opts.registryUsername = arguments.args[i+1];
7272
i++;
7373
} else if (left(a, 7) == "--host=") {
7474
opts.host = mid(a, 8, 99999);
75-
} else if (a == "--host" && i < n) {
75+
} else if (a == "--host" && $nextIsValue(arguments.args, i, n)) {
7676
opts.host = arguments.args[i+1];
7777
i++;
7878
} else if (left(a, 7) == "--keep=") {
7979
opts.keep = mid(a, 8, 99999);
80-
} else if (a == "--keep" && i < n) {
80+
} else if (a == "--keep" && $nextIsValue(arguments.args, i, n)) {
8181
opts.keep = arguments.args[i+1];
8282
i++;
8383
} else if (left(a, 10) == "--message=") {
8484
opts.message = mid(a, 11, 99999);
85-
} else if (a == "--message" && i < n) {
85+
} else if (a == "--message" && $nextIsValue(arguments.args, i, n)) {
8686
opts.message = arguments.args[i+1];
8787
i++;
8888
} else if (left(a, 10) == "--adapter=") {
8989
opts.adapter = mid(a, 11, 99999);
90-
} else if (a == "--adapter" && i < n) {
90+
} else if (a == "--adapter" && $nextIsValue(arguments.args, i, n)) {
9191
opts.adapter = arguments.args[i+1];
9292
i++;
9393
} else if (left(a, 10) == "--account=") {
9494
opts.account = mid(a, 11, 99999);
95-
} else if (a == "--account" && i < n) {
95+
} else if (a == "--account" && $nextIsValue(arguments.args, i, n)) {
9696
opts.account = arguments.args[i+1];
9797
i++;
9898
} else if (left(a, 7) == "--from=") {
9999
opts.from = mid(a, 8, 99999);
100-
} else if (a == "--from" && i < n) {
100+
} else if (a == "--from" && $nextIsValue(arguments.args, i, n)) {
101101
opts.from = arguments.args[i+1];
102102
i++;
103103
} else if (a == "--confirm") {
104104
opts.confirm = true;
105105
} else if (left(a, 7) == "--tail=") {
106106
opts.tail = mid(a, 8, 99999);
107-
} else if (a == "--tail" && i < n) {
107+
} else if (a == "--tail" && $nextIsValue(arguments.args, i, n)) {
108108
opts.tail = arguments.args[i+1];
109109
i++;
110110
} else if (left(a, 7) == "--role=") {
111111
// `deploy app <verb>` role filter; DeployAppCli reads opts.role. CLI audit H9.
112112
opts.role = mid(a, 8, 99999);
113-
} else if (a == "--role" && i < n) {
113+
} else if (a == "--role" && $nextIsValue(arguments.args, i, n)) {
114114
opts.role = arguments.args[i+1];
115115
i++;
116116
} else if (left(a, 12) == "--container=") {
117117
opts.container = mid(a, 13, 99999);
118-
} else if (a == "--container" && i < n) {
118+
} else if (a == "--container" && $nextIsValue(arguments.args, i, n)) {
119119
opts.container = arguments.args[i+1];
120120
i++;
121121
} else if (a == "--follow") {
@@ -125,4 +125,16 @@ component {
125125
}
126126
return opts;
127127
}
128+
129+
/**
130+
* True when the token after position i exists and is a plain value, not
131+
* another `--` flag. Guards every space-separated `--flag value` arm so a
132+
* value-taking flag with a missing value can never swallow the flag that
133+
* follows it — before this, `--release --dry-run` consumed --dry-run as
134+
* the version and a documented dry run dispatched live SSH (issue #3111).
135+
* Mirrors the identical rule in Module.cfc::$deployStripFlags.
136+
*/
137+
private boolean function $nextIsValue(required array args, required numeric i, required numeric n) {
138+
return arguments.i < arguments.n && left(arguments.args[arguments.i + 1], 2) != "--";
139+
}
128140
}

‎cli/lucli/tests/specs/deploy/DeployArgsParserSpec.cfc‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,30 @@ component extends="wheels.wheelstest.system.BaseSpec" {
6868
expect(structKeyExists(opts, "version")).toBeFalse();
6969
});
7070

71+
// Issue #3111: a bare --release followed by another flag must NOT
72+
// swallow that flag as its value. Before the guard, --release ate
73+
// --dry-run (version became the literal string "--dry-run" and
74+
// dryRun was never set), so a documented dry run dispatched real
75+
// SSH connections. $deployStripFlags in Module.cfc already applies
76+
// the same `left(next, 2) != "--"` rule; the parser now matches.
77+
it("does not swallow a following flag as the value of '--release' (issue ##3111)", () => {
78+
var opts = parser.parse(["app", "boot", "--release", "--dry-run"]);
79+
expect(structKeyExists(opts, "version")).toBeFalse();
80+
expect(opts.dryRun).toBeTrue();
81+
});
82+
83+
it("does not swallow a following flag as the value of '--version' (issue ##3111)", () => {
84+
var opts = parser.parse(["app", "boot", "--version", "--dry-run"]);
85+
expect(structKeyExists(opts, "version")).toBeFalse();
86+
expect(opts.dryRun).toBeTrue();
87+
});
88+
89+
it("does not swallow a following flag as the value of '--destination'", () => {
90+
var opts = parser.parse(["--destination", "--dry-run"]);
91+
expect(structKeyExists(opts, "destination")).toBeFalse();
92+
expect(opts.dryRun).toBeTrue();
93+
});
94+
7195
// CLI audit H9: --config aliases --configPath; the deploy guides
7296
// document --config but only --configPath was parsed.
7397
it("parses --config=value as an alias for configPath", () => {

‎cli/lucli/tests/specs/deploy/cli/DeployAppCliSpec.cfc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,20 @@ component extends="wheels.wheelstest.system.BaseSpec" {
9696
expect(len(out)).toBeGT(0);
9797
expect(out).toInclude("docker stop");
9898
});
99+
100+
// Issue #3111: `--release=1` hung ~76s under --dry-run because the
101+
// argv round-trip dropped the numeric value and the parser then
102+
// swallowed --dry-run. Pin the contract at this layer too: a
103+
// dry-run boot with a purely numeric version must never touch the
104+
// SshPool (strict fake throws on ANY unexpected command).
105+
it("dry-run with a numeric version never touches the SshPool (issue ##3111)", () => {
106+
var fake = new cli.lucli.services.deploy.lib.FakeSshPool({strict: true});
107+
var cli = new cli.lucli.services.deploy.cli.DeployAppCli(fake);
108+
var out = cli.boot({configPath: variables.fixture, version: "1", dryRun: true});
109+
expect(arrayLen(fake.calls())).toBe(0);
110+
expect(out).toInclude("docker run");
111+
expect(out).toInclude("demo-web-1");
112+
});
99113
});
100114
}
101115

‎cli/lucli/tests/specs/services/ArgSpecSpec.cfc‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,42 @@ component extends="wheels.wheelstest.system.BaseSpec" {
227227
expect(argv).toInclude("--setup-h2");
228228
});
229229

230+
// Issue #3111: CFML `==` boolean-coerces both operands, so
231+
// "1" == "true" and "0" == "false" are TRUE. `--release=1`
232+
// arrived as {release: "1"}, got re-emitted as a bare
233+
// --release flag (value dropped), and the downstream deploy
234+
// parser then swallowed the next token — --dry-run — as the
235+
// version, turning a dry run into a live SSH dispatch that
236+
// hung ~76s against the config stub's placeholder host.
237+
it("preserves a value of '1' as --key=1, not a bare flag (issue ##3111)", () => {
238+
var argv = new cli.lucli.services.ArgSpec().toArgv({
239+
"arg1": "app",
240+
"arg2": "boot",
241+
"release": "1",
242+
"dry-run": "true"
243+
});
244+
expect(argv).toInclude("--release=1");
245+
expect(argv).toInclude("--dry-run");
246+
});
247+
248+
it("preserves a value of '0' as --key=0, not a --no- negation (issue ##3111)", () => {
249+
var argv = new cli.lucli.services.ArgSpec().toArgv({"keep": "0"});
250+
expect(argv).toInclude("--keep=0");
251+
});
252+
253+
it("preserves boolean-castable words (yes/no) as values, not flags (issue ##3111)", () => {
254+
var argv = new cli.lucli.services.ArgSpec().toArgv({"release": "yes", "follow": "no"});
255+
expect(argv).toInclude("--release=yes");
256+
expect(argv).toInclude("--follow=no");
257+
});
258+
259+
it("still emits a bare flag for a native boolean true (MCP argCollection)", () => {
260+
var coll = {"arg1": "app"};
261+
coll["dry-run"] = javaCast("boolean", true);
262+
var argv = new cli.lucli.services.ArgSpec().toArgv(coll);
263+
expect(argv).toInclude("--dry-run");
264+
});
265+
230266
});
231267

232268
describe("toInputSchema() — typed MCP tool input schema", () => {

0 commit comments

Comments
 (0)