| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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>
…der (#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>
…, 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>
…ade (#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>
#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>
…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>
…ibe() 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>
…ing 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>
…tance 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>
…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>
* 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>
…ope (#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>
…#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>
…les 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>
…/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>
* 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 0db188a,
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 bd76e53 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 bd76e53 — 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>
…#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 ddac969 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>
…#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 b781745. 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 (f663b5a) 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>
#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 bb4bd76 (caches the engine type on $migratorDbType to avoid the per-insert metadata round-trip). A was reviewing 695f830 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 fdbbedc; 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() because SQLite can't DEFAULT a TIMESTAMP on ADD COLUMN) was skipped. Result: SQLite's applied_at landed NULL on every migration applied after the first app start. Fix: populate the cache BEFORE the hasName/hasAppliedAt early-return, guarded by StructKeyExists so it fires at most once per app process. The existing ALTER path now reads from the cache instead of issuing its own \$dbinfo round-trip (still inside \$ensureTrackingColumns, which runs outside transactions — safe). Regression test: added "populates applied_at across app restarts (regression for round-2 C2)" that explicitly simulates the broken sequence: migrate 001, wipe both caches (mimicking app restart), migrate 002, assert applied_at is still a parseable date. On the unpatched code this would fail on SQLite (applied_at = NULL). Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): refresh tracking-columns flag from probe on every call CI on Lucee 7 + SQLite still failed after the C2 patch (#2800 round 2). Same 6 failures: migrations rolled back with [SQLITE_ERROR] SQL error or missing dat[a] when applying 001. Root cause was a different stale- cache bug than C2 — the $trackingColumnsEnsured flag persisted across test runs but the underlying tracking table was dropped + recreated in each spec's beforeEach. $maybeEnsureTrackingColumns saw the flag set and skipped $ensureTrackingColumns; the freshly-bootstrapped table had only (version, core_level), but $setVersionAsMigrated still tried to INSERT against the missing `name` column → migration rolled back. Production analog: any operator-driven DROP / recreate of wheels_migrator_versions (rare but possible) would silently corrupt the next migrator run the same way. Fix: drop the skip-on-flag-set early return in $maybeEnsureTrackingColumns. Always call $ensureTrackingColumns; the helper is already idempotent and short-circuits when both columns are present (one $dbinfo column probe — cheap). Set the flag from the actual probe result on every call; clear it on probe failure or partial-ALTER outcome. The $migratorDbType cache from the C2 fix still avoids the per-INSERT $dbinfo round-trip, so the perf cost of removing the skip is one column probe per migrator call — well within the existing per-call cost. Regression test: "re-runs ALTER when the tracking table is dropped+recreated" sets up the exact failing scenario — applies 001 (sets cache), drops the tracking table, applies 001 again, and asserts the name column is populated. On the unpatched code the second migrateTo would error on the INSERT and roll back. Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * test(migrator): fix regression test setup — drop both tables The previous round's regression test for the stale-flag scenario left c_o_r_e_bunyips in place when dropping wheels_migrator_versions. On the second migrateTo("001") call, up()'s createTable("c_o_r_e_bunyips") failed because the table already existed, the transaction rolled back, and the expected INSERT row never landed (Expected [1] but received [0]). The bug under test was correctly fixed by the prior commit (2f71af2) — the failure was purely a test-setup miss. Drop both the tracking table AND c_o_r_e_bunyips so the second migrateTo can re-apply 001 from a clean schema. The cache-flag-stays-set scenario is preserved, which is what the regression test is actually verifying. Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * docs: sync main migrations.mdx with Plan 3 orphan display format basics/migrations.mdx's one-line summary of `wheels migrate info` still described orphan rows in the legacy "********** NO FILE **********" format only. Plan 3 enriched that — orphans with populated name + applied_at columns now show as `[?] <version> <name> (applied <timestamp>)`, falling back to the legacy display for pre-enrichment rows. Mirrors the same sync that landed in Plan 2 (when the orphan format was first added). The deep-dive page at basics/shared-development-databases.mdx already shows both formats with sample output; this brings the main reference in line. Refs #2780, #2798, #2799, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com>
…o sibling v4 docs (#2801) Carryover from PR #2799. Commit eacf4f6 fixed three stale references in v4-0-0/basics/migrations.mdx; the same family of references still appeared in three siblings and is fixed here in one pass: - v4-0-0/basics/seeding.mdx:26 - v4-0-1-snapshot/basics/migrations.mdx:28,36,49 - v4-0-1-snapshot/basics/seeding.mdx:26 The on-disk table has been wheels_migrator_versions since the c_o_r_e_* → wheels_* rename; no schema_migrations table exists anywhere in vendor/wheels/, cli/, or app/. The line-49 fix in v4-0-1-snapshot/basics/migrations.mdx also mirrors the "keys on the filename" → "keys on the timestamp prefix" wording fix from eacf4f6, since the tracking table stores only the timestamp version (e.g., 20260420143000), not the full filename. v3-0-0/command-line-tools/cli-guides/migrations.md:632-633 intentionally left as-is: v3 used a different table name pre-rename, so the references are historical and accurate. Verified: \`grep -rn schema_migrations web/sites/guides/src/content/docs/v4-0-0 web/sites/guides/src/content/docs/v4-0-1-snapshot\` now returns zero matches. Refs #2780, #2798, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…flag (#2781) (#2802) * feat(migrator): t.references() columnNames alias + underscore suffix flag (#2781) `t.references()` has shipped since the 2017 dbmigrate merge with two quirks that bite anyone writing FK columns: the parameter is `referenceNames` (every sibling helper uses `columnNames` via `$combineArguments`), and the generated column suffix is `<x>id` (no underscore), breaking Wheels model `belongsTo` defaults which expect `<x>_id`. This PR resolves both quirks without changing default behavior for existing apps. ## Changes - **`t.references()` accepts `columnNames`** as an alias for `referenceNames` via `$combineArguments(combine="referenceNames,columnNames", required=true)`. Both keep working; `columnNames` is the modern form. - **New setting `useUnderscoreReferenceColumns`** governs the column suffix. Framework default: `false` (legacy `<x>id` preserved). New apps generated via `wheels new` get `set(useUnderscoreReferenceColumns=true)` pre-wired in `config/settings.cfm`. When true, both the FK column and the polymorphic `<x>type` companion become `<x>_id` / `<x>_type`. - **Scaffold template** flipped to `t.references(columnNames="vacation")` reflecting the modern convention. - **Conventions documented**: new `vendor/wheels/migrator/CLAUDE.md` formalizes the singular/plural alias pattern + the suffix flag. Anti-pattern #15 added to top-level CLAUDE.md so AI agents pick up the rule. Doc-block on `$combineArguments` expanded to position it as the framework's canonical alias convention with citations. ## Tests 5 new specs in `vendor/wheels/tests/specs/migrator/referencesSpec.cfc` covering alias acceptance, both flag states, and the polymorphic suffix path. Full suite: 3761 pass / 0 fail (was 3756 pre-PR). ## Scope Focused PR1 of a planned 4-PR series. Migration.cfc command-version consistency (`addReference`, `dropReference`, `removeColumn`, `addColumn`, `changeColumn`, `addForeignKey` argument aliases + suffix flag in those sites) lands in PR2 to keep this surface tight. `wheels upgrade check` advisory-tier and specific advisories for opt-in conventions follow in PR3 and PR4. Closes #2781 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): address Reviewer A/B consensus findings (round 1) - Patch Migration.cfc::removeColumn(referenceName=) and addReference() to read $get("useUnderscoreReferenceColumns") so the wheels new template default (true) no longer produces wrong column names for these helpers - referencesSpec.cfc: correct file header to describe what the spec actually covers; sibling Migration.cfc DB-roundtrip coverage stays in migrationSpec.cfc - referencesSpec.cfc: add explicit useUnderscoreReferenceColumns=false reset to the alias describe block so test order can't false-fail it - referencesSpec.cfc: add direct backward-compat test asserting t.references(referenceNames="user") still produces the expected column - vendor/wheels/migrator/CLAUDE.md: drop the now-stale "lines 258, 278 hard-code & 'id'" anti-pattern note; both sites now route through $get - CHANGELOG.md [Unreleased] ### Added: document the columnNames alias and the useUnderscoreReferenceColumns setting (#2781) - web/sites/guides v4-0-0 + v4-0-1-snapshot migrations.mdx: document the columnNames argument as the preferred form, the legacy referenceNames back-compat, and the useUnderscoreReferenceColumns suffix flag Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * test(migrator): address Reviewer A/B round-2 — addReference test coverage Round-1 left two gaps: 1. `referencesSpec.cfc` header claimed Migration.cfc DB-roundtrip coverage in migrationSpec.cfc that didn't actually exist. 2. `Migration.cfc::addReference()` was patched to read $get("useUnderscore ReferenceColumns") but had zero direct tests anywhere. This commit closes both. ## Changes - `vendor/wheels/tests/specs/migrator/migrationSpec.cfc`: new "Tests addReference" describe block with two DB-roundtrip tests (`creates a FK on <name>id when flag is false` and `creates a FK on <name>_id when flag is true`). Both follow the existing "Tests addForeignKey" pattern and skip on SQLite (which doesn't support altering CONSTRAINTS), so they run against MySQL/PG/MSSQL/H2 in CI. - `vendor/wheels/tests/specs/migrator/referencesSpec.cfc`: header rewritten to accurately describe coverage — addReference DB-roundtrip now correctly points at migrationSpec.cfc, removeColumn(referenceName=) is noted as having TableDefinition-layer coverage only (PR2 follow-up adds direct DB-roundtrip via migrationCommandsSpec.cfc). - `vendor/wheels/migrator/CLAUDE.md`: Tests section updated to match the new spec organization — explicit about which file covers what, why TableDefinition-layer tests are preferred for argument plumbing, and that FK-touching tests skip on SQLite. ## Verification - `bash tools/test-local.sh migrator` (Lucee 7 + SQLite): 224 pass, 0 fail, 6 skipped (the 4 existing FK-altering tests plus my 2 new ones on the same SQLite skip path). No regressions. - The new tests fire on every non-SQLite engine in CI. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * test(migrator): add removeColumn(referenceName=) flag tests to close R2 coverage gap Round-2 follow-up commit. The bot's address-review described removeColumn(referenceName=) DB-roundtrip tests but didn't actually push them (my round-2 commit was already at HEAD). This commit makes the bot's address-review description retrospectively accurate and closes the coverage gap that finding 1 originally cited. ## Changes - `vendor/wheels/tests/specs/migrator/migrationSpec.cfc`: extend the existing "Tests removeColumn" describe block with two new it() blocks — one per flag state — that verify `removeColumn(referenceName=...)` computes the right column name to drop under both `useUnderscoreReferenceColumns=false` (legacy `<name>id`) and `=true` (modern `<name>_id`). These don't need the SQLite skip the addReference tests have because removeColumn just emits `ALTER TABLE DROP COLUMN`, which SQLite supports. - `vendor/wheels/tests/specs/migrator/referencesSpec.cfc`: header updated to drop the "no direct DB-roundtrip coverage yet" caveat for removeColumn(referenceName=) since that coverage now exists in migrationSpec.cfc. ## Verification - `bash tools/test-local.sh migrator`: 226 pass / 0 fail / 6 skipped (gained 2 from the new removeColumn tests; the addReference tests still skip on SQLite). - `bash tools/test-local.sh` (full): 3764 pass / 0 fail. No regressions. The new tests follow the same pattern as the existing "drops column from table" test in the same describe block. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * test(migrator): save/restore useUnderscoreReferenceColumns in migrationSpec beforeAll/afterAll Reviewer A round-3 robustness note: the flag-true tests in migrationSpec.cfc reset the flag at the end of the test body, but if addReference() or $dbinfo() throws between the set and the cleanup, the dirty global leaks into every subsequent spec in the file. Mirror the existing migratorObjectCase pattern — snapshot the flag in beforeAll and restore it in afterAll. The in-test resets stay as belt-and-suspenders; afterAll is the safety net for the throw case. Reviewer A's second item (misleading header comment in referencesSpec) was already fixed in dd2963c (previous commit) — the header now just points at "Tests addReference" and "Tests removeColumn" describe blocks without overclaiming what the TableDefinition-layer tests cover. ## Verification - `bash tools/test-local.sh migrator`: 226 pass / 0 fail / 6 skipped. No regressions; the addReference and removeColumn tests still pass under the new save/restore contract. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * test(migrator): capture removeColumn state before assert so cleanup always runs Reviewer A round-4 nit: the legacy and underscore removeColumn tests had a mid-test sanity expect() between createTable and dropTable. If the sanity assert fires, the test fails AND leaves the table in the database (next run recovers via force=true, but the dirty state is ugly). Refactor to the established addForeignKey / addReference pattern in the same file: capture before/after column lists, run cleanup, then do both asserts at the end. Cleanup is guaranteed regardless of which assert fails. Round-3 finding 1 (no beforeAll/afterAll save/restore for useUnderscoreReferenceColumns) was already fixed in 53034f9 — the reviewer was looking at dd2963c, one commit older. The in-test flag reset in the underscore test stays as belt-and-suspenders; the afterAll() guard added in 53034f9 is the safety net for an exception above that line. ## Verification - `bash tools/test-local.sh migrator`: 226 pass / 0 fail / 6 skipped. No regressions. The Reviewer B check failure on the prior SHA was a transient GitHub Actions auth error (HTTP 401: Bad credentials calling GraphQL), not a test or code failure. The new commit will trigger a fresh Reviewer B run. Signed-off-by: Peter Amiri <peter@alurium.com> 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> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…2781) (#2804) * feat(migrator): Migration.cfc command consistency sweep — follow-up to #2781 PR1 (#2802) fixed `t.references()` at the `TableDefinition` layer; this PR brings the matching command-version helpers in `Migration.cfc` into the same convention. **Argument-name consistency** — every column-accepting helper in Migration.cfc now accepts the same plural/singular alias shape as the `TableDefinition` helpers via `$combineArguments`: - `addColumn` / `changeColumn` — accept `columnNames` as alias for `columnName` - `removeColumn` — accepts `columnNames` as alias for `columnName` - `addReference` / `dropReference` — accept `columnName` / `columnNames` as aliases for `referenceName` - `addForeignKey` — accepts `columnName` as alias for `column` Legacy parameter names continue to work. The aliases match what every other column helper in `TableDefinition.cfc` already accepts. **Suffix-flag correctness** — two hard-coded `& "id"` concatenations in `Migration.cfc` now route through `useUnderscoreReferenceColumns`, matching the `references()` behavior shipped in PR1: - `removeColumn` (line 258) — when `referenceName=` is used, the dropped column name uses `_id` or `id` per the flag, so apps that opted into the underscore convention drop the right column. - `addReference` (line 278) — when adding an FK from `addReference()`, the column name follows the flag. Without these, an app with `set(useUnderscoreReferenceColumns=true)` that created `user_id` via `t.references()` would silently fail to drop or constrain it via the command-version helpers. `vendor/wheels/tests/specs/migrator/migrationCommandsSpec.cfc` — 4 new integration tests covering both the alias acceptance and the suffix-flag round-trips for `removeColumn`. Full suite: 3765 pass / 0 fail (was 3761 pre-PR2). `vendor/wheels/migrator/CLAUDE.md` updated with a complete table of modern alias → legacy parameter mappings and a note that all four reference-column concat sites now route through `$get`. PR1 (#2802) — t.references() at TableDefinition (CLOSES #2781) PR2 (this) — Migration.cfc command consistency PR3 — wheels upgrade scanner advisory tier (independent) PR4 — specific advisory entries for the underscore-references opt-in This branch is based on PR #2802. Once that merges to develop, GitHub will narrow this PR's diff to just the Migration.cfc + spec changes. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * refactor(migrator): address Reviewer A round-1 — symmetry + missing coverage PR2 round-1 review (Reviewer A) raised three correctness nits and four test-coverage gaps. All addressed here. ## Correctness - `removeColumn` now uses `$combineArguments(columnName,columnNames)` the same way every other helper in this PR does, replacing the manual `if (!Len(...) && Len(...))` block. Behavior matches the framework-wide alias precedence convention (alias wins when both present). - `addColumn` calls `$combineArguments` itself BEFORE delegating to `changeColumn`. Was previously relying on changeColumn's resolution; now the alias path is self-contained against future refactors that might stop delegating. - `addReference` and `dropReference` now carry an inline comment documenting the alias-wins precedence — previously unreachable when `referenceName` was `required`, now reachable since it's optional. ## Tests - `migrationCommandsSpec.cfc::changeColumn accepts columnNames alias directly`: direct changeColumn call that doesn't go through addColumn, guarding against the delegation-chain refactor case. - `addForeignKey accepts columnName alias for the legacy column param`: alias-resolution test (trap only `Wheels.IncorrectArguments`; SQL errors on SQLite are irrelevant to the alias contract). - `dropReference accepts columnName alias`: similar pattern. Closes the zero-coverage gap on dropReference flagged in the review. ## Docs - `addColumn` doc-block: documents the new `@columnNames` alias and fixes the `THe` typo on `@columnName`. ## Verification - `bash tools/test-local.sh migrator`: 231 pass / 0 fail (up 3 from 228 in the rebase commit — three new tests). - `bash tools/test-local.sh` (full): 3769 pass / 0 fail. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * docs(migrator): fix changeColumn javadoc — typo + missing @columnNames Round-2 review (Reviewer A) flagged that the previous commit only patched `addColumn`'s docblock, leaving the identical `THe` typo on `@columnName` (line 194) untouched in `changeColumn`'s docblock and missing the `@columnNames` alias entry that this PR added to the function signature. Fixes both: typo corrected, `@columnNames` line added matching the shape used in `addColumn`'s docblock and the TableDefinition column helpers. Reviewer A's other round-2 finding (commit type `test+refactor` invalid for commitlint) was already resolved by amending the prior commit to `refactor(migrator):` — that's the parent of this commit. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * docs(migrator): document the new alias parameters on all four sibling helpers Reviewer A round-3 comment-grade finding: addColumn and changeColumn javadocs document the new @columnNames alias, but the other four functions that gained alias parameters in this PR (removeColumn, addReference, dropReference, addForeignKey) were left undocumented. Closes the doc-only inconsistency: - removeColumn: adds @columnNames matching the addColumn/changeColumn shape. - addReference: adds @columnName + @columnNames with the precedence note ("columnNames wins" mirrors the inline code comment). - dropReference: same shape as addReference. - addForeignKey: adds @columnName as alias for the legacy @column. Pure javadoc — zero runtime changes. The whole PR's premise is "consistency for AI-driven and human-driven dev"; leaving four out of six javadocs on the new aliases was the opposite of that. ## Verification bash tools/test-local.sh migrator: 231 pass / 0 fail. No regressions. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): restore changeColumn required-arg enforcement (PR2 R2 regression) Reviewer B round-2 caught a real regression: the original `changeColumn` signature had `required string columnName` (no default), which CFML enforced at the parameter level — calling without it threw immediately. When this PR widened the signature to `string columnName, string columnNames` to accept the alias, the `required` keyword was dropped and `$combineArguments(required=false)` left no enforcement. A caller who passed neither would silently propagate an empty column name into downstream invoke()/SQL, producing cryptic errors instead of a clear "required argument" message. Reviewer B's literal fix ("just use required=true") would have broken the columnType="reference" path, where the column name is computed from `referenceName` and `columnName` is intentionally absent. The fix here uses a conditional `required` argument on `$combineArguments`: required = (arguments.columnType != "reference") For non-reference column types, the alias is required (restoring original enforcement). For reference types, the check skips — same shape the original code supported by ignoring columnName in that branch. Applied symmetrically to addColumn (which delegates to changeColumn). ## Regression tests migrationCommandsSpec.cfc gains a new "required-arg regression guards" describe block with three tests: - addColumn(table=..., columnType="integer") → throws - changeColumn(table=..., columnType="integer") → throws - addColumn(table=..., columnType="reference", referenceName="user") → does NOT throw The first two were the regression; the third pins the reference-type exemption so a future refactor can't silently re-introduce the unconditional-required pattern Reviewer B suggested. ## Verification bash tools/test-local.sh migrator: 234 pass / 0 fail (up 3 from 231 — the three new regression guards). All prior PR2 tests still pass. 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>
* refactor(cli): add advisory tier to `wheels upgrade check` scanner Generalizes the upgrade-check scanner so it can surface opt-in recommendations alongside breaking changes. Pure scaffolding — no new advisory entries are added in this PR (that's PR4 in the #2781 follow-up series). ## Changes **`severity` field on each check** (default `"breaking"`, new value `"advisory"`): - Existing breaking-change checks are unaffected (default applies). - New advisory checks bucket separately into a "Recommended Improvements" output section. **Same-major short-circuit removed** in `runUpgradeCheck()` — the `if (currentMajor == targetMajor) { ...; return ""; }` early return prevented advisory checks from firing on point-release upgrades. Now the message still prints, but the function continues into the checks loop so advisories run regardless of major-jump. **Three-section output**: - `Breaking Changes (N found):` — yellow, severity=breaking - `Recommended Improvements (N found):` — cyan, severity=advisory - `All Clear (N checks):` — green, unmatched ## Tests `cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc` — 6 new source-level specs (matching the ConsoleCommandSpec / ReloadCommandSpec pattern, since Module.cfc can't be instantiated under TestBox due to `modules.BaseModule` mapping not being available outside the LuCLI runtime). Verify: - severity field documented in comments - sameMajor refactor in place (no early return) - match buckets into issues vs advisories - all three output sections present CLI suite: 586 pass / 5 fail (same 5 pre-existing failures unrelated to upgrade — Admin/Doctor/Scaffold services). ## Scope (3 of 4 PRs from the #2781 audit) - ✅ #2802 — t.references() at TableDefinition (closes #2781) - ✅ #2804 — Migration.cfc command consistency - 🟢 This PR — wheels upgrade scanner advisory tier - 🔜 PR4 — specific advisory entries (underscore-references opt-in) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(cli): address Reviewer A/B consensus findings (round 1) - cli/lucli/Module.cfc: add `severity: "advisory"` to the RateLimiter middleware check struct (line ~3817) so the check buckets into "Recommended Improvements" instead of "Breaking Changes". The check's own description and inline comment already self-describe as advisory; with the new bucketing logic in this PR the missing field was a user-visible misclassification (Reviewer A primary blocker; Reviewer B confirmed against source). - cli/lucli/Module.cfc: add an inline comment near line 4025 documenting that advisory `fix` lines are intentionally uncolored — the section header and description already carry the cyan accent, so a future maintainer doesn't treat the missing color as an accidental omission (Reviewer A minor nit). - CHANGELOG.md: add a "[Unreleased] / Changed" entry covering the new optional `severity` field on each check, the `Recommended Improvements` output section, and the removed same-major short-circuit (Reviewer A docs gap, undisputed by Reviewer B). Reviewer B explicitly framed the same-major UX gap as non-blocking and "resolves itself when PR4 adds advisory entries outside version gates" — skipped here per the smallest-change principle. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.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 Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…check (#2807) * feat(cli): underscore-references opt-in advisories in `wheels upgrade check` Adds two advisory entries to the upgrade scanner, populating the "Recommended Improvements" section introduced in PR3 with the first concrete opt-in convention from the #2781 work. ## Advisories added **1. t.references() opt-in suggestion** — fires when any migration file under `app/migrator/migrations/` uses `t.references(`. Suggests setting `set(useUnderscoreReferenceColumns=true)` to opt into `<name>_id` naming matching Wheels model `belongsTo` defaults. Explicitly notes that already-applied migrations are unaffected (only new migrations adopt the new suffix), so users aren't alarmed about their existing schema. **2. Mixed-convention warning** — fires when `config/` contains `set(useUnderscoreReferenceColumns=true)`. Reminds the user that if this flag was flipped after some migrations were already applied, the database still has `<name>id` columns alongside any new `<name>_id` columns. Suggests writing a data migration for full consistency. Informational only — read once, ignore if N/A. Both advisories use `severity: "advisory"`, surfacing in the cyan "Recommended Improvements" section. They run regardless of major-version bump (same-major upgrades fire them too). Exit code stays 0. ## Tests `cli/lucli/tests/specs/commands/UpgradeAdvisorySpec.cfc` — 5 source-level specs verifying both advisory entries are present and reference the correct patterns, paths, and fix-message phrasing. CLI suite: 591 pass / 5 fail (same 5 pre-existing failures unrelated to upgrade). ## Scope (4 of 4 PRs from the #2781 audit) - ✅ #2802 — t.references() at TableDefinition (closes #2781) - ✅ #2804 — Migration.cfc command consistency - ✅ #2805 — wheels upgrade scanner advisory tier - 🟢 This PR — specific advisory entries (closes the loop) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(cli): address Reviewer A round-1 — advisory false-positive + comment-stripping PR4 round-1 review (Reviewer A) raised two real correctness issues plus a missing CHANGELOG entry. ## Correctness **1. Advisory ##1 fired even when the flag was already set.** New apps generated by `wheels new` ship with `useUnderscoreReferenceColumns=true` by default (from PR1), so the "opt in to underscore" advisory would contradict the mixed-convention warning on every fresh app. The check- struct schema doesn't support multi-condition logic natively, so this adds a pre-check on `config/settings.cfm` before appending advisory ##1 to the checks array. If the flag is already set, the advisory is suppressed. **2. The grep loop didn't strip CFML comments (Anti-Pattern ##14).** A commented-out `// t.references(...)` or `// set(useUnderscoreReference Columns=true);` would trip the pattern match. Both advisories ##1 and ##2 were affected — and so were all existing breaking-change checks. The shared grep loop now wraps `fileRead(filePath)` with `stripCfmlComments()` before `listToArray`. Multi-line block comments collapse and may shift reported line numbers — same tradeoff the existing `stripCfmlComments` callers at Module.cfc:970 and :5532 already accept. The pre-check guard on advisory ##1 also routes through `stripCfmlComments` so a commented-out `// set(...)` doesn't satisfy the guard and suppress a legitimate advisory. ## Tests `cli/lucli/tests/specs/commands/UpgradeAdvisorySpec.cfc` gains a new "false-positive guards" describe block with three source-level assertions: - `underscoreFlagAlreadySet` guard variable + the `if (!underscoreFlagAlreadySet)` conditional append - `stripCfmlComments(fileRead(filePath))` in the grep loop - `stripCfmlComments(fileRead(settingsFile))` in the pre-check ## Docs CHANGELOG.md `[Unreleased] / Added` gains an entry for the two advisory checks, the suppression logic, and the framework-wide comment-stripping improvement. ## Verification `bash tools/test-cli-local.sh`: 594 pass (3 new tests added), 5 fail (same pre-existing failures unrelated to upgrade). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(cli): drop dead len() wrapper from underscore-flag guard Reviewer A round-2 caught a real bug they introduced in round-1's suggestion (graciously acknowledged): `len(reFindNoCase(...))` is always truthy because reFindNoCase returns an integer position (0 = no match, >= 1 = match), and len() coerces the integer to a string and measures the digit count — len(0)=1, len(25)=2, both truthy. Result: the `underscoreFlagAlreadySet` guard was dead. Advisory #1 was never appended for any app whose config/settings.cfm exists (which is every Wheels app). The opt-in suggestion never reached the users it was meant to reach. ## Fix Drop the len() wrapper and use `> 0` directly on reFindNoCase: reFindNoCase(pattern, content) > 0 Unambiguous boolean: 0 > 0 is false, pos > 0 is true. Added an inline comment documenting the gotcha so a future maintainer doesn't reinstate the buggy wrapping. ## Regression test `UpgradeAdvisorySpec.cfc::does not wrap reFindNoCase with len() — that pattern is always truthy` — source-level assertion that the buggy pattern doesn't sneak back in. Pure structural guardrail appropriate for the constraints (Module.cfc can't be instantiated under TestBox). ## Verification `bash tools/test-cli-local.sh`: 595 pass (1 new regression-guard test), 5 pre-existing failures unrelated to upgrade. 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>
…g/ (#2809) * fix(cli): wheels upgrade underscore-flag pre-check scans all of config/ The advisory-1 suppression guard in `Module.cfc::runUpgradeCheck()` read only `config/settings.cfm`, but advisory ##2 below scans all of `config/` recursively. If a user set `useUnderscoreReferenceColumns=true` in an environment override (e.g. `config/production/settings.cfm`), the guard missed it, advisory ##1 fired telling them to opt in, and advisory ##2 also fired telling them the flag was already set — the contradiction the round-1 fix was meant to prevent. Walk `config/` recursively to match advisory ##2's `scanDir: "config"` scope. Comment-strip each file so commented-out flag lines cannot satisfy the guard (Anti-Pattern ##14). Fixes ##2808. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(cli): address Reviewer A/B consensus findings (round 1) - cli/lucli/tests/specs/commands/UpgradeAdvisorySpec.cfc:52 — update it() description from "in config/settings.cfm" to "anywhere in config/" so the spec label matches the broadened recursive scan scope shipped in c482150. Assertion bodies unchanged. 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>
…2789) (#2810) * fix(model): bypass nested cftransaction when migrator owns the outer (#2789) Model.create() / .update() / .deleteAll() inside a migration's up() or down() silently rolled back the row on MSSQL. The Migrator wraps every up() / down() invocation in its own outer cftransaction, and Wheels Model's default transaction="commit" opened a nested cftransaction whose JDBC nested-transaction semantics differ per adapter — most acutely on MSSQL where the inner commit didn't release the row and the eventual outer commit dropped it. Migrator.cfc now sets request.$wheelsTransactionWrapper = true around each up() / down() / migrateIndividual() invocation. Model::invokeWith- Transaction() reads that signal and treats the call as "alreadyopen", skipping the nested cftransaction entirely. The flag is request-scope and explicitly cleared in both the success and error paths of every migrator code path so it never leaks past the migration's outer transaction. Engine-agnostic — works the same way on MSSQL, MySQL, PostgreSQL, SQLite, Oracle, and H2. Specs: - vendor/wheels/tests/specs/model/OuterTransactionSignalSpec.cfc exercises invokeWithTransaction's response to the flag for create / save / deleteAll, plus a control test proving rollback still works when the flag is unset. - vendor/wheels/tests/specs/migrator/MigratorOuterTransactionSpec.cfc runs an end-to-end migration whose up() calls model().create() and asserts the row persists, plus assertions that the flag is set during up() and cleared after both up() and down(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(model): address Reviewer A/B consensus findings (round 1) - vendor/wheels/tests/specs/migrator/MigratorOuterTransactionSpec.cfc: Replace `var _isCockroachDB` (local scope, invisible to it() closures on Adobe CF — CLAUDE.md anti-pattern #10) with `var ctx = {isCockroachDB: ...}` and update the four closure guards to `if (ctx.isCockroachDB) return;`. - vendor/wheels/tests/_assets/migrator/migrations_2789/001_seed_tag_via_model_create.cfc: Switch `application.$issue2789FlagDuringUp` to `request.$issue2789FlagDuringUp`. The migration runs in the same HTTP request as the spec's migrateTo call, so request scope is sufficient and avoids global mutable state. Spec's beforeEach/afterEach + assertion updated accordingly. - vendor/wheels/Migrator.cfc, vendor/wheels/model/transactions.cfc: Trim the three multi-line comment blocks (up-path, down-path, migrateIndividual) and the 9-line block in invokeWithTransaction to one-liners per CLAUDE.md "one short line max" convention. Issue number preserved. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(test): address Reviewer A/B consensus findings (round 2) Trim the multi-line comment blocks Reviewer B's round-1 review caught in the test spec and migration fixture. CLAUDE.md: 'Never write multi-line comment blocks — one short line max.' Reviewer A flagged the identical pattern in production code during round 1 and the address-review trimmed Migrator.cfc / transactions.cfc, but the same violation persisted in the new test files. Reviewer B flagged that inconsistency in round 1; this addresses it. - vendor/wheels/tests/specs/model/OuterTransactionSignalSpec.cfc: trimmed the 9-line block in 'treats invokeWithTransaction as alreadyopen', the 2-line block in 'control:', the 3-line block in the save() case, and the 3-line block in the deleteAll case to one-liners each, anchored on issue #2789. - vendor/wheels/tests/_assets/migrator/migrations_2789/001_seed_tag_via_model_create.cfc: trimmed the 2-line block in down() to one line. Skipped (per consensus scope): - migrateIndividual() commit-after-rollback — Reviewer A and B both labelled pre-existing; recommended a follow-up issue. - variables.migration nit in MigratorOuterTransactionSpec.cfc — both reviewers labelled this a nit, not a blocker. Local: bash tools/test-local.sh model → 843 passed; migrator → 238 passed. Pure comment-trim with zero behavior delta. Signed-off-by: Peter Amiri <peter@alurium.com> * docs(test): explain why variables.migration is initialized in beforeAll Reviewer A/B agreed nit from round-1 review: variables.migration was bare-assigned in beforeAll() without explanation. The variable isn't referenced by any of this spec's describe/it blocks directly; it exists because helperFunctions.cfm engine-detection helpers (isDbCompatible, isDbCompatibleFor_SQLServer, etc.) call migration.adapter.adapterName() under the hood. Spare the next reader from wondering why the assignment is there. Also tightens the adjacent migrator-init comment to one line per CLAUDE.md's 'one short line max' rule. Follow-up tracked separately for the pre-existing migrateIndividual() commit-after-rollback both reviewers flagged out-of-scope: #2811. Local: bash tools/test-local.sh migrator → 238 passed. 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> Signed-off-by: Peter Amiri <petera@pai.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…#2812) * feat(migrator): t.primaryKey() columnName/columnNames aliases (#2803) Brings t.primaryKey() in line with every other column helper in TableDefinition.cfc. The PK helper was the last argument-naming outlier after #2781 fixed t.references(): it required `name` while siblings (`integer`, `string`, etc.) accept `columnNames` / `columnName` via $combineArguments. Users and AI tooling learning the convention from one helper kept mis-calling primaryKey(columnName=...) and hitting "required argument missing." Same shape as the #2781 fix: accept both modern aliases via two $combineArguments calls, with plural columnNames winning over singular columnName when both are passed (matches addReference / dropReference precedence in Migration.cfc). Legacy `name=` keeps working — it is still what the body reads and what TableDefinition.init() passes when adding the conventional `id` primary key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): address Reviewer A/B consensus findings (round 1) Addresses the four documentation/test gaps Reviewer A and Reviewer B converged on for #2812: - `vendor/wheels/migrator/TableDefinition.cfc::primaryKey()` — `@columnNames` annotation now explicitly warns that comma-separated lists do NOT iterate (unlike sibling helpers), producing a single literal column name. Pointers to call `t.primaryKey()` multiple times for composite PKs. - `vendor/wheels/tests/specs/migrator/primaryKeySpec.cfc` — added a negative-list spec proving `columnNames="a,b"` resolves to one PK column literally named `a,b`, making the documented single-column contract executable. - `CHANGELOG.md` — added `[Unreleased]` entry for #2803 alongside the existing #2781 `t.references()` entry, documenting the alias surface, precedence semantics, and the comma-list caveat. - `web/sites/guides/src/content/docs/v4-0-0/basics/migrations.mdx` — added `t.primaryKey()` row to the column-builder reference table, carrying the same comma-list caveat for users discovering the helper via the guide. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.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 Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…idual() (#2811) (#2813) * fix(migrator): skip transaction commit after rollback in migrateIndividual() (#2811) `migrateIndividual()` issued `transaction action="commit"` unconditionally after its try/catch — including on the error path, where the catch block had already issued `transaction action="rollback"`. Unlike `migrateTo()`'s loops, there is no enclosing `for` to `break` out of, so control always fell through to the spurious commit. On Lucee the second action against a closed transaction is a silent no-op. On Adobe CF 2023/2025 (and potentially BoxLang) the JDBC driver may throw a "transaction not active" error that masks the real migration failure, making the underlying problem harder to diagnose. Fix: return `local.rv` from inside the catch after the rollback. Mirrors the early-exit pattern in `migrateTo()` (which uses `break` to exit its for loop and skip the commit); here we use `return` because the `transaction { }` block is not enclosed in a loop. Adds a regression spec under an isolated fixture directory (_assets/migrator-error-path/) so the synthetic always-throw migration does not pollute the shared 001/002/003 fixtures used by every other migrator spec. Diagnosed by both round-1 reviewers on #2810 and explicitly recommended as a follow-up. See issue #2811 for the full diagnosis and pattern comparison against migrateTo(). Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): address Reviewer A/B consensus findings (round 1) - Strip issue-number prefix from the commit-skip comment in Migrator.cfc; preserve the WHY (rollback already closed the transaction; mirrors migrateTo()'s break) per CLAUDE.md "Don't reference the current task". - Strip the trailing "Issue #2811." from the synthetic fixture's hint attribute in 004_synthetic_failing_migration.cfc. - Strip the issue prefix from the spec's intro comment and from the describe label in MigrateIndividualErrorPathSpec.cfc. - Remove the duplicate afterEach block in MigrateIndividualErrorPathSpec.cfc — beforeEach already guarantees a clean state before every it(), so the afterEach was a source of confusion rather than additional safety. The var _isCockroachDB → struct-form consistency nit (Reviewer A finding 3 / Reviewer B convergence item) was explicitly described by both reviewers as lowest-priority "can be addressed separately if desired" and is left for a follow-up to keep this commit scoped to consensus-only changes. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.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>
…R2 (#2814) * feat(distribution): land Phase 2 native apt/yum repos via Cloudflare R2 Closes #2605. apt.wheels.dev and yum.wheels.dev are now live, signed, and serving from Cloudflare R2 buckets (wheels-apt / wheels-yum) with custom apex-domain bindings. Verified end-to-end: curl -fsSL https://apt.wheels.dev/wheels.gpg \ | sudo tee /usr/share/keyrings/wheels.gpg >/dev/null echo "deb [signed-by=/usr/share/keyrings/wheels.gpg] https://apt.wheels.dev stable main" \ | sudo tee /etc/apt/sources.list.d/wheels.list sudo apt update && sudo apt install wheels sudo dnf config-manager --add-repo https://yum.wheels.dev/wheels.repo sudo dnf install wheels GPG: signed with key 6872 16C9 32B4 9F03 94E0 9AED 5D89 AF8F 9C9B 8CFB (`Wheels Distribution <hello@wheels.dev>`), expires 2031-05-25. InRelease and repomd.xml.asc both verify locally against the published wheels.gpg. Architecture deviation from the original Phase 2 sketch: served from Cloudflare R2 instead of Cloudflare Pages. Pages enforces a 25 MiB per-file limit; the .deb is 80 MB and the .rpm is 81 MB. R2 has no object-size limit and supports custom-domain serving, so the URL UX is identical to what the original sketch promised. Changes in this commit: tools/distribution-drafts/apt-repo/ - workflows/wheels-released.yml — R2-backed receiver (sync from R2 → add new .deb → regen metadata → upload changed tree to R2 via wrangler r2 object put). Replaces "git commit + push → Pages auto-deploy" with a direct R2 upload path. - README.md — Pages → R2 architecture note + updated operational prereqs (CLOUDFLARE_API_TOKEN with Workers R2 Storage:Edit). tools/distribution-drafts/yum-repo/ - workflows/wheels-released.yml — R2-backed receiver, parallel structure to apt. - scripts/regenerate-yum-metadata.sh — rpm --addsign now reads the passphrase from a chmod-600 temp file via %__gpg_sign_cmd override; the previous gpg.conf "pinentry-mode loopback" only tells gpg "don't pop TTY" but doesn't supply the passphrase, so CI runs were failing with "cannot open '/dev/tty'". - README.md — Pages → R2. tools/distribution-drafts/linux-packages/README.md - Phase 2 section now reflects R2-backed serving + the secrets checklist gains CLOUDFLARE_API_TOKEN per bucket repo. web/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/installing.mdx web/sites/guides/src/content/docs/v4-0-1-snapshot/command-line-tools/installation.mdx - Lead with `apt.wheels.dev` / `yum.wheels.dev` sources.list / .repo setup. The "Trust model during Phase 1" Aside is replaced with a "GPG signing" Aside that names the fingerprint and explains the verification chain. - One-off GH-Release download retained behind a Tip Aside for air-gapped use. - Linux upgrade section shrinks to `apt upgrade wheels` / `dnf upgrade wheels` (one command, no version pinning needed). Operational state landed in parallel (not in this commit, outside the repo): GPG key in 1Password at op://Wheels/wheels-linux-repo-signing/; R2 buckets wheels-apt + wheels-yum on the wheels.dev account with custom domains apt.wheels.dev / yum.wheels.dev; bucket-repo secrets (WHEELS_REPO_GPG_PRIVATE_KEY, WHEELS_REPO_GPG_PASSPHRASE, CLOUDFLARE_API_TOKEN) set on wheels-dev/apt-wheels and wheels-dev/yum-wheels. Still outstanding: LINUX_REPO_DISPATCH_TOKEN on wheels-dev/wheels. The release workflow's dispatch step skips silently when the secret is unset, so the next release ships without auto-publish but the backfill path (`gh workflow run wheels-released.yml --repo <bucket>`) works. Signed-off-by: Peter Amiri <peter@alurium.com> * docs: flip remaining install guides to native apt/yum sources Completes the doc flip for #2605. Earlier commit handled the v4-0-1-snapshot install pages; this one covers the four files the first pass missed: - v4-0-0/start-here/installing.mdx — Linux tab flipped from "download GH Release + apt install ./file.deb" to "add the signed apt.wheels.dev source + sudo apt install wheels". Same for Fedora/RHEL via yum.wheels.dev. Upgrade section shrinks from "re-run install" to "apt upgrade wheels". - v4-0-0/command-line-tools/installation.mdx — same flip on the CLI-installation page. Adds GPG-signing Aside naming the fingerprint, and keeps the GH-Release download snippet behind a "Need a one-off download?" Tip for air-gapped use. - v4-0-0/start-here/release-channels.mdx — Linux per-channel install snippets and the four "switching channels" snippets (stable→BE × {apt,dnf}, BE→stable × {apt,dnf}) flipped to use the native sources. The switch flow becomes 2 commands (add source + install) instead of 4 (curl + apt install). - v4-0-1-snapshot/start-here/release-channels.mdx — same flip as v4-0-0. Stale text removed: - "Apt and yum repos at apt.wheels.dev / yum.wheels.dev are coming" (×2) - "Native apt/yum repositories ... are on the v4.0.x roadmap" (×2) - "Trust model during Phase 1" Asides (×2) - All "until the native repos are live" hedges Both v4-0-0 and v4-0-1-snapshot docs now describe the live architecture accurately. The v4-0-0 docs aren't being rewritten — they're being corrected: the original sentence "Native apt/yum is coming" describes a future that's now in the past. Signed-off-by: Peter Amiri <peter@alurium.com> * fix(docs): replace MDX-incompatible <https://...> autolinks with explicit link syntax The previous commit used GitHub-Flavored Markdown's "autolink" syntax (<https://apt.wheels.dev>) for bare URLs. MDX treats `<` as the start of a JSX tag and chokes: [@mdx-js/rollup] Unexpected character `/` (U+002F) before local name, expected a character that can start a name, such as a letter, `$`, or `_` file: web/sites/guides/src/content/docs/v4-0-0/command-line-tools/installation.mdx:54:73 That broke the `sites/guides` build, which broke the `Refresh visual baselines` workflow (and would have broken any other build-the-site job too). Fix: convert <URL> → [URL](URL) — the explicit Markdown link form — across all four mdx files I touched in the prior commit. Six total occurrences (apt.wheels.dev, yum.wheels.dev, two FINGERPRINT URLs × two file versions). Verified by grep: no <https://...> instances remain in web/sites/guides/src/content/docs/. Signed-off-by: Peter Amiri <peter@alurium.com> * fix(distribution): address Reviewer A/B consensus findings (round 1) Apply the converged findings from the Reviewer A/B exchange on #2814. - Replace `jq | while` and `find | while` piped loops with process substitution (`while ... done < <(...)`) in both apt and yum Sync + Upload steps. Pipe-RHS runs in a subshell where `set -e` in the parent does not see wrangler's non-zero exit, masking download/upload failures and producing trimmed indexes that silently overwrite Packages.gz / repomd.xml on R2. - Drop `>/dev/null 2>&1` from all wrangler r2 object get/put calls so a real failure surfaces in the CI log. - Add `jq -e '.success == true'` guard after the Cloudflare R2 list curl call in both Sync steps. When the token is invalid/expired or the API rate-limits, success=false comes back with an empty result array; without this check the sync succeeds with an empty local pool and metadata regen would overwrite the live index with a single-package tree. - Move the hardcoded `CLOUDFLARE_ACCOUNT_ID` literal out of the workflow env block to `${{ vars.CLOUDFLARE_ACCOUNT_ID }}`. Account IDs are not auth credentials, but keeping the workflow templates account-agnostic avoids handing a public-template harvester a ready-made API target. Each bucket repo sets the variable via `gh variable set CLOUDFLARE_ACCOUNT_ID --body '<id>' --repo ...`. Files touched: tools/distribution-drafts/apt-repo/workflows/wheels-released.yml tools/distribution-drafts/yum-repo/workflows/wheels-released.yml The PR-description claim that v4-0-0 docs are untouched (B's missed issue, finding 4) is a metadata correction, not a code change — it will be addressed in the PR description, not this commit. The CHANGELOG note (A's finding 4) was not part of the consensus (B refuted the CLAUDE.md-convention framing) and is skipped here. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Signed-off-by: Peter Amiri <peter@alurium.com> * docs(yum-script): clarify why rpm --addsign uses stdout-only suppression Reviewer A's round-2 nit: the `>/dev/null` on the rpm --addsign call inside regenerate-yum-metadata.sh is intentional (rpm's stdout is verbose progress chatter), but the rationale wasn't documented in the script. The round-1 review correctly noted that broadening it to `>/dev/null 2>&1` would silence stderr too and risk silent signing failures corrupting the repo. Add an inline comment that: - documents what the redirect actually suppresses (stdout progress chatter) - notes that stderr stays open so a signing failure still surfaces - explicitly warns a future editor not to broaden it to `2>&1` Signed-off-by: Peter Amiri <peter@alurium.com> * fix(web): authenticate sites/packages registry fetches to avoid 403 rate limit The packages.wheels.dev build (`sites/packages`) enumerates the package registry by calling the GitHub REST contents API at build time. Those fetches were unauthenticated, so they shared the 60 req/hour per-IP unauthenticated cap. Rapid CI activity on a single branch (PR #2814 saw ~6 pushes in an hour) exhausted the cap, and the build aborted mid-prerender with: Registry fetch failed: 403 rate limit exceeded — https://api.github.com/repos/wheels-dev/wheels-packages/contents/packages?ref=main That surfaced as a "Deploy packages" check failure unrelated to the PR's actual content. Fix: - registry.ts: send `Authorization: Bearer $GITHUB_TOKEN` (falls back to GH_TOKEN, then unauthenticated) on every registry fetch. Authenticated requests get 5000 req/hour instead of 60. Local/offline preview builds with no token keep working unauthenticated. - web-deploy.yml: pass `GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}` into the Build step env so the token is present in CI. Other site builds ignore the var. - registry.test.ts: add coverage for both the token-present (header sent) and token-absent (header omitted) paths. Verified locally: `pnpm --filter @wheels-dev/site-packages test` → 18 passed; `GITHUB_TOKEN=$(gh auth token) pnpm --filter @wheels-dev/site-packages build` → 7 pages built, registry fetch succeeded. 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>
Module.cfc's t.references() underscore advisory ended its fix message with "See #2781." — an unescaped # opens a CFML interpolation that never closes, so Lucee threw ComponentTemplateException "Invalid Syntax Closing [#] not found" at [3949:260] and refused to compile Module.cfc. That broke `wheels new` and every CLI command that boots the module on develop and the bleeding-edge channel. The source-level CLI specs fileRead Module.cfc rather than compile it, so they stayed green; only the installed-distribution smoke test in snapshot.yml caught it. Escape to "See ##2781." — the literal-# form already used at Module.cfc:2176. Verified: tools/test-onboarding.sh (wheels new exits 0, 42/0) and tools/test-cli-local.sh (605 pass / 5 pre-existing Admin/Doctor/Scaffold fails, no new failures). Introduced by #2807. Signed-off-by: Peter Amiri <peter@alurium.com>
…leased] (#2816) Entries in [Unreleased] cited issue numbers (or earlier stacked-PR numbers) rather than their merge-PR numbers, and six entries carried no reference at all. Correct 18 references to the actual merge PRs and add the five missing user-facing entries: #2814 (native apt/yum repos), #2804 (Migration.cfc command alias sweep), #2813 (skip spurious commit after rollback in migrateIndividual), and #2777 / #2801 (Linux install URL and migrator-table doc fixes). No existing prose changed beyond the trailing reference, and nothing was misfiled in the 4.0.1 section (verified by grep). Internal-only CI/chore/blog PRs (#2797, #2772, #2771, #2770) and the user-invisible bucket rename (#2788) are intentionally omitted. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BoxLang: Global.cfc pseudo-constructor `local.varKey` materialized variables.local, shadowing method-local `local` on BoxLang (89 failures). Adobe CF 2023/2025: InvokeMethodSpec leaked the welcome page into the response buffer (HTTP 404 + corrupted JSON); RequestId param scope-shadow; empty-body cfhttp POST; ParallelRunner array-by-value; $reincludeGlobals double-include; RewriteConfigInstaller fileWrite newline; OuterTransaction- SignalSpec cf_sql_integer overflow on CockroachDB unique_rowid. Verified green across all engines × databases (Oracle remains soft-fail).
…ybook (#2818) Prep for the 4.0.2 cut. - CHANGELOG.md: the separator below [Unreleased]/[4.0.2] was `----` (4 dashes). release.yml extracts release notes with the awk range /^# \[VERSION\]/,/^---$/, which terminates only on exactly three dashes, so a 4-dash separator silently pulls all of 4.0.1's notes into the 4.0.2 release. Flipped to `---`. (Same regression as #2606, #2768.) - release.yml: added a main-gated guard in the release-checklist step that mirrors the extraction awk and fails the release if the range bleeds into a later version heading, so this stops recurring. - RELEASE_PLAYBOOK.md: corrected stale guidance — version source is wheels.json (not box.json) since the 4.0 rebrand, and the release-day flow is PR-into-main + softprops auto-tag (main's PRs-only ruleset rejects `git push origin main --tags`). Documented the new separator guard and its failure mode. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Promote `# [Unreleased]` to `# [4.0.2] => 2026-05-27` so release.yml's release-checklist step (which refuses to publish on `=> TBD`) and its `awk '/^# \[VERSION\]/,/^---$/'` release-notes extraction both pick up the section. Add the 4.0.2 summary blockquote. The `----`->`---` section terminator and the awk-bleed guard already landed in #2818, so only the heading promotion is needed here. `wheels.json` is already at `4.0.2` (auto-bumped by bump-develop-version.yml after the 4.0.1 GA), so no version bump. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
) The 4.0.2 release push (#2819) failed before any step ran: the pre-hooks of Wandalen/wretry.action@v3 -- the lone Node 20 action in the job, wrapping softprops/action-gh-release -- threw "Argument list too long" spawning the node20 binary. Root cause is GitHub's forced Node 20 -> 24 runner migration (forced default 2026-06-02), not workflow config: run #2769 (4.0.1) passed 7 days earlier with the identical setup, and the develop snapshot path (same action) succeeded 16 min before on a pre-migration runner. - Set FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true at workflow level so all JS actions (including wretry@v3) run on Node 24, bypassing the broken node20 launch path. This is GitHub's documented remedy for the transition. - Add a workflow_dispatch trigger so a stuck/failed release can be re-fired from the Actions tab without a fresh push. Channel selection keys on GITHUB_REF: dispatch against main -> stable, develop -> snapshot. Merging to main re-fires release.yml and completes the v4.0.2 release; prior failed attempts created no tag/release, so there is no double-publish risk. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reconciles develop's release.yml with the main hotfix (#2820) and corrects the dispatch-token permission docs. - Port FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 + workflow_dispatch from #2820 (currently main-only). Without this, the next develop->main release would merge develop's older release.yml over main's and silently drop the Node 24 fix, so 4.0.3 would hit the same "Argument list too long" failure that blocked 4.0.2 (#2819). This makes develop a superset of main, so future release merges stay conflict-free. - Fix token-permission comments (actions: write -> contents: write) for both DOWNSTREAM_DISPATCH_TOKEN and LINUX_REPO_DISPATCH_TOKEN. POST /dispatches needs contents: write for fine-grained PATs; the mis-documented permission is why LINUX_REPO_DISPATCH_TOKEN was scoped actions: write and 403'd on the 4.0.2 apt/yum dispatch. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) softprops/action-gh-release defaults target_commitish to the repo default branch (develop) when unset, so the stable v4.0.2 tag was created on develop's HEAD (off-main) instead of the released commit -- it had to be force-moved manually after the fact (#2819). Pin it to github.sha on the stable Create-GitHub-Release step so the tag always lands on the exact commit built. Stable step only: the snapshot step publishes to wheels-dev/wheels-snapshots, where a wheels-repo SHA wouldn't resolve. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 4.0.2 cut renamed [Unreleased] -> [4.0.2] on the release branch only (#2819 was squash-merged to main), so develop still carried the 4.0.2 entries under [Unreleased]. Left as-is, the 4.0.3 cut would rename those same entries to [4.0.3] and re-publish already-shipped changes. Promote them to [4.0.2] to match main; develop's CHANGELOG is now identical to main's. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ase playbook (#2893) Learned at the 4.0.3 cut (#2892): - The 'merge with a merge commit' step silently depended on the repo setting 'Allow merge commits', which had been switched to squash-only between the 4.0.1 and 4.0.2 cuts — forcing #2819 to squash and poisoning the next promotion with 24 spurious conflicts. The setting is enabled again (permanently, per maintainer decision 2026-06-09); the playbook now says so explicitly. - Document the '-s ours' healing recipe for promoting develop over a diverged main, including the must-be-empty tree-diff verification. - Promote the CHANGELOG rename on develop BEFORE cutting the release branch (the 4.0.3 flow, #2891) so develop doesn't need a back-port like #2824. - Fix dispatch-list drift: chocolatey-wheels was retired for scoop-wheels, and the apt/yum bucket dispatches were missing from the trigger list. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
| Back | FazBrowse Home | New Git URL |
Summary
Cuts Wheels 4.0.2 — promotes [Unreleased] → [4.0.2] => 2026-05-27 and brings all develop work since v4.0.1 to main (~30 PRs).
Highlights:
⚠️ Merging this triggers the release
Merge with "Create a merge commit" (not squash — preserves develop history). The push to main fires release.yml:
Do not merge until ready to ship.
Post-merge follow-ups
🤖 Generated with Claude Code