Skip to content

Commit 06e490c

Browse files
wheels-bot[bot]github-actions[bot]bpamiri
authored
fix(cli): wheels packages help now documents add as canonical install verb (#2729)
* fix(cli): `wheels packages` help now documents `add` as canonical install verb LuCLI auto-introspection emitted a help row advertising `install <name> [--force] Install a package`, but LuCLI's own built-in extension installer intercepts the literal `install` verb across all modules before dispatch reaches `Module.cfc::packages()`. The advertised verb is unreachable — same trap that hit `wheels browser install` (renamed to `wheels browser setup` in #2345). Fix: own the help text directly. `wheels packages help`, `wheels packages --help`, and `wheels packages -h` now short-circuit to a hand-written `$packagesHelp()` that documents `add` as the canonical install verb, lists every reachable sub-verb, and includes a Note paragraph explaining why `install` does not work. Short-circuit fires before any network call (registry, manifest fetch), so help is always fast and offline. Spec at `cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc` asserts the help output documents `add`, lists every canonical sub-verb, mentions LuCLI interception, and does not advertise an `install <name> [--force] Install a package` row. Fixes #2713 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(cli): drop dead -h guard clause and unblock TDD gate for cli/lucli specs Addresses Reviewer A & B consensus from PR #2729 (round 1): - `.github/workflows/bot-tdd-gate.yml`: add `cli/lucli/tests/specs/` to both the spec regex and the `impl_changes` exclusion. The packages help spec lives there, so the gate currently rejects the PR with "Bot PRs must include a failing-then-passing spec" even though the spec exists. Excluding the path from `impl_changes` also avoids double-counting it as both spec and implementation, which B flagged as a latent risk. - `cli/lucli/Module.cfc`: remove the dead `|| sub == "-h"` clause from the packages() help short-circuit. `-h` is consumed by $packagesArgsToOptions (sets opts.help = true) and stripped from positionals by $packagesStripFlags before `sub` is read, so the third clause is unreachable. Added a comment documenting the data flow and parenthesized the Elvis expression for clarity. - `cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc`: add a third test case exercising `[\"-h\"]` directly. Asserts the short flag reaches the same hand-written help body as `help` and `--help` (verified by the presence of "wheels packages add" in the output). Skipped per B's round-1 analysis: the DCO/Signed-off-by trailer change A proposed. The existing trailer matches the commit author, which is what the DCO App validates — rewriting it would create a mismatch. There is also no DCO check in the current required-checks list. Refs #2713 --------- Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Peter Amiri <peter@alurium.com>
1 parent 81321f7 commit 06e490c

4 files changed

Lines changed: 143 additions & 4 deletions

File tree

.github/workflows/bot-tdd-gate.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ jobs:
5858
echo "$changed"
5959
echo "---"
6060
61-
spec_changes=$(echo "$changed" | grep -E '^(tests/specs/|vendor/wheels/tests/specs/)' || true)
61+
spec_changes=$(echo "$changed" | grep -E '^(tests/specs/|vendor/wheels/tests/specs/|cli/lucli/tests/specs/)' || true)
6262
if [[ -z "$spec_changes" ]]; then
63-
echo "::error::Bot PRs must include a failing-then-passing spec under tests/specs/ or vendor/wheels/tests/specs/"
63+
echo "::error::Bot PRs must include a failing-then-passing spec under tests/specs/, vendor/wheels/tests/specs/, or cli/lucli/tests/specs/"
6464
echo ""
6565
echo "This PR was authored by the bot (or on a bot branch) but contains no spec changes."
6666
echo "Either add a spec, or close this PR and reopen with one."
@@ -70,7 +70,7 @@ jobs:
7070
# Narrow the test-path exclusion to `specs/` so non-spec files under tests/ (e.g.
7171
# vendor/wheels/tests/html.cfm, the test-runner result-page UI rendered to users'
7272
# browsers) correctly count as implementation when a bot fix touches them.
73-
impl_changes=$(echo "$changed" | grep -vE '^(tests/specs/|vendor/wheels/tests/specs/|\.ai/|CHANGELOG\.md|docs/|web/|\.github/)' || true)
73+
impl_changes=$(echo "$changed" | grep -vE '^(tests/specs/|vendor/wheels/tests/specs/|cli/lucli/tests/specs/|\.ai/|CHANGELOG\.md|docs/|web/|\.github/)' || true)
7474
if [[ -z "$impl_changes" ]]; then
7575
echo "::error::Bot PR has tests but no implementation"
7676
echo ""

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo
3535

3636
### Fixed
3737

38+
- `wheels packages --help` / `wheels packages help` / `wheels packages -h` now emit a module-owned help string that documents `add` as the canonical install verb and explains why typing `install` does not work (LuCLI's built-in extension installer intercepts the literal verb before dispatch reaches the module — same trap that hit `wheels browser install``wheels browser setup` in #2345). Previously the auto-introspected help drifted from the real CLI surface, advertising an `install <name> [--force]` row that never actually installed anything (#2713)
3839
- Package manifest field reference in `web/sites/guides/.../packages.mdx` (both v4-0-0 and v4-0-1-snapshot copies) and `CLAUDE.md`: the inter-package dependency field is `requires`, not `dependencies`. The legacy 3.x plugin shape used `dependencies` in `box.json`; the modern `PackageLoader` (`vendor/wheels/ModuleGraph.cfc`) has always read `requires`, plus `replaces` (exclusion / migration path) and `suggests` (soft load-order edge). Copying the old example manifest would have shipped a package that loaded but silently ignored its declared dependencies — no error, no warning, just a missing-dep failure at the first runtime call into the absent dependency. All three docs now use `requires` and the previously undocumented `replaces` / `suggests` fields are covered alongside. Same PR also tightens the guide's description of `wheelsVersion` mismatches: not just "logged" but a hard skip — incompatible packages are excluded from the load order before their CFC is instantiated and recorded in `failedPackages` with the constraint and running version named in the log (#2734)
3940
- `paginationLinks()` now emits a one-time per-request `WriteLog(type="warning", ...)` deprecation notice pointing 3.x → 4.x upgraders at `paginationNav()` (the all-in-one helper) and the individual `firstPageLink`/`previousPageLink`/`pageNumberLinks`/`nextPageLink`/`lastPageLink` composables. `wheels upgrade check --to=4.0.0` now also greps `app/views/` for `paginationLinks(` and flags every hit with a remediation pointer, closing the silent-rot gap surfaced by titan Phase 2.4 (#2714)
4041
- `paginationNav()` now throws `Wheels.PaginationNav.InvalidArgument` when passed an argument that none of its sub-helpers (`paginationInfo`, `firstPageLink`, `previousPageLink`, `pageNumberLinks`, `nextPageLink`, `lastPageLink`) accept. Previously, typos such as `prependToList="<ul>"` were silently dropped by CFML's `argumentCollection` dispatch, leaving users to wonder why a styling argument had no effect. The check is gated on `application.wheels.showErrorInformation` so production is unaffected; development environments fail fast and the error names both the rejected arguments and the full allowlist of accepted pass-through keys (#2717)

cli/lucli/Module.cfc

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2056,6 +2056,21 @@ component extends="modules.BaseModule" {
20562056
var positional = $packagesStripFlags(args);
20572057
var sub = arrayLen(positional) >= 1 ? positional[1] : "list";
20582058

2059+
// `--help` / `-h` short-circuits to a deterministic help string the
2060+
// module owns directly. LuCLI's auto-introspected help previously
2061+
// drifted from the real CLI surface — advertising the dead `install`
2062+
// verb that LuCLI itself intercepts (#2713). Owning the text here
2063+
// guarantees `wheels packages help`, `wheels packages --help`, and
2064+
// `wheels packages -h` all reach $packagesHelp().
2065+
//
2066+
// Note: `-h` is consumed by $packagesArgsToOptions (sets opts.help =
2067+
// true) and stripped from positionals by $packagesStripFlags before
2068+
// `sub` is read, so it arrives here as opts.help — never as a
2069+
// positional. No `sub == "-h"` clause is needed.
2070+
if ((opts.help ?: false) || sub == "help") {
2071+
return $packagesHelp();
2072+
}
2073+
20592074
switch (sub) {
20602075
case "list":
20612076
var mainCli = new modules.wheels.services.packages.PackagesMainCli();
@@ -2121,6 +2136,43 @@ component extends="modules.BaseModule" {
21212136
}
21222137
}
21232138

2139+
// Hand-written help for `wheels packages`. Owned by the module rather than
2140+
// auto-derived from picocli introspection because the auto-help drifted
2141+
// from the real CLI surface (#2713 — advertised `install <name> [--force]`
2142+
// even though LuCLI's built-in extension installer intercepts the literal
2143+
// `install` verb before dispatch reaches this module). Same trap that hit
2144+
// `wheels browser install` (renamed to `setup` in #2345).
2145+
private string function $packagesHelp() {
2146+
var nl = chr(10);
2147+
var help = "Usage: wheels packages <subcommand> [options]" & nl;
2148+
help &= " Install, update, search, and list Wheels packages from the wheels-packages registry." & nl & nl;
2149+
help &= "Subcommands:" & nl;
2150+
help &= " list [--tag=<tag>] List packages (optionally filtered by tag)" & nl;
2151+
help &= " search <query> Search package names, descriptions, and tags" & nl;
2152+
help &= " show <name> Show package details and compatible versions" & nl;
2153+
help &= " add <name>[@<version>] [--force] Install a package into vendor/<name>/ (canonical)" & nl;
2154+
help &= " update <name> --yes Update an installed package" & nl;
2155+
help &= " update --all --yes Update every installed package" & nl;
2156+
help &= " remove <name> Delete an installed package from vendor/" & nl;
2157+
help &= " registry refresh Bust the 24-hour registry cache" & nl;
2158+
help &= " registry info Show the registry URL and cache state" & nl;
2159+
help &= " help, --help, -h Show this help" & nl & nl;
2160+
help &= "Note: the install verb is `add`, NOT `install`." & nl;
2161+
help &= " Typing `wheels packages install <name>` is intercepted by LuCLI's built-in" & nl;
2162+
help &= " extension installer before dispatch reaches this module, and prints" & nl;
2163+
help &= " '[INFO] No git or extension dependencies to install' without installing" & nl;
2164+
help &= " anything. Use `wheels packages add <name>` instead. Same trap that bit" & nl;
2165+
help &= " `wheels browser install` (renamed to `wheels browser setup` in #2345)." & nl & nl;
2166+
help &= "Examples:" & nl;
2167+
help &= " wheels packages list" & nl;
2168+
help &= " wheels packages search ui" & nl;
2169+
help &= " wheels packages add wheels-basecoat" & nl;
2170+
help &= " wheels packages add wheels-basecoat@1.0.1" & nl;
2171+
help &= " wheels packages update --all --yes" & nl;
2172+
help &= " wheels packages remove wheels-basecoat" & nl;
2173+
return help;
2174+
}
2175+
21242176
private struct function $packagesArgsToOptions(required array args) {
21252177
var opts = {};
21262178
var n = arrayLen(arguments.args);
@@ -2133,6 +2185,8 @@ component extends="modules.BaseModule" {
21332185
opts.yes = true;
21342186
} else if (a == "--force") {
21352187
opts.force = true;
2188+
} else if (a == "--help" || a == "-h") {
2189+
opts.help = true;
21362190
} else if (left(a, 6) == "--tag=") {
21372191
opts.tag = mid(a, 7, 99999);
21382192
} else if (a == "--tag" && i < n) {
@@ -2151,13 +2205,17 @@ component extends="modules.BaseModule" {
21512205
while (i <= n) {
21522206
var a = arguments.args[i];
21532207
if (left(a, 2) == "--") {
2154-
var booleans = "--all,--yes,--force";
2208+
var booleans = "--all,--yes,--force,--help";
21552209
if (!find("=", a) && !listFindNoCase(booleans, a) && i < n && left(arguments.args[i+1], 2) != "--") {
21562210
i++;
21572211
}
21582212
i++;
21592213
continue;
21602214
}
2215+
if (a == "-h") {
2216+
i++;
2217+
continue;
2218+
}
21612219
arrayAppend(out, a);
21622220
i++;
21632221
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* Tests `wheels packages help` / `wheels packages --help` via Module.cfc.
3+
*
4+
* Issue #2713: the help output must document `add` (not `install`) as the
5+
* canonical install verb, and must explain why `install` does not work
6+
* (LuCLI's built-in extension installer intercepts the literal verb before
7+
* dispatch reaches this module).
8+
*/
9+
component extends="wheels.wheelstest.system.BaseSpec" {
10+
11+
function beforeAll() {
12+
variables.testHelper = new cli.lucli.tests.TestHelper();
13+
variables.tempRoot = testHelper.scaffoldTempProject(expandPath("/"));
14+
variables.mod = new cli.lucli.Module(cwd = variables.tempRoot);
15+
}
16+
17+
function afterAll() {
18+
testHelper.cleanupTempProject(variables.tempRoot);
19+
}
20+
21+
function run() {
22+
23+
describe("wheels packages help", () => {
24+
25+
it("treats `help` positional as a help request (no network call)", () => {
26+
mod.__arguments = ["help"];
27+
var out = mod.packages();
28+
expect(Len(out)).toBeGT(0);
29+
});
30+
31+
it("treats `--help` flag as a help request", () => {
32+
mod.__arguments = ["--help"];
33+
var out = mod.packages();
34+
expect(Len(out)).toBeGT(0);
35+
});
36+
37+
it("treats `-h` short flag as a help request", () => {
38+
mod.__arguments = ["-h"];
39+
var out = mod.packages();
40+
expect(Len(out)).toBeGT(0);
41+
// Sanity: the short flag reaches the same hand-written help body,
42+
// so it should mention `add` just like the other two forms.
43+
expect(out).toInclude("wheels packages add");
44+
});
45+
46+
it("documents `add` as the canonical install verb", () => {
47+
mod.__arguments = ["help"];
48+
var out = mod.packages();
49+
expect(out).toInclude("wheels packages add");
50+
});
51+
52+
it("does not advertise `install <name>` as a working verb", () => {
53+
mod.__arguments = ["help"];
54+
var out = mod.packages();
55+
// The historic help row "install <name> [--force] Install a package"
56+
// must not appear — it advertises a verb that LuCLI intercepts.
57+
expect(REFindNoCase("install[[:space:]]+<name>[[:space:]]+\[--force\][[:space:]]+Install a package", out)).toBe(0);
58+
});
59+
60+
it("explains that `install` is intercepted by LuCLI", () => {
61+
mod.__arguments = ["help"];
62+
var out = mod.packages();
63+
expect(out).toInclude("LuCLI");
64+
expect(REFindNoCase("intercept", out)).toBeGT(0);
65+
});
66+
67+
it("lists every canonical sub-verb", () => {
68+
mod.__arguments = ["help"];
69+
var out = mod.packages();
70+
expect(out).toInclude("list");
71+
expect(out).toInclude("search");
72+
expect(out).toInclude("show");
73+
expect(out).toInclude("add");
74+
expect(out).toInclude("update");
75+
expect(out).toInclude("remove");
76+
expect(out).toInclude("registry");
77+
});
78+
});
79+
}
80+
}

0 commit comments

Comments
 (0)