Skip to content

fix(cli): wheels packages help now documents add as canonical install verb#2729

Merged
bpamiri merged 6 commits into
developfrom
fix/bot-2713-wheels-packages-help-advertises-install-but-the-wo
May 16, 2026
Merged

fix(cli): wheels packages help now documents add as canonical install verb#2729
bpamiri merged 6 commits into
developfrom
fix/bot-2713-wheels-packages-help-advertises-install-but-the-wo

Conversation

@wheels-bot
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot commented May 15, 2026

Documents add as the canonical install verb in wheels packages --help / wheels packages help / wheels packages -h, and explains why typing install does not work. LuCLI's built-in extension installer intercepts the literal install verb across all modules before dispatch reaches Module.cfc::packages(), so the previously-advertised install <name> [--force] Install a package row pointed at a verb that could never run. Same trap that hit wheels browser install (renamed to wheels browser setup in #2345).

Fixes #2713

Summary

  • New $packagesHelp() method on Module.cfc emits a hand-written help string the module owns directly (rather than relying on auto-introspection that drifted from the real CLI surface).
  • wheels packages help (positional), wheels packages --help, and wheels packages -h all short-circuit to it before any network call — help is fast and offline.
  • The help text lists every canonical sub-verb (list, search, show, add, update, remove, registry refresh, registry info), documents add as the canonical install verb, and includes a Note paragraph explaining the LuCLI interception so users typing the old install form understand what is happening.

Files

  • cli/lucli/Module.cfc — short-circuit in packages(), new $packagesHelp(), --help / -h parsing in $packagesArgsToOptions / $packagesStripFlags.
  • cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc — failing-then-passing spec asserts help documents add, lists every sub-verb, mentions LuCLI interception, and does not advertise the dead install <name> [--force] Install a package row.
  • CHANGELOG.md — Fixed entry under [Unreleased].

Checklist

  • Tests (failing-then-passing spec in cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc)
  • Framework Docs
  • AI Reference Docs
  • CLAUDE.md
  • CHANGELOG.md
  • Test runner — local bash tools/test-cli-local.sh execution was blocked by sandbox permissions in this run, so the spec-then-impl flow is left for CI (bot-tdd-gate.yml verifies both spec and impl diffs exist; pr.yml runs the suite end-to-end).

…tall 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>
@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 15, 2026

Wheels Bot — No doc updates

Reviewed this PR's diff and found no docs that need updating (the guide pages under web/sites/guides/.../command-line-tools/commands/packages/ already document add as the canonical install verb, already include the LuCLI interception note in install.mdx, and CLAUDE.md's Package System section already lists wheels packages add <name> — the PR fixed the CLI help text to match the docs, not the other way around).

Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: The implementation is correct and well-reasoned — $packagesHelp() cleanly owns the help text, the short-circuit fires before any network call, and all three invocation forms (help, --help, -h) are wired up. However, the PR has a hard blocker: the TDD Gate CI check is already failing because the spec lives at cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc, which the gate's path regex does not cover. There is also a dead-code clause in the guard condition and the Signed-off-by identity does not match the required bot identity. The correctness and cross-engine story are sound; these process issues need fixing before merge.

Correctness

Dead-code guard clause — sub == "-h" at Module.cfc line 2065

if (opts.help ?: false || sub == "help" || sub == "-h") {

$packagesStripFlags unconditionally strips -h before this line runs (it adds its own if (a == "-h") { i++; continue; } guard in this same PR), so -h can never appear in positional and therefore sub can never equal "-h". The condition evaluates correctly because opts.help ?: false handles the -h case via $packagesArgsToOptions, but the third clause is misleading dead code. A future maintainer could read it as "positional -h is a supported form" and be confused when tracing the path.

Concrete fix: remove || sub == "-h" from the guard and add a comment that -h reaches $packagesHelp() via opts.help, set by $packagesArgsToOptions.

CI / TDD Gate (blocks merge)

The "Bot PR TDD Gate" required check is already failing:

##[error]Bot PRs must include a failing-then-passing spec under
         tests/specs/ or vendor/wheels/tests/specs/

The gate's path pattern is '^(tests/specs/|vendor/wheels/tests/specs/)'. The new spec is at:

cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc

This path matches neither prefix, so spec_changes is empty and the gate exits 1. The spec itself is correct and should remain at cli/lucli/tests/specs/ — that is the right home for CLI unit tests (all existing command specs live there). The fix needed is to the gate's pattern in .github/workflows/bot-tdd-gate.yml:

# current
grep -E '^(tests/specs/|vendor/wheels/tests/specs/)'

# fix — add the CLI test path
grep -E '^(tests/specs/|vendor/wheels/tests/specs/|cli/lucli/tests/specs/)'

That one-line change should be included in this PR or in a pre-merge fixup commit.

DCO / Commit Trailer Identity

The commit trailer is:

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

The required identity per _shared-rails.md is:

Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com>

The DCO GitHub App is a required status check. If it validates that the Signed-off-by identity matches the configured bot user, this will block merge. Add a fixup commit with the correct trailer.

Tests

The spec at cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc is structurally sound: it extends wheels.wheelstest.system.BaseSpec, uses BDD describe/it, scaffolds the module with TestHelper, and asserts the right things. Two minor gaps:

  1. No -h test. The spec exercises ["help"] and ["--help"] but not ["-h"] directly. Given that -h is explicitly mentioned in the PR description and the help text documents it, adding mod.__arguments = ["-h"]; expect(Len(mod.packages())).toBeGT(0); would close the loop.

  2. REFindNoCase return-value assertion is semantically correct but reads ambiguously. The form:

    expect(REFindNoCase("install[[:space:]]+<name>...", out)).toBe(0);

    relies on the reader knowing REFindNoCase returns 0 when not found. A clearer equivalent:

    expect(out).notToInclude("install <name> [--force]");

    This is a readability nit — the regex check correctly guards the historic row. No correctness issue.

Verdict

The implementation logic is clean and the fix is correct. Two items block merge:

  1. Fix the TDD gate path regex in bot-tdd-gate.yml to include cli/lucli/tests/specs/.
  2. Fix the Signed-off-by trailer to use the wheels-bot[bot] identity.

The dead-code guard clause and missing -h test case are worth addressing in this PR but are not merge-blockers.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 15, 2026

Wheels Bot — Reviewer B (round 1)

A correctly identified the hard blocker (TDD gate) and the dead-code nit, and the overall verdict of "request changes" is right. One finding is a false positive with wrong remediation guidance: the DCO concern. Convergence is appropriate on the real changes.

Sycophancy

None detected. A's praise ("implementation is correct and well-reasoned") is backed by the correctness analysis, not handed out freely.

False positives

DCO finding — partially wrong reasoning and wrong fix.

A claims the Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> trailer "will block merge" and recommends changing it to the wheels-bot[bot] identity. Both claims are incorrect:

  1. The DCO GitHub App validates that the Signed-off-by trailer matches the commit author, not a "configured bot user." The existing trailer does match the commit author (41898282+claude[bot]@users.noreply.github.com). If the DCO App is installed, it would pass, not block.

  2. Changing the trailer to Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> would introduce a trailer/author mismatch — the opposite of a DCO fix.

  3. Looking at gh pr checks 2729 output: there is no DCO check in the current required checks list at all.

The real concern is a policy violation — claude[bot] authored the commit rather than wheels-bot[bot] per _shared-rails.md. That is worth noting, but it does not manifest as a DCO App CI failure, and A's proposed remediation is wrong.

Missed issues

None material. A caught the TDD gate blocker (confirmed: Bot PR TDD Gate: fail in CI), the dead-code clause, and the missing -h test case. Coverage is solid.

One minor observation A did not raise: if the TDD gate regex is expanded to include cli/lucli/tests/specs/, the new spec file will also appear in impl_changes (since the grep -v exclusion list in bot-tdd-gate.yml does not exclude cli/lucli/tests/specs/). This means the spec is double-counted as both spec and impl. The gate still passes (both buckets are non-empty), so it is not a problem in practice, but a cleaner fix would also exclude cli/lucli/tests/specs/ from the impl_changes grep. Not a blocker.

Verdict alignment

A's COMMENTED verdict (neither APPROVE nor REQUEST_CHANGES at the API level) is consistent with the findings — two items it names as merge blockers are real (TDD gate failing) and meaningful (policy: wrong bot identity). Appropriate.

Convergence

Aligned. Changes are needed. The actionable fix set:

  1. bot-tdd-gate.yml — spec pattern (blocks merge): add cli/lucli/tests/specs/ to the spec regex, and ideally also exclude it from impl_changes:
    spec_changes=$(echo "$changed" | grep -E '^(tests/specs/|vendor/wheels/tests/specs/|cli/lucli/tests/specs/)' || true)
    impl_changes=$(echo "$changed" | grep -vE '^(tests/specs/|vendor/wheels/tests/specs/|cli/lucli/tests/specs/|\.ai/|CHANGELOG\.md|docs/|web/|\.github/)' || true)
  2. cli/lucli/Module.cfc — remove dead clause (nit): drop || sub == "-h" from the guard at the top of packages().
  3. PackagesCommandSpec.cfc — add -h test (minor gap): exercise ["-h"] directly to match the three documented invocation forms.

DCO / Signed-off-by should NOT be in the fix set: the existing trailer correctly matches the commit author; changing it to wheels-bot[bot] would create a mismatch.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 15, 2026

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

  • .github/workflows/bot-tdd-gate.yml — added cli/lucli/tests/specs/ to both the spec regex and the impl_changes exclusion list. Resolves the hard TDD-gate blocker A flagged: the spec at cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc previously matched neither prefix so spec_changes was empty and the gate exited 1. B also called out the double-counting risk on impl_changes; both edits land in this commit.
  • cli/lucli/Module.cfc:2065 — removed the dead || sub == "-h" clause from the help-short-circuit guard. $packagesStripFlags filters -h out of positional before sub is read, so the third clause could never fire. Expanded the comment above the guard to document that -h arrives via opts.help (set by $packagesArgsToOptions), never as a positional.
  • cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc — added a [\"-h\"] test case alongside the existing [\"help\"] and [\"--help\"] cases, matching the three documented invocation forms.

Intentionally skipped (not part of the consensus):

  • DCO / Signed-off-by trailer identity — per Reviewer B's round-1 analysis, the existing trailer matches the commit author (41898282+claude[bot]@users.noreply.github.com), which is exactly what the DCO App validates. Changing it to the wheels-bot[bot] identity would create an author/trailer mismatch — the opposite of a DCO fix. This commit also signs off as the configured git author for the same reason.

Test runner — bash tools/test-cli-local.sh is blocked by sandbox permissions in this address-review environment (same constraint that hit the initial propose-fix run on this PR). Leaving end-to-end validation to CI: bot-tdd-gate.yml should now pass on this SHA, and pr.yml will execute the spec suite.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on approve or the outer-loop cap (5 rounds) is reached.

@bpamiri bpamiri marked this pull request as ready for review May 15, 2026 19:47
…i 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
Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot -- Reviewer A

TL;DR: This is the fixup commit addressing all three round-1 blockers. The dead guard clause is removed with a data-flow comment; the TDD gate path regex is extended in both the spec-detection and impl-exclusion expressions; the missing -h test case is added. No new correctness, cross-engine, or security issues were introduced. Verdict: approve (submitted as comment because GitHub does not allow self-approval on bot-authored PRs).

Correctness

All round-1 blockers are resolved:

  1. Dead guard clause removed. cli/lucli/Module.cfc line 2070 is now:

    if ((opts.help ?: false) || sub == "help") {
    

    The previous clause || sub == "-h" was unreachable: packagesArgsToOptions consumes -h into opts.help, and packagesStripFlags removes it from positional before sub is read. The added comment at lines 2066-2069 documents that data flow for future maintainers.

  2. TDD gate path regex. Both the spec-detection grep (line 61) and the impl-exclusion grep (line 73) in .github/workflows/bot-tdd-gate.yml now include cli/lucli/tests/specs/. The PR spec file (PackagesCommandSpec.cfc) registers as a spec change; Module.cfc registers as implementation -- gate passes cleanly.

  3. Elvis precedence. Parenthesizing (opts.help ?: false) is correct and explicit. Without parens, ?: could bind to the full right side of || depending on CFML engine operator-precedence rules.

Tests

The -h test case (PackagesCommandSpec.cfc lines 37-45) asserts both that output is non-empty and that it contains "wheels packages add", confirming the short flag routes to the same hand-written help body.

Minor nit: expect(Len(out)).toBeGT(0) on line 38 is redundant -- the following toInclude assertion also fails on empty output. No correctness impact.

Commits

Commit 81caf1a is authored by Peter Amiri with no Signed-off-by trailer. If the DCO App validates human-authored commits it would need Signed-off-by: Peter Amiri peter@alurium.com. The commit body notes the DCO App is not in the required-checks list, consistent with Reviewer B's round-1 finding. Flagging for awareness; not a merge blocker.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 15, 2026

Wheels Bot — Reviewer B (round 1)

A's review of the fixup commit is accurate and well-grounded. All three round-1 blockers are verifiably resolved (CI confirms: Bot PR TDD Gate now passes), the verdict of approve is consistent with the findings, and A's two remaining observations (the redundant assertion nit and the DCO awareness flag) are correctly scoped. Aligned on approve.

Sycophancy

None detected. A's approval is earned — it is supported by line-specific verification of each resolved blocker, not free-handed.

False positives

None. Spot-checking A's specific claims against the diff:

  • Dead guard removed: cli/lucli/Module.cfc now reads if ((opts.help ?: false) || sub == "help") with no || sub == "-h" clause. Correct.
  • TDD gate regex: both the spec_changes grep (line 61) and the impl_changes exclusion (line 73) in bot-tdd-gate.yml now include cli/lucli/tests/specs/. Correct, and CI confirms the gate passes.
  • Elvis precedence: A notes that parenthesizing (opts.help ?: false) is explicit and avoids a potential precedence ambiguity. Technically the behavior is equivalent either way under CFML's documented ?: / || precedence, but the parens make intent unambiguous across engines (BoxLang precedence rules are less well-documented). A correctly calls it a clarification, not a correctness fix.
  • Redundant Len(out) > 0 nit: A flags the assertion on line 38 of the -h test as redundant because toInclude("wheels packages add") already fails on empty output. Correct. A also correctly limits this to the -h case — the help and --help test cases use Len(out) > 0 as their only assertion, where it is not redundant.
  • DCO concern: commit 81caf1a09 is authored by Peter Amiri with no Signed-off-by trailer. No DCO App check appears in the required-checks list for this PR (verified via gh pr checks 2729). A flags it for awareness without calling it a merge blocker. Accurate.

Missed issues

None material. The diff touches only bot-tdd-gate.yml, cli/lucli/Module.cfc, PackagesCommandSpec.cfc, and CHANGELOG.md. No security-sensitive paths, no migrations, no DI wiring, no cross-engine CFML surface (the CLI module runs on the JVM, not the CFML engine). The scope is narrow and A's coverage is appropriate.

Verdict alignment

A's COMMENTED/approve verdict is consistent with the findings: blockers are gone, CI is green, and the two remaining observations are correctly classified as nits rather than blockers.

Convergence

Aligned. A's verdict (approve) is the right call on this SHA. No changes are needed.

…s-packages-help-advertises-install-but-the-wo

# Conflicts:
#	CHANGELOG.md
Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This is the third pass on PR #2729 (head SHA 88654fa60). The merge commit brings in develop cleanly; the substantive changes from commits 33321057f (initial implementation) and 81caf1a09 (fixup: dead guard clause removed, TDD gate extended, -h test added) are all addressed and correct. Round-1 and round-2 blockers are resolved. No correctness, cross-engine, or security issues in the new state. Two minor nits remain — one test redundancy and the DCO trailer situation on the human-authored commit — neither blocks merge.

Correctness

Implementation logic is clean. The data flow through $packagesArgsToOptions -> $packagesStripFlags -> packages() is consistent:

  • --help and -h both set opts.help = true in $packagesArgsToOptions (cli/lucli/Module.cfc line 2189).
  • $packagesStripFlags correctly excludes --help from the boolean-skip branch (it is in the booleans list at line 2207) and explicitly skips -h via its own guard (lines 2213-2215).
  • The guard if ((opts.help ?: false) || sub == "help") at line 2070 fires before the switch and before any network call. Elvis precedence is correctly parenthesised.
  • $packagesHelp() is a clean string accumulator with no branching or external dependencies — always fast and offline.

The install case retained in the switch (lines 2103-2115) as a documentation marker with a friendly redirect is a good defensive measure for the day LuCLI stops intercepting the verb.

Tests

cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc — one redundancy nit.

Line 40: expect(Len(out)).toBeGT(0) in the -h test is unreachable as a distinct failure mode because line 43 (expect(out).toInclude("wheels packages add")) already fails on empty output. Not wrong; just noise.

The REFindNoCase("install[[:space:]]+<name>...", out).toBe(0) negative assertion at line 57 is correct — POSIX [[:space:]] is supported by both Lucee and Adobe CF (confirmed by existing usage in vendor/wheels/Model.cfc:30 and vendor/wheels/tests/specs/cli/LinuxPackageStagingSpec.cfc:82). The assertion properly guards against the old row being re-added.

TDD Gate (.github/workflows/bot-tdd-gate.yml)

Both the spec-detection pattern (line 61) and the impl-exclusion pattern (line 73) now include cli/lucli/tests/specs/. With this PR's files:

  • cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc registers as spec
  • cli/lucli/Module.cfc registers as implementation
  • .github/workflows/bot-tdd-gate.yml and CHANGELOG.md are excluded from both buckets

Gate passes cleanly.

Commits

Three commits in this PR:

  1. 33321057ffix(cli): wheels packages help now documents add as canonical install verb — valid conventional commit. Has Signed-off-by: claude[bot]. Identity mismatch vs. required wheels-bot[bot] was flagged in round 1; round 2 confirmed DCO is not in the required-checks list — not a merge blocker.

  2. 81caf1a09fix(cli): drop dead -h guard clause and unblock TDD gate for cli/lucli specs — valid conventional commit. No Signed-off-by trailer for the human author (bpamiri). Same DCO note — not a required check.

  3. 88654fa60 — Merge commit incorporating origin/develop. Standard merge commit format; no conventional-commit requirement.

Docs

CHANGELOG [Unreleased] entry is present and accurate. The PR checklist correctly leaves Framework Docs, AI Reference Docs, and CLAUDE.md unchecked — this is a pure CLI/help-text fix with no framework API or convention changes.


No blocking issues. The nits above (redundant Len check, DCO trailers) are carry-overs already acknowledged in prior rounds and below the merge-threshold. Change is ready.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 15, 2026

Wheels Bot — Reviewer B (round 1)

A's review of the post-merge SHA is accurate and concise. The prior-round convergence (approve on 81caf1a09) holds: the head SHA is a clean merge commit from develop with no new implementation delta, and A's re-verification of the three original blockers is correct. Aligned on approve.

Sycophancy

None detected. A's "change is ready" conclusion is backed by line-specific verification of each resolved blocker, not handed out freely.

False positives

None. Spot-checking A's specific claims:

  • install case at lines 2103–2115: Verified. The case exists at line 2099 in the current file with the documented-marker comment A describes. A's characterization is accurate.
  • POSIX [[:space:]] support: A cites vendor/wheels/Model.cfc:30 and vendor/wheels/tests/specs/cli/LinuxPackageStagingSpec.cfc:82 as existing passing usages. Both files do contain [[:space:]] patterns (confirmed via grep), and those specs pass in CI — sufficient empirical backing for A's claim.
  • Dead guard removal: The guard now reads if ((opts.help ?: false) || sub == "help") with no || sub == "-h" clause, matching the diff.
  • TDD gate patterns: Both the spec-detection line and the impl-exclusion line in bot-tdd-gate.yml include cli/lucli/tests/specs/. Correct.

Missed issues

None material. A notes the only two carry-over nits (redundant Len assertion in the -h test, DCO trailer on the human-authored commit) and correctly scopes both as below the merge threshold. The diff touches only bot-tdd-gate.yml, Module.cfc, PackagesCommandSpec.cfc, and CHANGELOG.md — no security-sensitive paths, no migrations, no cross-engine CFML surface in scope.

Verdict alignment

A's COMMENTED/ready verdict is consistent with the findings: no blockers remain, CI is green, and the outstanding observations are acknowledged nits from prior rounds.

Convergence

Aligned. A's assessment (approve, no blocking issues) is the right call on this SHA. No changes are needed.

bpamiri added 2 commits May 15, 2026 18:47
…s-packages-help-advertises-install-but-the-wo

# Conflicts:
#	CHANGELOG.md
…s-packages-help-advertises-install-but-the-wo

# Conflicts:
#	CHANGELOG.md
Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This is a fourth pass triggered by the merge-to-develop commit (48e534997). The PR's own substantive changes — $packagesHelp(), the short-circuit guard, the TDD gate path extension, and the spec — are all intact and unchanged from the last reviewed SHA (88654fa60). The merge introduced one real diff: a CHANGELOG.md conflict between the PR entry and several develop entries (#2712, #2714, #2717, CORS fixes). The conflict was resolved correctly — all entries from both sides are present and no PR entry was dropped. No new correctness, cross-engine, or security issues. Verdict: comment (carry-over nits only; nothing blocks merge).

Correctness

Merge conflict resolution is correct.

The CHANGELOG.md conflict (noted in the merge commit message) was resolved by preserving the PR's #2713 Fixed entry and inserting the develop-side entries (#2714, #2717, CORS #2707/#2704, and #2712 Added) around it. No entry was duplicated or dropped. Verified by diffing 88654fa60..48e534997 -- CHANGELOG.md.

PR implementation files are intact.

The $packagesHelp() function, the short-circuit guard if ((opts.help ?: false) || sub == "help"), and the $packagesArgsToOptions/$packagesStripFlags additions are all present in the merged state at their original line numbers. The .github/workflows/bot-tdd-gate.yml path additions for cli/lucli/tests/specs/ are also intact.

Develop-side changes in the merge commit are unrelated to this PR.

cli/lucli/Module.cfc received one additional hunk (the paginationLinks() upgrade-check grep from #2714), and vendor/wheels/PackageLoader.cfc received the CFML-mapping feature from #2732. Neither touches the packages help path.

Tests

No new test changes. The PackagesCommandSpec.cfc spec is intact. Carry-over nit from round 3 remains:

  • cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc line 38: expect(Len(out)).toBeGT(0) in the -h test is redundant — the following toInclude assertion already fails on empty output. Minor noise; no correctness impact.

Commits

New commits since last reviewed SHA:

  • a575c521bfix(middleware): emit Vary: Origin from Cors when reflecting allowed origins (#2724) — valid, from develop.
  • f9a4312b6fix(middleware): Cors no longer emits raw comma list as Access-Control-Allow-Origin (#2725) — valid, from develop.
  • ce47c8024fix(view): paginationNav rejects unknown sub-helper arguments (#2726) — valid, from develop.
  • 221612c29 — merge commit.
  • 151feb319feat(config): auto-register per-package CFML mapping in PackageLoader (#2732) — valid, from develop.
  • 48e534997 — merge commit (conflict in CHANGELOG.md, resolved correctly).

All conventional-commit subjects are within 100 chars and conform to the commitlint.config.js allowlist. The DCO situation on the human-authored 81caf1a09 (Peter Amiri, no Signed-off-by) is unchanged from round 2 — DCO is not a required status check on this repo, so it does not block merge.


No blocking issues. The merge is clean and the PR is ready for a human approving review.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 16, 2026

Wheels Bot — Reviewer B (round 1)

A's fourth-pass review is accurate and appropriately scoped. Prior convergence (approve on 88654fa60) holds: the merge commit brought only a CHANGELOG.md conflict resolution and unrelated develop-side changes, and A correctly limits the review to verifying those did not disturb the PR's substantive implementation. Aligned on approve.

Sycophancy

None detected. A's conclusion ("no blocking issues, ready for human approving review") is backed by line-specific verification of the CHANGELOG conflict resolution and implementation file integrity — not handed out freely.

False positives

None. Spot-checking A's specific claims against the diff:

  • CHANGELOG conflict resolution: the diff shows the #2713 Fixed entry inserted at the top of the Fixed section; develop-side entries (#2714, #2717, CORS, #2732) were already in the base and appear intact. Accurate.
  • Implementation files intact: $packagesHelp(), the short-circuit guard if ((opts.help ?: false) || sub == "help"), and the TDD-gate path additions are all present at expected locations in the diff. Correct.
  • Develop-side changes unrelated: the paginationLinks() hunk in Module.cfc and the PackageLoader.cfc CFML-mapping feature are from separate PRs with no overlap with the packages help path. Accurate.
  • Commit conventional-commit compliance: all six commits A lists conform to commitlint.config.js. No violations.

Missed issues

None material. A correctly scopes this pass to what the merge commit changed. The two carry-over nits from prior rounds — the redundant Len(out) > 0 assertion in the -h test and the DCO trailer on the human-authored 81caf1a09 commit — remain unchanged, and A acknowledges both at their prior-round severity (nits, not blockers).

Verdict alignment

A's COMMENTED verdict (nothing blocks merge) is consistent with the findings. The PR's blockers were resolved two SHAs ago; this pass confirms the merge did not reintroduce them.

Convergence

Aligned. A's assessment matches B's prior convergence on 88654fa60. The merge is clean, CI is green, and no new issues were introduced. Approve.

…s-packages-help-advertises-install-but-the-wo

# Conflicts:
#	CHANGELOG.md
Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: Fifth pass on PR #2729 (head SHA 41c981a9a). The new merge commit pulls in #2734 (anatomy-of-a-package blog post + manifest doc fixes: dependencies to requires / replaces / suggests in CLAUDE.md and both guide snapshots). The PR own implementation -- $packagesHelp(), the short-circuit guard, the TDD gate path extension, and the spec -- is unchanged from the last reviewed SHA (48e534997). CHANGELOG conflict resolved correctly, all round-1/2 blockers long since closed. One minor carry-over nit remains. Verdict: comment (nothing blocks merge).

Correctness

Merge conflict resolution is correct. The CHANGELOG.md conflict between the PR #2713 Fixed entry and the #2734 Fixed entry from develop was resolved by preserving both entries. No entry was dropped or duplicated. Confirmed via git show 41c981a9a -- CHANGELOG.md.

#2734 content is correct on its own terms. CLAUDE.md now shows requires, replaces, and suggests in the example manifest (replacing the legacy dependencies key), and adds a prose paragraph describing the three fields. This matches the PackageLoader / ModuleGraph.cfc behaviour described in existing CLAUDE.md text. No inconsistency introduced.

PR implementation is intact. The $packagesHelp() accumulator, the if ((opts.help ?: false) || sub == "help") short-circuit at cli/lucli/Module.cfc line 2070, the --help / -h parsing additions to $packagesArgsToOptions and $packagesStripFlags, and the bot-tdd-gate.yml path extensions for cli/lucli/tests/specs/ are all unchanged from the round-3 reviewed SHA (88654fa60).

Tests

One carry-over nit -- redundant Len check in the -h test. cli/lucli/tests/specs/commands/PackagesCommandSpec.cfc line 40: expect(Len(out)).toBeGT(0) is unreachable as a distinct failure mode -- an empty out will fail on the toInclude at line 43 first. Minor noise, no correctness impact. Carried over from round 3.

Commits

New commit since last reviewed SHA: 41c981a9a -- merge commit (origin/develop into PR branch), conflict in CHANGELOG.md resolved correctly. All earlier commits (33321057f, 81caf1a09) were reviewed in rounds 1 and 2. DCO trailer situation on 81caf1a09 (human-authored, no Signed-off-by) is unchanged and confirmed non-blocking (DCO App is not a required status check on this repo).


No blocking issues. The PR is clean and ready for a human approving review on develop.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 16, 2026

Wheels Bot — Reviewer B (round 1)

A's fifth-pass review is accurate and concise. Prior convergence (approve on `48e534997`) holds: the head SHA `41c981a9a` is a merge commit from develop carrying only a CHANGELOG conflict resolution and unrelated `#2734` develop-side changes, with no new implementation delta. A's verification of each resolved blocker and the carry-over nit is correct. Aligned on approve.

Sycophancy

None detected. A's "nothing blocks merge" conclusion is backed by line-specific verification of each claim, not handed out freely.

False positives

None. Spot-checking A's specific claims against the diff:

  • Short-circuit guard: cli/lucli/Module.cfc reads if ((opts.help ?: false) || sub == "help") with no || sub == "-h" clause. Confirmed in diff.
  • CHANGELOG conflict resolution: the #2713 Fixed entry was inserted at the top of the Fixed section; the #2734, #2714, and #2717 entries from develop appear as context lines (unchanged). No entry dropped or duplicated. Correct.
  • TDD gate regex: both the spec_changes grep and the impl_changes exclusion in bot-tdd-gate.yml include cli/lucli/tests/specs/. Confirmed in diff.
  • Implementation intact: $packagesHelp(), the short-circuit guard, and the arg-parsing additions are all present in the diff, unchanged from prior reviewed SHAs. Correct.
  • Redundant Len(out) > 0 nit: in the -h test, expect(Len(out)).toBeGT(0) is indeed redundant — an empty out would fail toInclude("wheels packages add") first. A correctly limits the nit to the -h case; the help and --help tests use Len(out) > 0 as their only assertion, where it is not redundant. Correct.

Missed issues

None material. The diff touches only bot-tdd-gate.yml, CHANGELOG.md, cli/lucli/Module.cfc, and PackagesCommandSpec.cfc. No security-sensitive paths, no migrations, no DI wiring, no cross-engine CFML surface in scope (the CLI module runs on the JVM). Coverage is appropriate for the scope of this PR.

Verdict alignment

A's COMMENTED verdict (nothing blocks merge, ready for human approving review) is consistent with the findings: all round-1 blockers are long resolved, CI is green, and the outstanding nit is correctly scoped as non-blocking.

Convergence

Aligned. A's verdict (approve, no blocking issues) is the right call on this SHA. No changes are needed.

@bpamiri bpamiri merged commit 06e490c into develop May 16, 2026
7 checks passed
@bpamiri bpamiri deleted the fix/bot-2713-wheels-packages-help-advertises-install-but-the-wo branch May 16, 2026 03:34
bpamiri added a commit that referenced this pull request May 22, 2026
…spatch (#2786)

* fix(cli): make `wheels packages install` a real alias for `add` in dispatch

The `case "install":` branch in `Module.cfc::packages()` previously printed
a warning to stdout and returned an empty string instead of installing
anything. That was wrong for every caller path that actually reaches
module dispatch — the stdio MCP server, scripted in-process clients, and
the spec suite — because `PackagesMainCli.install()` itself has been a
transparent alias for `add()` since #2729. The dispatch layer was the
only place where the alias broke.

The shell-facing `wheels packages install <name>` is still intercepted
by LuCLI's built-in extension installer upstream of module dispatch and
remains broken on that path (documented in the module-owned `--help`
text). This change only fixes the paths that LuCLI does NOT intercept.

Both verbs now share a single fall-through case body so validation,
error shape, and install behavior cannot drift apart again.

Fixes #2785

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* docs(web/guides): clarify install-as-alias behavior in packages CLI section

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* fix(cli): address Reviewer A/B consensus findings (round 1)

- PackagesCommandSpec: add `expect(installResult.type).toBe(addResult.type)`
  after the existing `.notToBe("")` assertion so the equivalence claim in
  the surrounding comment is actually enforced. A regression where
  `install` throws at argument validation (before the registry call)
  would have satisfied `.notToBe("")` but diverged from `add`'s shape;
  the new assertion pins it.
- CHANGELOG: terminal period on the new `[Unreleased] / ### Fixed`
  entry for consistency with surrounding entries.

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* fix(web/guides): address Reviewer A/B consensus findings (round 2)

- web/sites/guides/src/content/docs/v4-0-0/digging-deeper/packages.mdx
  (line 320) — scope the install-as-alias note to v4.0.1+. The previous
  wording asserted the alias was transparent on MCP / in-process paths,
  but that's only true after this PR (which targets v4.0.1). On v4.0.0
  itself, MCP also no-ops; the versioned v4.0.0 docs now say so
  explicitly and point readers to the v4.0.1 snapshot for the alias
  behavior. The v4-0-1-snapshot/ copy was already correct and is
  untouched.

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

---------

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wheels packages --help advertises 'install' but the working verb is 'add' — help text drift

1 participant