[bpamiri]

Summary

Continues the #2861 ArgSpec migration (foundation in #2862; eight leaf commands in the prior batch) by moving two more CLI subcommands — console and test — off the getArgs() / argsFromCollection() argv round-trip and onto cli.lucli.services.ArgSpec, consuming LuCLI's structured argCollection directly.

These are the two genuinely leaf commands left (they fully consume their args). The remaining getArgs() callers all forward raw argv onward and are deferred: the dispatchers (generate, create, db, browser), the parser-delegating deploy / packages / migrate, and the LuCLI-passthrough start. This drops the getArgs() callers from 10 → 8.

What changed

• parseConsoleArgs(coll) — --password=<value> via ArgSpec.option; an empty result lets the command auto-detect the reload password.
• parseTestArgs(coll) — --filter (+ documented --directory alias), --reporter, --db (tracked explicit so the runner distinguishes an implicit default from a chosen one), --verbose / --ci / --core, --no-test-db, a bare positional filter, and -v (delivered by LuCLI as a positional). Preserves $normalizeTestFilter and the full runTests(...) argument set.
• console() / test() now call parse…Args(structuredArgs(arguments)); downstream logic is unchanged (same variable names).
• Probe pass-throughs ($parseConsoleArgs, $parseTestArgs) + 13 new server-free specs in CommandArgParsingSpec.cfc.

Behavioral notes

• Fixes the latent arg1-gate the round trip masked: named-only invocations (wheels console --password=x, wheels test --core with no positional) now take effect instead of silently running with defaults.
• One deliberate delta per command: the space-separated option forms (wheels console --password secret, wheels test --filter models) give way to the --key=value forms — LuCLI delivers a space-separated value as a bare flag + a separate positional, never a named value. Consistent with the leaf-command batch.

Verification

TDD (red → green) on Lucee 7 via the CLI suite (/wheels/cli/tests):

• CommandArgParsingSpec: 38 → 51 pass, 0 fail, 0 error (the 13 new specs).
• No new failures attributable to this change. The only red bundles in the local Docker harness (SshClientSpec, SshPoolSpec, ServerCommandsSpec) are pre-existing, server/SSH-dependent, and pass in CI (the full CLI suite is green on develop — 681/0/0 — before this change, which adds 13). Confirmed via stash-isolate-rerun.

Type of Change

• Refactoring (no behavior change beyond the two documented deltas)

Feature Completeness Checklist

• DCO sign-off — Signed-off-by: on the commit
• Tests — 13 new specs in CommandArgParsingSpec.cfc
• CHANGELOG.md — entry under [Unreleased]
• Framework Docs — n/a (CLI-internal argument sourcing)

Refs #2861, #2862, #2855.

[wheels-bot]

Wheels Bot — Reviewer A

This PR continues the ArgSpec migration (#2861) by moving console and test off the getArgs()/argsFromCollection() argv round-trip. The implementation is clean, the documented behavioral deltas are honest, and the 13 new specs give solid coverage of the main paths. I'm approving; two minor test-coverage gaps are noted below.

Correctness

No logic bugs found.

dbExplicit detection (Module.cfc line 595) uses structKeyExists(arguments.coll, "db") rather than checking the parsed value. This is the correct approach — it distinguishes a user-supplied --db=sqlite from the silent default, which is exactly what the runner needs.

--directory precedence (Module.cfc line 564):

var filter = len(parsed.directory) ? parsed.directory : parsed.filter;

When both --filter and --directory are supplied simultaneously, --directory wins. The old round-trip had non-deterministic behavior here (struct iteration order in argsFromCollection), so the new behavior is strictly more predictable. No objection — but the test suite doesn't cover this combination (see Tests below).

Positional + named filter together: when {filter: "models", arg1: "security"} arrives, parsed.filter is set first and the !len(filter) guard on line 582 prevents the positional from overriding it. The old argsFromCollection put positionals before named flags in the reconstructed argv, so named flags also won in the old code. The semantics are preserved. ✓

Cross-engine

The CLI test suite runs on LuCLI (Lucee 7), and the added code uses only standard CFML: reFindNoCase, mid, left, len, val, trim, struct/array operations. No bracket-notation calls inside closures, no application-scope function members, no Left(str, 0) Lucee 7 crash, no struct member-function collisions. ✓

The positional-collection walk (Module.cfc lines 572–584) uses standard for (var key in struct) iteration, not a closure, so the closure-scope capture and obj["key"]() anti-patterns don't apply. ✓

Tests

13 new specs in CommandArgParsingSpec.cfc via ModuleArgvProbe. Happy path, defaults, each named option, each flag, the -v-as-positional edge case, and the -v models index-gap case are all covered. Two gaps worth filling in a follow-up:

1. Both --filter and --directory supplied simultaneously. The current behavior (--directory wins) is deterministic and better than the old code, but there is no spec to pin it. A one-liner would do:

   it("directory alias wins when both --filter and --directory are supplied", () => {
       var o = probe.$parseTestArgs({filter: "models", directory: "controllers"});
       expect(o.filter).toBe("controllers");
   });

2. Named --filter + bare positional combined. The !len(filter) guard on line 582 correctly makes the named option win, but there is no spec asserting this. A test like {filter: "models", arg1: "security"} → filter == "models" would lock in the precedence rule.

Neither gap is a blocker; both are easily added to CommandArgParsingSpec independently.

Docs

CHANGELOG [Unreleased] entry is present. The entry is lengthy but matches the verbose style established by the prior eight-leaf-command entry directly above it. No change to user-facing guides is required (CLI-internal argument sourcing). ✓

Commits

Single commit: refactor(cli): migrate console + test to ArgSpec, dropping the argv round-trip (#2861) — 88-character header, valid refactor type, cli scope, not ALL-CAPS. DCO Signed-off-by: Peter Amiri <peter@alurium.com> is present. ✓

The commit body honestly flags the two behavioral deltas (named-only invocations fixed; space-separated forms dropped for --key=value). Good.

Security

No SQL, no user-input reflection, no secrets in fixtures. ✓

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is substantive and well-grounded. The correctness analysis tracks the actual diff, the cross-engine checklist is accurate, and the two flagged test gaps are real. One minor factual error in the commit-header count (A wrote 88 chars; the title is 86), but this does not affect the conclusion. I align with the approve verdict.

Sycophancy

None detected. Each approval is backed by a specific code citation or a direct comparison to the old behavior.

False positives

One minor count error worth noting, not a material false positive:

• A wrote the PR title header is "88-character" — it is 86 characters (echo -n "refactor(cli): migrate console + test to ArgSpec, dropping the argv round-trip (#2861)" | wc -c → 86). The title still passes the ≤ 100-char gate, so the conclusion is unaffected.

No other false positives. A's claims about dbExplicit using structKeyExists(arguments.coll, "db") (checking the raw collection, not the defaults-filled parsed struct), the --directory precedence branch, and the !len(filter) positional guard are all accurate against the diff.

Missed issues

None. The two test-coverage gaps A identified (simultaneous --filter + --directory, and named --filter + bare positional) are the only obvious gaps. The bracket-access arguments.coll["arg" & idx] in the positional loop is not inside a closure, so the Adobe CF 2021/2023 obj["key"]() parser crash (cross-engine invariant #4) does not apply. left(token, 2) is guarded by len(token) so the Lucee 7 Left(str, 0) crash (invariant #8) does not trigger. No security, migration, or DI surface touched.

Verdict alignment

Approve is consistent with the findings: no logic bugs, clean cross-engine profile, adequate coverage with two non-blocking gaps that are easy follow-ups to CommandArgParsingSpec.

Convergence

Aligned. A and I agree: the PR is correct, the behavioral deltas are documented and intentional, and the two missing specs are minor enough not to block. Joint recommendation is approve.