fix(cli): stop numeric --release values from defeating deploy --dry-run - #3146
Conversation
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>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3111 — wheels deploy app boot --release=1 --dry-run opening real SSH connections because CFML's coercing == in ArgSpec.toArgv() turned the value "1" into a bare --release flag, and DeployArgsParser's space-separated value arms then swallowed --dry-run as the version. Both root causes are fixed correctly, the fix mirrors established prior art, and all three layers are pinned with red-first specs. Verdict: approve.
Correctness — verified clean
cli/lucli/services/ArgSpec.cfc:179-185— the exact compare (compareNoCase(stringValue, "true") == 0) is the right tool: it stays case-insensitive (matching the old==behavior for the literal strings LuCLI actually normalizes to) while refusing the boolean coercion that ate1/0/yes/no. The new!isSimpleValue(value) → continueat line 175-177 preserves the old skip behavior for complex values (previously every branch wasisSimpleValue-guarded), andtoString()keeps the native-boolean MCP shape working — pinned by the new spec atArgSpecSpec.cfc:259-264.cli/lucli/services/deploy/cli/DeployArgsParser.cfc:137-139—$nextIsValue()is byte-for-byte the same rule as the pre-existingModule.cfc:2668(left(arguments.args[i+1], 2) != "--"), so the two deploy tokenizers now agree, which was exactly the disagreement that produced the bug. All 17 space-separated value arms are converted uniformly — I checked the full file; none were missed.- Degradation path is sound: a value-less
--releasenow falls through all arms (same as the already-spec-pinned end-of-args silent drop,DeployArgsParserSpec.cfc:61-68) and--dry-runis parsed on the next iteration.
One intentional behavior note worth having on the record (not a defect): forms like --follow=yes that previously worked by accident of boolean coercion (re-emitted as bare --follow) now pass through as the literal --follow=yes, which downstream literal-token matchers ignore. The documented forms (bare --flag, --no-flag, native booleans) are unaffected and now spec-pinned. Non-lossy passthrough is the correct contract for toArgv() per its own docblock (#2855/#2861).
Tests
Thorough — three layers, red-first per the PR's evidence table:
ArgSpecSpec.cfc:237-264— 4 new specs covering1,0,yes/no, and the native-boolean MCP shape.DeployArgsParserSpec.cfc:71-94—--release/--version/--destinationnever swallow a following flag; thedryRunassertions would have failed red (key not set) under the old code.DeployAppCliSpec.cfc:105-112— end-to-end pin withFakeSshPool({strict: true}); I verifiedFakeSshPool.cfc:13-18accepts{strict}via init opts and:90-92throws on any unexpected command, so the zero-calls assertion is backed by a fake that can't silently pass.
The ##3111 escaping in it() titles is correct per the CLAUDE.md #-escape gotcha.
Docs
- Changelog fragment present at
changelog.d/deploy-numeric-release-dry-run-hang.fixed.md— correct<slug>.fixed.mdformat, no directCHANGELOG.mdedit. .ai/wheels/deploy.mdneeds no update: the fix restores its documented--dry-runcontract ("print commands without executing") rather than changing behavior.
Commits
Single commit fix(cli): stop numeric --release values from defeating deploy --dry-run — valid type, valid scope, subject under 100 chars, explains the why. Conforms to commitlint.config.js.
Summary
wheels deploy app boot --release=1 --dry-runhung ~76s withOperation timed out— a documented dry run was opening real SSH connections to the config stub's placeholder host (192.168.0.1). Non-numeric labels (v1,abc1234) worked fine.Fixes #3111
Root cause (two stacked defects)
ArgSpec.toArgv()used CFML's coercing==to detect boolean flags. In CFML,"1" == "true"and"0" == "false"evaluate TRUE (both operands coerce). So the collection{release: "1", dry-run: "true"}was re-emitted as a bare--releaseflag — value dropped (and--release=0became--no-release;yes/nocoerced too). Red-test evidence on origin/develop:toArgv({arg1:"app", arg2:"boot", release:"1", "dry-run":"true"})produced[app, boot, --dry-run, --release], and{keep:"0"}produced[--no-keep].DeployArgsParser's space-separated--release <value>arm swallowed the next token unconditionally (i < nonly). With the bare--releasefrom (1) adjacent to--dry-run, the version became the literal string--dry-runandopts.dryRunwas never set — the dispatch layer then ran live SSH until the TCP connect timeout. NotablyModule.cfc::$deployStripFlagsalready refused to consume a---prefixed token as a value; the two tokenizers disagreed.Fix
toArgv()flag detection now uses an exact string compare (compareNoCase(toString(value), "true"/"false")). Native booleans from MCP argCollections stringify totrue/falseand keep emitting bare/--no-flags;1,0,yes,nosurvive as values.DeployArgsParsernow goes through$nextIsValue(), which refuses to consume a following--token — mirroring the existing$deployStripFlagsrule. A value-less--releasenow degrades to the established silent-drop behavior (thenDeployAppClithrows its clearMissingVersionerror) instead of eating the next flag.Specs (TDD red-first, pinning all three layers)
ArgSpecSpec:--release=1/--keep=0/yes/noround-trip as values, not flags; native boolean still emits a bare flag (4 new)DeployArgsParserSpec:--release/--version/--destinationnever swallow a following flag;--dry-runsurvives (3 new)DeployAppCliSpec: dry-run boot with a numeric version performs zero SshPool calls (strictFakeSshPoolthat throws on any unexpected command) + still renders the buffereddocker run ... demo-web-1line (1 new)Evidence
Docker harness (lucee7,
/wheels/cli/tests?format=json):The remaining 1 fail / 2 errors are pre-existing harness artifacts, identical in the red baseline:
SshClientSpec/SshPoolSpec(docker-in-docker unavailable:docker: command not foundintools/deploy-sshd-up.sh) andServerCommandsSpecreload-endpoint (needs a live managed server).🤖 Generated with Claude Code