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

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

Copy link
Copy Markdown
Contributor

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

wheels-bot Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

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).

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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

wheels-bot Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

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

wheels-bot Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

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

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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

wheels-bot Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

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

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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

wheels-bot Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

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

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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

wheels-bot Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

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

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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

wheels-bot Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

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>
bpamiri added a commit that referenced this pull request May 27, 2026
* chore: bump develop snapshot target to 4.0.2 (#2770)

Manual bump after the v4.0.1 GA — `bump-develop-version.yml` fired via
`repository_dispatch` (the #2609 fix worked) but failed in 12s on a
second issue: `peter-evans/create-pull-request@v6` hit
`remote: Duplicate header: "Authorization"` because the `actions/checkout`
step left credentials persisted that conflict with the action's own token.
See run 26173817714 for the failed log.

Setting `wheels.json` to `4.0.2` so subsequent develop snapshots are
tagged `4.0.2-snapshot.<run>`. This is a baseline, not a commitment —
the next GA's scope decision is made at tag-cut time.

Follow-up issue tracks the workflow fix (add `persist-credentials: false`
to the checkout step).

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(release): fix bump-develop-version.yml duplicate Authorization header (#2771)

`actions/checkout@v6` defaults to `persist-credentials: true`, which
writes `http.https://github.com/.extraheader = AUTHORIZATION: basic <GITHUB_TOKEN>`
to the local `.git/config`. `peter-evans/create-pull-request@v6` then
sets its own `extraheader` for the dispatch token, and the next git
operation sends both Authorization headers — GitHub returns HTTP 400
with `remote: Duplicate header: "Authorization"`.

First observed on the v4.0.1 GA (2026-05-20, run 26173817714); manual
workaround was #2770. Setting `persist-credentials: false` keeps
peter-evans/create-pull-request as the sole Authorization authority.

This is a documented peter-evans/create-pull-request gotcha when the
caller uses a non-default token.

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(web/blog): Wheels 4.0.1: Adobe CF hardening, Windows Scoop fixes, and the post-GA shakeout (#2772)

Walks through the ~100 PRs that landed between 4.0.0 and 4.0.1: Adobe CF
2023/2025 attributeCollection + onError + env() + Vite asset-walk chain,
the Windows Scoop wheels.cmd cmd.exe pre-parser fix, paginationNav()
viewStyle presets, whereIn([]) short-circuit, CORS preflight/Vary/multi-
origin fixes, plural mappings, Oracle bulk-insert + CockroachDB advisory
locks, BoxLang adapter fixes, deploy CLI hardening, and Rocky Linux RPM
fixes from the titan production cutover.

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(cli): guard application.wo in onError so init failures don't cascade (#2774)

* fix(cli): guard application.wo in onError so init failures don't cascade

When the Wheels Injector fails to load during onApplicationStart (a stale
/wheels mapping under Lucee Express 7 is the symptom users hit on the
"Your First 15 Minutes" tutorial), application.wo is never assigned. The
existing recovery try/catch inside onError swallows a second failure
silently and then unconditionally calls application.wo.$getRequestTimeout(),
which throws "The key [WO] does not exist." and replaces the real
diagnostic with a cryptic cascade.

Add a StructKeyExists(application, "wo") guard right after the recovery
try/catch in cli/lucli/templates/app/public/Application.cfc (the template
behind `wheels new`) and the demo public/Application.cfc. When the global
isn't there, render a minimal HTML error page and return — the user sees
"Wheels failed to initialize" plus the original exception message instead
of the cascade.

Fixes #2773

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

* docs(web/guides): note application error fallback and init failure in troubleshooting docs

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

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

- Set HTTP 500 status code in the onError fallback in both
  cli/lucli/templates/app/public/Application.cfc and public/Application.cfc
  so monitoring tools and CDNs don't cache the Wheels-init failure as
  a successful response. Uses a plain struct for cfheader's
  attributeCollection per CLAUDE.md cross-engine invariant #10
  (Adobe CF 2023/2025 reject the arguments scope on built-in tags).
- Document the no-nested-braces assumption behind catchClosePattern in
  vendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc so a future
  edit that adds nested braces inside the outer catch knows why the
  silent fallback to scanFrom=1 is the safety net.
- Fix the contradictory recovery steps in the first-15-minutes guide
  (wheels reload requires a running server) at
  web/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx.
- Replace the speculative "pre-4.0.2" wording in
  .ai/wheels/troubleshooting/common-errors.md with "4.0.1 or earlier"
  since the fix is still in [Unreleased].

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

* chore(web): refresh visual baseline(s) (all)

Manually triggered baseline refresh via
.github/workflows/refresh-visual-baselines.yml
on branch fix/bot-2773-first-15-minutes-tutorial-fails-the-key-wo-does-no.

Run when an intentional content/layout change makes the visual-regression
check fail. The new PNG(s) under web/tests/visual-baselines/ are now the
expected rendering; re-run the failing visual-regression job to flip the
check green.

---------

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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix(ci): stop double-nesting framework inside Linux .deb/.rpm packages (#2776)

* fix(ci): stop double-nesting framework inside Linux .deb/.rpm packages

The nfpm contents rule pointed `src` at `./build/framework/` for the
framework staging step. `wheels-core-VER.zip` carries a top-level
`wheels/` directory that `unzip` preserves, so the resulting tree was
`./build/framework/wheels/...`. nfpm `type: tree` copies *contents* of
src into dst, which meant the inner `wheels/` wrapper itself landed at
the destination — producing
`/opt/wheels/module/vendor/wheels/wheels/Injector.cfc` instead of
`/opt/wheels/module/vendor/wheels/Injector.cfc`.

After the user-side wrapper sync (`/opt/wheels/module/*` →
`~/.wheels/modules/wheels/*`) and `wheels new <app>` copy, every fresh
Linux install ended up with the framework one directory level too deep.
Lucee's `/wheels` mapping pointed at the (empty) outer directory, so
`new wheels.Injector("wheels.Bindings")` in the generated
`public/Application.cfc` threw `could not find component or class with
name [wheels.Injector]` on the first request. The existing onError
handler then dereferenced `application.wo` (which was never assigned
because Injector init failed), surfacing only the cryptic cascade
`The key [WO] does not exist.` — issue #2773.

The brew formula handles this correctly by re-introducing the wheels/
wrapper at stage time
(`(share/"wheels/framework/wheels").install Dir["*"]`). Both Linux nfpm
configs now pin `src` at `./build/framework/wheels/` so the contents
flatten into `/opt/wheels/module/vendor/wheels/` as intended.

The published 4.0.1 .deb / .rpm artifacts ship the broken layout (1
.deb download, 0 .rpm at time of fix). A re-released 4.0.2 will be
needed to deliver the fix to users — the change here is to the build
config only, not to any framework or CLI code.

Tests: `vendor/wheels/tests/specs/cli/LinuxPackageStagingSpec.cfc`
gains a per-channel `it()` that asserts `src: ./build/framework/wheels/`
+ `dst: /opt/wheels/module/vendor/wheels/` are paired in each nfpm
yaml. Structural assertion follows the existing #2700 pattern (the
file already pins four other packaging invariants the same way).

Note on local verification: the structural spec was sanity-checked via
equivalent grep / perl POSIX patterns over the YAMLs (positive match
for the fixed form, zero matches for the buggy form). Running the
spec through the CFML runner locally was blocked by a port-8081
collision with two stale wheels server processes from prior dev
sessions — CI compat-matrix will run the spec across every engine ×
DB on this PR.

Closes #2773

Signed-off-by: Peter Amiri <peter@alurium.com>

* test(ci): add negative guard for buggy framework src in nfpm yamls

Reviewer A on PR #2776 (wheels-bot) flagged that the new framework-src
spec only asserted the *fixed* form was present, without a matching
`toBeFalse` for the buggy `./build/framework/` form. The file's existing
wrapper-routing checks (lines 60-68 / 81-106) already use a dual-
assertion pattern; the new spec was a one-sided outlier.

Add the negative guard: if a future copy-paste leaves both the bare
`src: ./build/framework/` and the fixed `src: ./build/framework/wheels/`
in the same yaml, nfpm would stage both — the bare one reintroduces
the double-nesting and breaks every fresh Linux install. The spec now
fails loudly in that scenario instead of silently passing on the
presence of the fixed entry.

The two regexes are mutually exclusive by construction: the positive
matches `framework/wheels/` followed by whitespace + `dst:`; the
negative matches `framework/` followed *immediately* by whitespace +
`dst:`. Since `wheels` isn't whitespace, `[[:space:]]+` can't bridge
across it, so the negative regex cannot false-positive on the fixed
form. Confirmed via perl POSIX equivalent against both nfpm yamls
plus a synthetic buggy fixture.

Also adds an inline comment to the positive assertion documenting why
`[[:space:]]+` works across the YAML line break (POSIX `[[:space:]]`
resolves to Java's `\s` in both Lucee and Adobe CF, which includes
`\n`) — addresses Reviewer A's Nit 2 observation that the cross-line
match hadn't been locally verified.

Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(web/guides): correct Linux bleeding-edge install URLs to wheels-be_* (#2777)

* docs(web/guides): correct Linux bleeding-edge install URLs to wheels-be_*

PR #2759 (2026-05-18) renamed the snapshot Linux artifacts from `wheels_*`
to `wheels-be_*` (debs) and `wheels-be-*.x86_64.rpm` (rpms) so the package
name itself differentiates the channel. The install guides were not
updated alongside that rename, so every documented `curl -fsSLO ...`
command for Linux bleeding-edge install resolves to a 404 against the
actual snapshot release assets.

Verified against v4.0.2-snapshot.1923 (published 2026-05-20):

  Guide says:    .../wheels_4.0.2.snapshot.1923_amd64.deb   → 404
  Actual asset:  .../wheels-be_4.0.2.snapshot.1923_amd64.deb

Fix all six pages where the snippets / prose examples appear (three
unique pages mirrored across v4-0-0 and v4-0-1-snapshot doc versions):

  start-here/installing.mdx                 — "Want bleeding-edge?" aside
  start-here/release-channels.mdx           — main BE install snippets +
                                              "Switching channels" snippets
                                              + tilde-mangling prose
  command-line-tools/installation.mdx       — bleeding-edge install snippets

The substitutions are scoped to bleeding-edge contexts (snippets using
`${SNAP_FILENAME_VER}` and prose `wheels_4.0.0.snapshot.*` filename
examples). Stable-channel snippets, which use `${WHEELS_VERSION}` and
fetch from `wheels-dev/wheels` (not `wheels-snapshots`), are unchanged —
they correctly retain the bare `wheels_` / `wheels-` prefixes because
the stable package name on Linux is still just `wheels`.

Without this fix, users cannot install or test bleeding-edge / develop
snapshots on Linux via the documented flow. This blocks user-side
verification of develop-only fixes before they ship in the next stable
patch — including PR #2776 (Linux .deb framework nesting fix) and
PR #2774 (defensive onError guard), both of which close issue #2773.

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(web/guides): fix release-channels.mdx — missed BE Tab URLs + Linux switching semantics

Round-1 reviewer findings on PR #2777:

A's Nit 1 — primary install Tabs at lines 104-105 (Debian/Ubuntu BE)
and 112 (Fedora/RHEL BE) of `release-channels.mdx` still resolved to
404. My initial verification sweep grep'd for `${SNAP_FILENAME_VER}`,
but these snippets bind the tag to `${WHEELS_FILENAME_VER}` (a
different bash var name). The fix is the same — point at the
`wheels-be_` / `wheels-be-` artifacts.

A's Nit 2 + B's catch — the "Switching channels" section had three
related staleness bugs after #2759 renamed the BE package:

  1. Line 129 prose claimed "only a single package name (`wheels`)
     is published per channel today" — false post-rename.

  2. Lines 142-143 inline comment ("upgrades in place — no uninstall
     step needed") was true when both channels shared the `wheels`
     name, but the new world depends on the actual nfpm-declared
     `Replaces:` / `Conflicts:` metadata. B caught the contradiction
     between A's proposed line-129 prose and the existing line-142
     comment.

  3. Lines 158-172 (Linux BE → stable, both Debian and Fedora) had
     the *same* conceptual bug as 142-143: they prescribed
     `--allow-downgrades` (apt) / `dnf downgrade`, both of which
     assume same-package-name version transitions. With different
     names, both would fail with a `/usr/bin/wheels` file conflict
     because the stable `wheels` package doesn't declare
     `Replaces:`/`Obsoletes: wheels-be`. Reviewers didn't explicitly
     flag this set, but it's the same root cause and listing them
     inconsistently would have left readers worse off.

Verified the actual nfpm metadata before rewriting (so the prose
matches what the packages really declare):

  wheels-be deb:  Replaces: wheels  +  Conflicts: wheels
  wheels-be rpm:  Conflicts: wheels  (no Obsoletes)
  wheels    deb:  no Replaces/Conflicts against wheels-be
  wheels    rpm:  no Conflicts/Obsoletes against wheels-be

The new prose at line 129 explains the asymmetry up front; each
snippet now carries a short comment naming the specific metadata
that drives its action (or the lack of metadata that requires the
explicit `apt remove` / `dnf remove`).

Stable-channel snippets and stable install Tabs are unchanged.

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(web/guides): name the actual Conflicts declaration in BE→stable comments

Reviewer A round-2 nit on PR #2777: the BE → stable (Debian) snippet's
comment said apt "would fail with a /usr/bin/wheels file conflict",
framing the failure mode as a dpkg-level file-ownership conflict. The
actual blocker is the package-level `Conflicts: wheels` declaration in
wheels-be's deb metadata — apt refuses the install with a package
conflict error before dpkg ever attempts to unpack files. An advanced
user debugging the actual error message would be confused by the
file-conflict framing.

Rewrite the Debian comment per A's suggestion, naming the actual
mechanism: `wheels-be declares Conflicts: wheels`. Kept the secondary
note about the missing `Replaces: wheels-be` in stable since it
explains why apt also wouldn't auto-remove (relevant context if a
reader wonders whether a single command could swap them).

Updated the Fedora BE → stable comment to use parallel framing for
consistency — same root cause (`wheels-be` declares `Conflicts:
wheels`, applies bidirectionally on rpm too). Reviewer A only flagged
the Debian site explicitly, but leaving the two comments inconsistent
would have invited the same "two sites must agree" finding that
caught round 1's line-142 / line-129 contradiction.

Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(test): BrowserTest reports unwired this.browser with browserDescribe() hint (#2782)

* fix(test): BrowserTest reports unwired this.browser with browserDescribe() hint

Plain describe() blocks inside BrowserTest subclasses left this.browser
as an empty string, so the first DSL call surfaced as
"function [visitUrl] does not exist in the String" — a misleading error
that hits every newcomer on iteration 1. Install an UnwiredBrowserGuard
sentinel at this.browser before browserDescribe() wires a real
BrowserClient (and after $endBrowserContext tears it down) so any method
call throws Wheels.BrowserTest.NotWired with a message naming
browserDescribe() as the fix.

Fixes #2778

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

* docs(web/guides): note Wheels.BrowserTest.NotWired when describe() used instead of browserDescribe()

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

---------

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

* chore: rename Phase 2 bucket repos to apt-wheels / yum-wheels for naming consistency (#2788)

Phase 2 Linux native-repo templates and dispatch wiring referenced
`wheels-dev/apt-wheels-dev` and `wheels-dev/yum-wheels-dev`. The `-dev`
suffix appeared to mirror the DNS form (`apt.wheels.dev`
→ `apt-wheels-dev`) but reads as a redundant org echo inside the
`wheels-dev` org and breaks the established `<package-manager>-wheels`
naming used by the other sister repos (`homebrew-wheels`,
`scoop-wheels`, `chocolatey-wheels`).

Rename everywhere to drop the `-dev` suffix:

- wheels-dev/apt-wheels-dev → wheels-dev/apt-wheels
- wheels-dev/yum-wheels-dev → wheels-dev/yum-wheels

The actual bucket repos were just created under the new names today
(2026-05-21 ~20:15 UTC), so this PR brings the templates / docs /
release-workflow dispatch in sync with the on-GitHub reality before
the first end-to-end dispatch fires. No live infrastructure references
the old names yet — Cloudflare Pages, DNS, and the bucket-side CI
secrets all post-date this rename.

Mechanical substitution across 9 files (27 references). Stable `wheels`
package name (the bare `wheels` in nfpm configs and `apt install
wheels` snippets) is untouched — only the org-namespaced repo names
change.

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(test): resolve BrowserTest base URL through layered lookup at instance time (#2783)

* fix(test): resolve BrowserTest base URL through layered lookup at instance time

Specs running against a non-default port (Titan on 60050, scaffolds on
60080) previously had to compare getBaseUrl() against a sentinel string
and override it manually because $resolveBaseUrl() returned
http://localhost:8080 unconditionally and the only escape hatch
(WHEELS_BROWSER_TEST_BASE_URL) is cached by the JVM at process start.

$resolveBaseUrl() now consults, in order: this.baseUrl per-spec override,
get("browserTestBaseUrl") Wheels setting, -Dwheels.browserTest.baseUrl
JVM property, WHEELS_BROWSER_TEST_BASE_URL env, $detectBaseUrlFromCgi()
auto-detect (the test runner reaches the suite over HTTP, so cgi already
names the right host:port), then the localhost:8080 default. The CGI
auto-detect skips when port==8080 so existing default-port runs are
unchanged.

Fixes #2779

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

* docs(web/guides): document BrowserTest layered base-URL resolution (#2779)

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

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

- Strip canonical ports (http:80, https:443) in $detectBaseUrlFromCgi to
  match URL conventions; update the https:443 spec expectation and add a
  dedicated http:80 case (vendor/wheels/wheelstest/BrowserTest.cfc:297-299,
  vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc:51-61).
- Document why the "falls back through layers" assertion is intentionally
  weak — JVM env vars are read-only from CFML and the Wheels get() setting
  needs a live framework context, so layer isolation isn't fully testable
  at that level (vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc:28-43).
- Update browser-test guides (v4-0-0 + v4-0-1-snapshot L319) to recommend
  setting this.baseUrl in the component pseudo-constructor instead of
  beforeAll — super.beforeAll() calls $resolveBaseUrl() before a
  beforeAll-override can take effect, silently inheriting layer 2-6 results.
- Mirror the same ordering note in .ai/wheels/testing/browser-testing.md.

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>

* fix(cli): make `wheels packages install` a real alias for `add` in dispatch (#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>

* fix(model): quote column identifiers in SELECT clause builder (#2787)

* fix(model): quote column identifiers in SELECT clause builder

The WHERE and ORDER BY clause builders already routed column names
through the adapter's $quoteIdentifier, but $createSQLFieldList — the
SELECT/GROUP BY engine — appended the column part raw. Models backed
by tables with reserved-word column names (e.g. `key`, `order`,
`group`) blew up on `findAll`/`findOne`/dynamic finders with cryptic
SQL syntax errors as soon as the SELECT list mentioned the column.

Also strips quote chars from the property extracted by the
duplicate-column rename loop so the alias replacement still matches
the unquoted ` AS <alias>` form, and updates the empty-pagination
columnList extraction in read.cfc to strip identifier quotes before
stripping the table prefix.

Fixes #2784

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

* docs(web/guides): note reserved-word column support via property alias in models guide

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

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

- Condense 4-line block comments at vendor/wheels/model/read.cfc:217
  and vendor/wheels/model/sql.cfc:634 to single-line comments
  (CLAUDE.md: "Never write multi-paragraph docstrings or multi-line
  comment blocks — one short line max").
- Stop using $quoteColumn() for the table-name argument in
  vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc;
  switch to the model's public $quotedTableName() helper so the spec
  names match what each helper actually quotes.
- Add a zero-row paginated findAll spec to
  reservedColumnQuotingSpec.cfc that exercises the QueryNew branch in
  vendor/wheels/model/read.cfc:225 with an aliased column, covering
  the path the original spec did not reach.
- Mention ORDER BY alongside SELECT and GROUP BY in
  web/sites/guides/src/content/docs/v4-0-1-snapshot/basics/models-and-the-orm.mdx
  so readers do not infer ORDER BY is unsafe with reserved-word
  columns (ORDER BY already routes through $quoteIdentifier).

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

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

Condense the remaining multi-line comment blocks in reservedColumnQuotingSpec.cfc
to single lines per CLAUDE.md ("Never write multi-paragraph docstrings or
multi-line comment blocks — one short line max"):

- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:10 — 3-line
  block about City's id -> countyid alias condensed.
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:18 — 3-line
  block about Author.firstName (property == column) condensed.
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:40 — 6-line
  block added in round 1 inside the zero-row pagination it() condensed to
  the single-line form Reviewer A supplied.

The line-30 GROUP BY comment was already single-line; A's "30-32" citation
was off-by-one for that one. No production code changed; pure comment-style
fix. Test totals unchanged at 4 pass / 0 fail in the spec; full model suite
remains 839 pass / 0 fail / 0 error / 11 skipped across 35 bundles.

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>

* fix(test): auto-bind include-injected globals into WheelsTest spec scope (#2793)

* fix(test): auto-bind include-injected globals into WheelsTest spec scope

`WheelsTest`'s pseudo-constructor used `getMetaData(application.wo).functions`
to discover which Wheels globals to copy into a spec's `variables`/`this`
scope. That metadata enumerates only methods declared on the CFC body and
silently skips symbols merged in via `cfinclude` — which is how
`vendor/wheels/Global.cfc` pulls user helpers from `app/global/functions.cfm`.
Apps with custom helpers (`can()`, `hasRole()`, etc.) had to manually rebind
each one in `beforeAll()`.

The loop now iterates `application.wo` as a struct and binds every UDF
detected by `isCustomFunction()`, while preserving the existing
public-only filter for metadata-declared methods and the don't-clobber
guard for scope members the spec (or its base class) already provides.

Fixes #2790

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

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

- Promote include-injected UDFs from `variables` to `this` in
  `vendor/wheels/Global.cfc` after `include "/app/global/functions.cfm"`,
  so the auto-bind loop in `vendor/wheels/WheelsTest.cfc` discovers them
  uniformly across Lucee, Adobe CF, and BoxLang. Lucee's struct-iteration
  over a CFC instance surfaces both `this` and `variables` scopes, but
  Adobe CF only reliably exposes `this`-scope members — without the
  promotion, the original bug (#2790) would silently persist on Adobe CF
  even with the new iteration path in WheelsTest.cfc.

- Fix the misleading header comment in
  `vendor/wheels/tests/specs/wheelstest/WheelsTestAutoBindIncludesSpec.cfc`.
  Bracket-notation assignment from outside writes to `this` scope, not
  `variables` — so the probe simulates the post-promotion shape, not the
  raw include shape. Comment now spells this out explicitly.

- Add a new `it` case that asserts the probe key is enumerated by
  `for (key in application.wo)`. Guards the iteration mechanism the
  auto-bind loop depends on, so failures on any engine where struct-
  iteration is narrower than expected would fail this spec rather than
  silently pass-but-not-test downstream.

Addresses Reviewer A's cross-engine concern (Adobe CF struct-iteration
contract) and Reviewer B's joint recommendation option (b): promote
include-injected helpers to `this` so the iteration path is uniform.
The accompanying spec correction handles A's "spec injects via wrong
scope" finding and B's "misleading header comment" note.

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

* fix(test): seed `local` scope before pseudo-constructor for-iterator

The promotion loop added in round 1 of the consensus fixup crashed
Lucee 7 with `variable [local] doesn't exist` at Global.cfc:3861 —
the test runner couldn't even reach a spec before bailing out.

In a CFC pseudo-constructor (component body, not inside a function),
the `local` scope is not auto-created. Direct assignment to
`local.X = ...` will seed it, but `for (local.X in Y)` tries to read
`local` first as the iterator's target parent and fails. WheelsTest.cfc
gets away with the same loop shape only because it does
`local.metaIndex = {}` earlier in its own pseudo-constructor; Global.cfc
had no such seeding line.

Add the minimum seeding statement (`local.varKey = "";`) directly above
the loop and document the cross-engine reason inline. The loop's
filter logic is unchanged.

The original review couldn't catch this — the round-1 address-review
sandbox lacked a working test runner so the fix went out unverified.

Signed-off-by: Peter Amiri <peter@alurium.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>

* fix(mapper): reject redundant namespace prefix in to= and controller= (#2794)

* fix(mapper): reject redundant namespace prefix in to= and controller=

Inside `.namespace("foo")` (or equivalent `.scope()` / `.package()`),
writing `to="foo/dashboard##index"` instead of `to="dashboard##index"`
silently produced a `foo.foo/dashboard` controller path that downstream
got flattened to a `Foodashboard`-style class lookup with an opaque
`Wheels.ViewNotFound` error — leaving users to chase the symptom rather
than the route definition.

`$match()` now detects when the parsed controller starts with the scope's
package converted to slash form and throws `Wheels.MapperArgumentInvalid`
at registration time. The error names the namespace and the offending
value and points at the correct shorter form.

Fixes #2791

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

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

- Snapshot `local.fromTo` / `local.originalTo` before the `to=` parse
  block so the error detail can distinguish `to=` vs direct `controller=`
  callers (Reviewer A nit).
- Add `Len(arguments.package) > 0` to the guard's outer condition so an
  empty package does not yield `prefix = "/"` and spuriously reject
  controllers whose path starts with a slash (Reviewer A response,
  Reviewer B round-1 missed-issue).
- Collapse multi-line block comments above the guard in `matching.cfc`
  and above the new `it()` group in `MatchingSpec.cfc` to one-liners
  to comply with CLAUDE.md style (both reviewers).
- Add a spec asserting `$match()` with `package = ""` and a controller
  starting with `/` is not falsely rejected.

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

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

- vendor/wheels/mapper/matching.cfc:328 — change local.hh = "##" to
  local.hh = "####" so the error-suggestion detail renders as
  to="dashboard##index" (source-correct CFML), not to="dashboard#index"
  (Reviewer A finding, Reviewer B verified).
- vendor/wheels/tests/specs/mapper/MatchingSpec.cfc:241–242 — collapse
  the 2-line comment inside the "Allows controllers..." spec body to a
  single line per CLAUDE.md "one short line max" rule (both reviewers).

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>

* fix(events): re-include app/global/*.cfm on bare ?reload=true when files change (#2795)

* fix(events): re-include app/global/*.cfm on bare ?reload=true when files change

Adding a helper to `app/global/functions.cfm` (or anything it `<cfinclude>`s)
used to require the password-gated `?reload=true&password=...` path. Bare
`?reload=true` re-ran config and routes but left `application.wo` (the
`Global.cfc` instance) intact, so the symbols merged into its variables scope
at construction time stayed frozen — the page rendered without error and the
new helper was silently undefined.

The fix follows the Rails/Phoenix per-request mtime-check pattern recommended
by the research comment: snapshot `app/global/*.cfm` mtimes on application
start, and on bare `?reload=true` in development re-evaluate the include if
any tracked file has been added, removed, or touched. The password-gated
`applicationStop()` path still does a full re-init unchanged — this just
makes the muscle-memory path actually work.

Three new helpers on `wheels.Global`:

- `$snapshotGlobalIncludes(directory)` — struct of `path → dateLastModified`
- `$globalIncludesChanged(snapshot, directory)` — diff against current state
- `$reincludeGlobals(file)` — re-evaluate the include against the live
  Global instance

New setting `reloadOnGlobalChange` defaults to `true` in development and
`false` everywhere else; opt out with `set(reloadOnGlobalChange=false)`.

Fixes #2792

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

* docs(web/guides): document reloadOnGlobalChange setting and bare reload behavior

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

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

- Replace `DirectoryCreate(baseDir, true)` with `java.io.File.mkdirs()`
  in `reloadGlobalsSpec` — the createPath flag is Lucee-only and the
  call lives in beforeEach, so every spec in the group fails on Adobe CI.
- Assert `IsDefined("application.wo.fxReinclude")` after each
  `$reincludeGlobals` call so the contract (re-included helpers must
  remain callable on application.wo) is guarded against silent no-ops.
- Lift include-declared functions from local/variables onto this in
  `$reincludeGlobals` so the contract holds across engines: include
  inside a method body would otherwise leave declarations in scopes
  that aren't reachable via `application.wo.helper()`.
- Wrap the bare ?reload=true re-include in a double-checked
  `lock name="wheels_reload_globals"` to eliminate the race between
  two concurrent ?reload=true hits.
- Document the development-only environment guard as intentional so a
  future maintainer doesn't try to enable `reloadOnGlobalChange` in
  staging and debug a silent no-op.
- Add docblocks to `$globalIncludesChanged` and `$reincludeGlobals` so
  all three new global-includes helpers carry consistent documentation.

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

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

- vendor/wheels/Global.cfc:$reincludeGlobals — drop the
  !ArrayFind(beforeVars, key) snapshot-diff guard from the second loop.
  On Adobe CF the include updates variables[key] in place across calls,
  so the guard silently skipped re-binding the updated function onto
  `this` on the second ?reload=true. Re-lifting is idempotent and the
  path is development-only.
- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc:88 — replace
  the second IsDefined assertion with a return-value check so a stale
  Adobe-CF binding cannot slip past CI (expect(...fxReinclude()).toBe("second")).

Local verification on Lucee 7 + SQLite after server reload:
core suite 3698 pass / 0 fail / 0 error; global suite 113 pass / 0 fail
/ 0 error (the 7 reloadGlobals specs all green).

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

* fix(test): address Reviewer A/B consensus findings (round 3)

- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc — hoist
  `g` and `baseDir` out of the `describe()` arrow-function callback
  and into `run()` as a shared `ctx` struct, then update every
  beforeEach / afterEach / it closure to reference `ctx.g` and
  `ctx.baseDir`. On Adobe CF 2023/2025 CFML closures cannot reach
  an enclosing function's `local` scope (CLAUDE.md cross-engine
  invariant ##3); the prior layout silently relied on Lucee 7's
  lexical capture and would have thrown "variable baseDir is
  undefined" inside every nested closure on Adobe CI, crashing all
  seven specs.

Local verification on Lucee 7 + SQLite (existing test server,
forced ?reload=true&password=wheels first):
  global directory (wheels.tests.specs.global): 113 pass / 0 fail / 0 error
  full core suite:                              3698 pass / 0 fail / 0 error

Lucee was already green before this fix because it captures the
enclosing arrow-function `local`; the change is to make the Adobe
CI legs match. Adobe verification is left to CI as the local
harness cannot run Adobe.

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

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

- web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/dev-server.mdx — tighten the bare `?reload=true` note so it mentions `app/global/functions.cfm` (and any files it `<cfinclude>`s) instead of the looser `app/global/*.cfm`. `$globalIncludesChanged` watches every `*.cfm` under `app/global/` but `$reincludeGlobals` re-evaluates only `functions.cfm` and the files it transitively includes; the previous wording implied a developer could drop a standalone helper file directly and have it bind, which isn't true. Matches the more accurate wording already in `core-concepts/environments-and-configuration.mdx`.
- web/sites/guides/src/content/docs/v4-0-1-snapshot/command-line-tools/wheels-commands/dev-server.mdx — same wording change for the v4-0-1 snapshot copy, keeping the two doc trees in sync.
- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc — drop the dead `mappingPath` local variable; it was only used to compute `absPath`, and the `$reincludeGlobals` calls inside the `notThrows` closures already repeat the path literally (closures can't reach the enclosing `local.mappingPath` on Adobe CF anyway). Inlines the string into `ExpandPath()` directly.

Local verification on Lucee 7 + SQLite:
  global directory (wheels.tests.specs.global): 113 pass / 0 fail / 0 error
  reloadGlobalsSpec only:                       7 pass / 0 fail / 0 error

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

* fix(events): address Reviewer A/B consensus findings (round 5)

- vendor/wheels/tests/specs/global/reloadGlobalsSpec.cfc — add a test
  for the DateCompare != 0 branch in $globalIncludesChanged, the
  "developer edited an existing helper" path the PR is designed to
  serve. Uses the backdated-snapshot approach from Reviewer A's
  response (DateAdd seconds -60 on the snapshot entry) rather than
  Sleep(1100), so the test is deterministic across filesystems with
  different mtime granularities.
- vendor/wheels/events/EventMethods.cfc — append application name to
  the wheels_reload_globals lock so concurrent ?reload=true hits from
  different apps on a shared Adobe CF server no longer serialize on a
  single global lock. Development-only and uncommon in practice, but
  the fix is a one-liner.

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs: tighten CHANGELOG entry to distinguish detection from re-evaluation scope

- CHANGELOG.md — round-5 Reviewer A nit. Opening clause said "re-includes
  app/global/*.cfm" which overstates the re-evaluation scope: detection
  is broad (every *.cfm under app/global/) but $reincludeGlobals only
  re-evaluates functions.cfm and the files it transitively <cfinclude>s.
  Rewording matches the round-4 dev-server.mdx tightening.

Signed-off-by: Peter Amiri <peter@alurium.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>

* fix(test): keep test-local.sh from silently dying on missing ~/.lucli/express (#2796)

Under `set -euo pipefail`, `find ~/.wheels/express ~/.lucli/express ...`
exits non-zero whenever any path arg is missing (stderr suppressed via
`2>/dev/null`, but the exit status survives), `pipefail` propagates it
through `head -1`, and the command-substitution assignment trips `set -e`.
The cleanup trap then fires with no server to clean up, so the user sees
"Starting Wheels CLI server on port 8080..." with EXIT=1 and no
`/tmp/wheels-test-server.log` produced — broken for every install since
the lucli→wheels rebrand window closed and `~/.lucli/express/` stopped
being created.

Drop the now-dead `~/.lucli/express` fallback (the rename landed in 3.0
and recent CLI releases extract Lucee Express to `~/.wheels/express/`
only) and add `|| true` for defense in depth so a truly fresh install
(before `wheels start` has ever run) leaves `LUCEE_LIB` empty and the
downstream `[ -n "\$LUCEE_LIB" ]` guard skips the JDBC pre-install
cleanly.

Verified: `bash tools/test-local.sh wheels.tests.specs.wheelstest` now
runs the server, produces `/tmp/wheels-test-server.log`, and passes all
137 specs across 38 suites in ~17s.

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: allow-list APPROVED + CHANGES_REQUESTED in Reviewer A guard (#2797)

* fix(ci): skip COMMENTED reviews in Reviewer A guard

The post-submission guard in bot-review-a.yml scans for "bogus" wheels-bot
reviews on the head SHA and dismisses any with a body shorter than 200
chars or missing the canonical `wheels-bot:review-a` marker. GitHub's
dismiss API only accepts APPROVED or CHANGES_REQUESTED reviews; passing a
COMMENTED review id returns HTTP 422 ("Can not dismiss a commented pull
request review") and crashes the step, red-X'ing the Reviewer A check
even when A's real substantive review landed cleanly seconds earlier.

wheels-bot itself occasionally posts placeholder COMMENTED reviews mid-
cycle as it probes the `gh pr review` CLI before issuing the real one
(observed bodies: "placeholder test - ignore", "placeholder2 - updating",
"test with dollar sign: see \$reincludeGlobals function"). Those leak out
as public COMMENTED reviews, which is what the guard is meant to clean
up — but it can't dismiss them via this API, so it has to skip them.

Add `select(.state != "COMMENTED")` to the jq pipeline that selects
actionable reviews. COMMENTED reviews don't gate merging anyway — only
APPROVED and CHANGES_REQUESTED do — so leaving them in PR history is
acceptable noise. The guard now only acts on what it can actually
dismiss.

Observed on PR #2795 commit 0db188a5a5d27cd80b58939df5e0c8dd7464a00b,
job run 26296842347.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* ci: allow-list APPROVED + CHANGES_REQUESTED in Reviewer A guard

Tighten the previous COMMENTED-state filter to an explicit allow-list:
APPROVED and CHANGES_REQUESTED are the only review states that (a)
GitHub's dismiss API accepts and (b) gate merging. Switching from
"!= DISMISSED && != COMMENTED" to "state IN (APPROVED, CHANGES_REQUESTED)"
also covers PENDING reviews, which return the same HTTP 422 from the
dismiss endpoint if they ever land on a head SHA.

Follow-up to bd76e53de4 per Reviewer A's allow-list suggestion on #2797.
Reviewer B confirmed PENDING is reachable and the closed-set form is
strictly safer than the deny-list. No behavioral change for the
COMMENTED case fixed in bd76e53de4 — that state is still excluded.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(migrator): handle orphan versions in shared dev databases (#2780) (#2798)

* fix(migrator): handle orphan versions in shared dev databases (#2780)

When wheels_migrator_versions records a version whose migration file is
not in the current checkout (shared dev DB / peer applied a migration
whose file isn't yet in this branch), wheels migrate latest no longer
takes a misleading "down" branch and silently no-ops.

Changes:

* Migrator.$getOrphanVersions() — diffs the tracking table against on-
  disk files and returns versions with no matching file, sorted ascending.

* Migrator.migrateTo() — branches on "orphan-at-top" before the existing
  direction check. When every DB version above target is an orphan: emits
  a warning naming the orphans, then either applies pending local files
  (up branch) or prints a clear "Nothing to do" naming current vs target.
  When SOME above-target versions are legitimate down candidates and
  SOME are orphans, emits the warning but lets the existing down loop
  handle the rest (orphan rows skip naturally because the loop iterates
  files only).

* Migrator.\$buildInfoOutput() — extracted info rendering so it is unit-
  testable without the HTTP dispatcher. Orphan rows render with a [?]
  marker and the literal "********** NO FILE **********" (Rails-style),
  with a footer explaining the cause.

* public/views/cli.cfm info handler — delegates to \$buildInfoOutput().

* New spec OrphanDetectionSpec covers \$getOrphanVersions + the
  directional fix across five scenarios (clean state, single orphan,
  multiple orphans, nothing-to-do path, legitimate down still works).

* New spec MigratorInfoSpec covers \$buildInfoOutput rendering.

* Docs: new guides page at basics/shared-development-databases.mdx
  walks through what an orphan is, three resolution paths, and the
  recommendation to avoid shared dev DBs. AI-side reference at
  .ai/wheels/troubleshooting/shared-dev-databases.md.

Note on local verification: a leaked JVM from another worktree is
holding Tomcat's shutdown port (8081), blocking new wheels server
starts on this machine. Tests are queued for CI to run across the full
engine + DB matrix. Local TDD will run on next machine restart.

Refs #2780

Signed-off-by: Peter Amiri <peter@alurium.com>

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

- Reword "Your latest local migration" to "Your target version" in the
  orphan nothing-to-do message. arguments.version is the target, not
  necessarily the latest local file (Migrator.cfc).
- Add mixed-case OrphanDetectionSpec covering orphan + legitimate down
  candidate above target: warning names the orphan, down branch still
  runs, c_o_r_e_dropbears is dropped after rollback.
- Drop docs/superpowers/plans/2026-05-22-orphan-migration-detection.md
  (1053-line agentic plan duplicated in PR body / commit message).
- Add CHANGELOG entry under [Unreleased] Fixed describing the orphan
  detection, three migrateTo branches, and the [?] info row.

Non-blocking items left for follow-up: double getVersionsPreviouslyMigrated()
fetch in migrateTo() (B disputed scope but underlying concern is valid),
and the deferred local test-local.sh migrator run (sandbox lacks the
wheels CLI binary; compat-matrix CI will validate on the new SHA).

Refs #2780, #2798

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

* docs(migrator): fix stale plan reference in shared-dev-databases.md

The 1053-line agentic plan file at
docs/superpowers/plans/2026-05-22-orphan-migration-detection.md was
deleted in commit ddac9690 per Reviewer A/B convergence, but the
AI-side reference doc still pointed to it.

Replaced the dead link with a PR reference and tightened the
follow-up section to describe the work in prose instead of pointing
at plan files that may or may not exist.

Round-2 Reviewer A nit.

Refs #2780, #2798

Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* feat(migrator): doctor/forget/pretend reconciliation commands (#2780) (#2799)

* feat(migrator): doctor/forget/pretend reconciliation commands (#2780)

Follow-up to #2798. Adds three new `wheels migrate` subcommands for
manual reconciliation against the tracking table — the Flyway
`validate` / `repair` / `SkipExecutingMigrations` analogues for
Wheels.

* `wheels migrate doctor` — single-command health report. Lists
  orphans, pending local migrations, and applied count. Pure read;
  never mutates. Built on Plan 1's `$getOrphanVersions()`.

* `wheels migrate forget <version> --yes` — removes a single row
  from `wheels_migrator_versions` without running `down()`. Refuses
  if the version has a matching local file (use `migrate down`
  instead) or if the version isn't in the table. Idempotent.

* `wheels migrate pretend <version> --yes` — inserts a row into
  `wheels_migrator_versions` without running `up()`. Refuses if
  already applied or if no local file matches.

Both `forget` and `pretend` require explicit `--yes` to mutate;
without it they print what would happen and exit. The CLI side wires
through a new `runForgetOrPretend()` helper that handles the gating;
the HTTP dispatcher in `cli.cfm` reads the version from
`request.wheels.params.version`.

New spec `MigratorReconciliationSpec.cfc` covers ~12 scenarios
across the three methods: clean state, unhealthy with orphans,
unhealthy with pending, refusal paths, and the no-mutation guarantee
for invalid inputs.

Docs: extended `basics/shared-development-databases.mdx` with the
new commands (Option 2 reworked, Option 2b added, "Comprehensive
diagnostic" section added). AI-side reference and CHANGELOG entry
follow the same shape.

Note on local verification: a leaked JVM from another worktree
continues to hold Tomcat's shutdown port (8081); CI compat-matrix
will validate across the full engine + DB matrix.

Refs #2780, #2798

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(migrator): address Reviewer A round-1 findings on #2799

- forgetVersion() now delegates to the existing private
  $removeVersionAsMigrated() helper instead of running an inline
  DELETE. The helper wraps the DELETE in the same request.$wheelsDebugSQL
  guard that $setVersionAsMigrated() uses, so forgetVersion() and
  pretendVersion() are now symmetric: both are no-ops in debug-mode
  request contexts instead of silently corrupting the tracking table.

- runForgetOrPretend() now wraps the version argument with
  URLEncodedFormat() when building the reconcile URL. $sanitiseVersion()
  on the server side strips non-digits before SQL use (no SQL injection
  path), but raw URL-special characters (&, =, %) in the CLI argument
  could inject spurious query parameters before the sanitiser ran.

Both items were flagged by Reviewer A on commit b7817459b. No new tests
needed: existing MigratorReconciliationSpec covers the success and
refusal paths for forgetVersion(), and the URL-encoding fix is on the
CLI client side which is not exercised by core unit tests.

Refs #2780, #2799

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(cli): doctor output prints yellow when migrator is unhealthy

Reviewer A flagged that runMigration("doctor") always printed in green
regardless of the report's healthy flag. Green on an unhealthy result
(orphans or pending migrations present) reads as "everything is fine"
when actually attention is needed.

The fix switches color to yellow when:
  - action == "doctor"
  - response includes a `healthy` key
  - healthy is false

Other actions (latest/up/down/info) stay green on success — they
either succeed or throw, so the binary mapping holds. Only `doctor`
returns a structural "succeeded but unhealthy" state.

Non-blocking per A's COMMENTED verdict, but a real UX improvement.

Refs #2780, #2799

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs: cross-reference doctor/forget/pretend in main migration docs

The cumulative state of #2798 (merged) and #2799 (this PR) added
three new migrate subcommands plus the [?] orphan display in
`migrate info`. PR-scope docs already covered the deep-dive page
at basics/shared-development-databases.mdx, but the main migrations
reference still said "four subcommands". Catching up:

- basics/migrations.mdx now lists seven subcommands and points to
  shared-development-databases for the reconciliation deep dive.
  The `info` description mentions the [?] orphan row format.

- CLAUDE.md's MCP/CLI table row now lists `doctor` under
  wheels_migrate and adds a separate "Migrator reconciliation"
  row for forget/pretend.

- CLAUDE.md's Migrations & Seeding section gains a "Shared Dev
  DB Reconciliation" subsection summarising the new commands and
  pointing at the deep-dive docs.

- CLAUDE.md's Reference Docs section adds the
  .ai/wheels/troubleshooting/shared-dev-databases.md link.

Refs #2780, #2798, #2799

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs: replace stale schema_migrations references with wheels_migrator_versions

Three carryover Rails table-name references in basics/migrations.mdx
(lines 28, 36, 59) predate the c_o_r_e_* → wheels_* rename and don't
match any code in vendor/wheels/, cli/, or app/. Reviewer A flagged
them on PR 2799 round-4 review (f663b5a55) as worth fixing since the
file was already being touched; the surrounding edits in this PR
already use the correct `wheels_migrator_versions` name (line 46).

Other stale references in basics/seeding.mdx and the
v4-0-1-snapshot/ copies are out of scope for this PR; left for a
follow-up cleanup.

Refs #2780, #2799

Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>

* feat(migrator): enrich wheels_migrator_versions with name + applied_at (#2780) (#2800)

* feat(migrator): enrich wheels_migrator_versions with name + applied_at (#2780)

Final follow-up to #2798 + #2799. Adds two nullable columns to the
tracking table so `wheels migrate info` and `wheels migrate doctor`
can show *what* a peer applied and *when* — even for orphan rows
whose migration file isn't in the local checkout.

## Schema change

- `wheels_migrator_versions.name VARCHAR(255) NULL` — migration name
  derived from filename (e.g. `create_users`)
- `wheels_migrator_versions.applied_at TIMESTAMP NULL DEFAULT
  CURRENT_TIMESTAMP` — when the migration was applied. SQLite gets
  TEXT with CFML-side `Now()` injection on insert because SQLite
  can't DEFAULT a column on ADD COLUMN.

Both nullable, additive, backward compatible. Existing rows
(pre-enrichment) stay NULL and display version-only in the info
output. Going-forward-only — no backfill.

## Bootstrap

`Migrator.$ensureTrackingColumns()` probes via `$dbinfo` and ALTERs
the table per-engine when columns are missing. Idempotent (skip
when already present). Per-engine SQL covers MySQL, PostgreSQL,
SQLite, MSSQL, Oracle, H2, and CockroachDB — same pattern as the
existing `renameSystemTables()` machinery at Migrator.cfc:932.

Wrapped by `$maybeEnsureTrackingColumns(appKey)` which caches on
`application[appKey].$trackingColumnsEnsured` so the ALTER runs
once per app process, not on every migrator call. Non-fatal: if
the ALTER fails (locked table, weird permissions), the flag stays
unset and the legacy schema continues to work.

Called from `$getVersionsPreviouslyMigrated()` after both the
existing-table happy path and the bootstrap-creates-table path.

## Population

`$setVersionAsMigrated(version, migrationName)` gains an optional
`migrationName` arg. When non-empty AND the enriched-columns flag
is set, includes `name` (and on SQLite, `applied_at`) in the
INSERT. Other engines rely on the column's CURRENT_TIMESTAMP
default for `applied_at`.

Callers updated to pass the name: both up-loops in `migrateTo()`,
`migrateIndividual()`, and `pretendVersion()` (captures the
migration's name from `getAvailableMigrations()` during the
local-file existence check).

## Display

`$getOrphanVersionsWithMeta()` — new public helper. Returns
`{version, name, appliedAt}` structs for each orphan. Falls back
to bare structs when columns aren't ensured or the SELECT fails.

`$buildInfoOutput()` and `cli.cfm`'s `doctor` case now render
`[?] <version> <name> (applied <timestamp>)` when populated, with
the legacy `[?] <version> ********** NO FILE **********` fallback
for legacy NULL orphans.

`doctor()` adds `orphansWithMeta` to its result struct alongside
the existing `orphans: array of strings` (kept for
backward-compat with `migrateTo()`'s direction logic).

## Docs

- `web/sites/guides/.../basics/shared-development-databases.mdx`:
  updated `info` sample output to show the enriched format + a
  note explaining the legacy fallback.
- `.ai/wheels/troubleshooting/shared-dev-databases.md`: new
  "Schema enrichment" section documenting the helpers and behavior.
- `CLAUDE.md`: updated the `info` format line and added the
  tracking-table schema summary in the Shared Dev DB Reconciliation
  subsection.
- `CHANGELOG.md`: entry under `[Unreleased] Changed`.

## Tests

`SchemaEnrichmentSpec.cfc` covers three scenarios:
- $ensureTrackingColumns adds both columns on first call
- Idempotent re-run (added=[])
- Name column populated for newly applied migrations

Existing specs (migratorSpec, OrphanDetectionSpec, MigratorInfoSpec,
MigratorReconciliationSpec) all preserve backward compat — the new
name column is opt-in (only written when caller passes it), and
the new display logic falls back to legacy rendering for NULL
metadata.

## Known follow-ups (non-blocking, separate PRs)

- Double `$getVersionsPreviouslyMigrated()` fetch carry-over from
  reviewer notes on #2798 and #2799 — still pending. This PR
  doesn't worsen the situation; the cached `$trackingColumnsEnsured`
  flag means `$ensureTrackingColumns` only probes columns once per
  process.

Refs #2780, #2798, #2799

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(migrator): don't call \$dbinfo inside $setVersionAsMigrated transaction

CI on Lucee 7 + SQLite revealed two correctness bugs in #2800:

1. \$setVersionAsMigrated called \$dbinfo(type="version") to detect
   SQLite-vs-other-engines before deciding whether to include applied_at
   in the INSERT. But \$setVersionAsMigrated runs INSIDE migrateTo's
   open JDBC transaction, and SQLite (and possibly other engines under
   concurrent load) can't service JDBC metadata calls inside an open
   transaction. Result: [SQLITE_ERROR] SQL error or missing database,
   the migration's transaction rolled back, and every test that
   applied a migration with the enriched schema active failed.

   Fix: cache the engine type on application[appKey].\$migratorDbType
   in \$ensureTrackingColumns() (which always runs outside transactions),
   then read it from the cache in \$setVersionAsMigrated(). No more
   \$dbinfo calls during inserts.

2. \$maybeEnsureTrackingColumns set the \$trackingColumnsEnsured cache
   flag whenever \$ensureTrackingColumns() didn't throw — even when
   the column probe failed (e.g. table didn't exist yet) and no
   ALTERs ran. Subsequent \$setVersionAsMigrated calls then saw the
   flag set and tried to INSERT with a `name` column that didn't
   actually exist on the table → SQL error → rolled back transaction.

   Fix: only set the cache flag when BOTH rv.hasName and rv.hasAppliedAt
   are true after \$ensureTrackingColumns() returns. Failures, missing
   tables, or partial-ALTER states leave the flag unset so subsequent
   calls retry.

Together these failures broke six specs across migratorSpec,
OrphanDetectionSpec, and SchemaEnrichmentSpec — all symptoms of the
same two root causes. With the fix in place, \$setVersionAsMigrated
is back to a single $query call with no metadata interrogation, and
the enriched-INSERT path only fires when the schema is genuinely
confirmed.

Refs #2780, #2798, #2799, #2800

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(migrator): address Reviewer A round-1 findings on #2800

C1 (correctness): The shared-dev-databases guide example showed
"(applied ...)" timestamps on [x] rows, but the code in
$buildInfoOutput only populates appliedAt for orphan rows (the
DB isn't re-queried for applied_at on local-file rows). Aligning
the guide example with what the code outputs — keep the timestamp
display on the [?] orphan row where the schema enrichment actually
shows through, drop it from the [x] rows. Surrounding prose already
correctly scopes the enrichment to orphans.

T1 (test quality): SchemaEnrichmentSpec's "populates the name column"
test used expect(Len(rows.name) > 0).toBeTrue() which collapses to
a boolean before the matcher sees it — on failure the error message
is just "Expected [false] to be [true]" with no hint of the actual
value. Replaced with expect(rows.name).notToBeEmpty() so failures
include the actual name (or absence of one).

T2 (coverage): Added a new "populates applied_at for newly applied
migrations" spec that queries the applied_at value and asserts it
parses as a date. Covers both code paths: the column-DEFAULT
CURRENT_TIMESTAMP that fires on MySQL/Postgres/MSSQL/Oracle/H2 AND
the CFML-side Now() that SQLite needs because it can't DEFAULT a
TIMESTAMP on ADD COLUMN.

T3 (lint): Added a comment block above the describe block
explaining why CockroachDB is skipped — mirrors the existing
pattern in migratorSpec, OrphanDetectionSpec, and MigratorInfoSpec
(numeric-version test fixtures don't run cleanly against CockroachDB;
compat-matrix.yml treats CockroachDB as soft-fail). The guard is
intentional, not vestigial.

Also clearing the new $migratorDbType app-scope cache in the
beforeEach/afterEach alongside the existing $trackingColumnsEnsured
cleanup, so each test starts from a clean state.

A's performance note about $dbinfo in $setVersionAsMigrated is
already addressed in commit bb4bd761b (caches the engine type on
$migratorDbType to avoid the per-insert metadata round-trip). A
was reviewing 695f8300c and hadn't seen that commit yet.

Refs #2780, #2800

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs: correct stale [x] timestamp claim in .ai schema-enrichment ref

Reviewer B's round-1 convergence on #2800 caught a missed-by-A
follow-on of the C1 finding: the .ai/wheels/troubleshooting/
shared-dev-databases.md doc's first display bullet read

  - `[x] <version> <name> (applied <timestamp>)` when populated

but the code only renders the (applied ...) suffix on orphan rows.
The MDX guide example was corrected in fdbbedc35; this commit
brings the .ai-side reference in line and adds a sentence explaining
why $buildInfoOutput doesn't show applied_at for [x] rows (it would
require re-querying the tracking table for every getAvailableMigrations
result, which is a bigger change than this PR is taking on).

Refs #2780, #2800

Signed-off-by: Peter Amiri <peter@alurium.com>

* fix(migrator): populate \$migratorDbType cache before early-return (#2800 C2)

Reviewer A round-2 caught a latent bug in \$ensureTrackingColumns:
the early-return when both enriched columns are already present
fired BEFORE the \$migratorDbType cache was populated. The cache is
on application scope, so it gets cleared on every app restart /
?reload=true. On the second app start onward, the early-return
fired, \$migratorDbType stayed empty, and \$setVersionAsMigrated's
SQLite branch (which writes an explicit applied_at via CFML Now()
becau…
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