fix(model): hasMany shortcut no longer breaks the association via include expansion

[wheels-bot]

Summary

Declaring shortcut on a hasMany association poisoned every call path through that association. The hasMany shortcut argument defaults through to an opposite-side chain ("#singularize(shortcut)#,#name#", e.g. "role,userRoles") that is consumed by the shortcut dispatcher in $associationMethod. But $expandThroughAssociations treated any 2-element through as a this-model through-include and unconditionally rewrote the include (e.g. "userRoles" → "role(userRoles)"). The include parser then looked up association role on the current model, which doesn't exist, and threw Wheels.AssociationNotFound — breaking the plain association method (user.userRoles()), the shortcut method (user.roles()), and findAll(include="userRoles").

The fix gates the 2-element branch on its first segment actually being an association on the current model (mirroring the existence check already present in the 1-element branch). The shortcut default never satisfies that, so it now falls through unchanged — restoring pre-6579f246c behaviour for shortcuts while preserving the nested-join expansion added for #449.

Related Issue

Fixes #3109

Type of Change

• Bug fix

Feature Completeness Checklist

• DCO sign-off -- commit carries Signed-off-by:
• Tests -- vendor/wheels/tests/specs/model/hasManyShortcutSpec.cfc (failing → passing) with a new Member/MemberTeam/Team HABTM fixture covering the plain association method, the shortcut method (far-side row correctness), include expansion, and the $expandThroughAssociations rewrite directly
• Framework Docs -- left for bot-update-docs.yml (the documented many-to-many pattern in basics/associations.mdx depends on this fix)
• AI Reference Docs -- left for bot-update-docs.yml
• CLAUDE.md -- left for bot-update-docs.yml
• Changelog fragment -- changelog.d/3109-hasmany-shortcut.fixed.md
• Test runner passes -- see below

Test Plan

Ran the framework model suite against Lucee 7 + SQLite:

• New spec alone (before fix): 0 pass, 1 fail + 3 errors — first assertion Expected [memberTeams] but received [team(memberTeams)]; the other three threw association named \team` could not be found on the `member` model` (the reported bug, reproduced).
• New spec alone (after fix): 4 pass, 0 fail.
• Full model layer (wheels.tests.specs.model) after fix: 897 pass, 0 fail, 0 error (11 skipped, 908 specs) — no regressions.

The change uses only cross-engine-safe builtins (StructKeyExists, ListFirst, ListLast, ListAppend); CI's compat-matrix will confirm Adobe/BoxLang.

Note: the bot sandbox has no wheels binary on PATH, so tools/test-local.sh could not be invoked; the suite was run against the already-running Lucee 7 server on port 60007 via /wheels/core/tests, which is what that script ultimately hits.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: This PR fixes #3109 by gating $expandThroughAssociations's 2-element through rewrite on the first segment actually being an association on the current model (vendor/wheels/model/sql.cfc:1213). I verified the diagnosis independently and it holds: hasMany defaults through to "#singularize(shortcut)#,#name#" (vendor/wheels/model/associations.cfc:70) and documents it as "a list of two association names representing the chain from the opposite side of the many-to-many relationship to this model" (associations.cfc:59), and the shortcut dispatcher consumes that chain on the join model via ListFirst/ListLast (vendor/wheels/model/onmissingmethod.cfc:286-291) — so the unconditional rewrite introduced in 6579f246c (#449) was wrong for every conventional shortcut, exactly as the PR body claims. Verdict: comment — no blocking findings; one non-blocking test-coverage note below.

Correctness

Verified, no findings:

• The gate is the right discriminator between the two through shapes. A this-model chain (the #449 nested-join case) by definition has its first segment registered on the current model, so it still expands; a shortcut's opposite-side chain (e.g. "team,memberTeams" on Member) never does, so it now falls through unchanged. I attempted to refute this with the "model coincidentally has an association named after singularize(shortcut)" edge and concluded that case is precisely the configuration #449's expansion was written for — the gate preserves it rather than breaking it.
• The fallback branch appends local.currentInclude (sql.cfc:1218), consistent with the function's other as-is fallbacks at sql.cfc:1194-1198.
• Plain hasMany without shortcut stores through = ",name" (leading empty list element), which ListLen counts as 1, so it routes to the 1-element branch and falls through as before — no behavior change outside the shortcut path.

Tests

• The new spec (vendor/wheels/tests/specs/model/hasManyShortcutSpec.cfc) covers the bug well: the $expandThroughAssociations rewrite directly, the plain association method, findAll(include=...), and far-side row correctness through the shortcut — and the PR body documents the failing-before/passing-after run. Good.
• Non-blocking: the preserved branch — a 2-element through whose first segment IS a current-model association (sql.cfc:1213-1215, the #449 path) — still has zero coverage: no fixture under vendor/wheels/tests/_assets/models/ declares such a chain (grep -rn "through" vendor/wheels/tests/_assets/models/ matches only a comment in the new Member.cfc). This gap predates the PR, but since this change touches exactly that branch's guard, a follow-up fixture exercising the gate-true path would lock in that #449's expansion survived. Fine to defer to a separate issue.

Conventions / Cross-engine / Docs / Commits — all clean

• Fix uses only safe builtins (StructKeyExists, ListFirst, ListLast, ListAppend); $expandThroughAssociations keeps public + $-prefix (Cross-Engine Invariant 7).
• Spec follows house patterns: g = application.wo preamble (matches crudSpec.cfc:5), arrow-function BDD on wheels.WheelsTest, and ##3109 correctly escaped in the describe-string (unescaped # appears only in // comments, which is safe).
• populate.cfm fixture data uses bare model().create() — established pattern in that file (lines 462+); new tables use the shared local.identityColumnType / local.storageEngine machinery and the join table is first in the drop list.
• Changelog fragment changelog.d/3109-hasmany-shortcut.fixed.md present with a valid fixed type; no direct CHANGELOG.md edit.
• Single commit conforms to commitlint (fix(model): …, header < 100 chars, body explains the why, DCO sign-off matches the author email). Docs checkboxes honestly deferred to bot-update-docs.yml.

[wheels-bot]

Wheels Bot — Docs updated

Added a doc commit to this PR:

• web/sites/guides/src/content/docs/v4-0-0/basics/associations.mdx — Fixed the many-to-many code examples: both User and Role examples were showing two hasMany declarations with the same name (the second silently overwrites the first). A single hasMany(name=..., shortcut=...) declaration is all that is needed — it provides both the plain association method (user.userRoles()) and the shortcut method (user.roles()). Also updated the prose to reflect the corrected single-declaration pattern and noted that include works correctly alongside a shortcut declaration (the bug this PR fixes).

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: This head adds one commit on top of the previously-reviewed fix for #3109: a web/sites/guides correction removing the redundant (and pre-fix, broken) duplicate hasMany(name="userRoles") declaration from the many-to-many guide. The code fix is byte-identical to the head I reviewed at e7d5f8ce2 (no blocking findings there — re-verified below), and the docs change is accurate. However, the new commit's header is 104 characters, exceeding commitlint.config.js's header-max-length: [2, 'always', 100]. Per the repo's commit rules that is a blocking finding, so the verdict is request changes — the fix is a one-line reword/amend. (Submitted as a comment-state review because GitHub rejects a changes-requested state on the bot's own PR; please treat it as request-changes.)

Correctness

No findings. Re-verified at this head:

• The gate at vendor/wheels/model/sql.cfc:1212 (StructKeyExists(local.associations, local.firstAssociation)) correctly discriminates a this-model through chain (the #449 nested-join case, still expanded) from a shortcut's opposite-side chain, which hasMany defaults to "#singularize(shortcut)#,#name#" (vendor/wheels/model/associations.cfc:71) and documents as the chain "from the opposite side" (associations.cfc:59) — that chain's first segment is never a current-model association, so it now falls through unchanged.
• The updated guide text is factually correct against the fixed behavior: a single hasMany(name="userRoles", shortcut="roles") yields both user.userRoles() (plain association method) and user.roles() (shortcut dispatcher), and findAll(include="userRoles") no longer throws — each path is exercised by vendor/wheels/tests/specs/model/hasManyShortcutSpec.cfc. The old example's two same-name hasMany declarations would have overwritten each other anyway, so the removal also fixes a latent doc bug.

Tests

• Coverage of the bug itself is good (expansion unit check, plain method, include, far-side shortcut rows) — unchanged from the prior review.
• Non-blocking, carried forward: the gate-true path — a 2-element through whose first segment IS a current-model association (sql.cfc:1212-1214, the #449 expansion) — still has no fixture: git grep -rn "through" vendor/wheels/tests/_assets/models/ matches only a comment in the new Member.cfc. Fine to defer to a follow-up issue, as the prior review said.

Docs

• The guide edit is complete: both example blocks (User side at associations.mdx:235-239 and Role side at associations.mdx:257-262) were fixed, and no other hasMany(name="userRoles") duplicate remains anywhere in web/, .ai/, or CLAUDE.md (grep clean).
• Nit: the PR body's checklist still says "Framework Docs — left for bot-update-docs.yml", but commit 095d36fe9 updated web/sites/guides/src/content/docs/v4-0-0/basics/associations.mdx in this very PR. Please tick that box / update the note so the checklist reflects reality.

Commits

• Blocking — commitlint header-max-length: commit 095d36fe9's header is 104 characters:

  docs(web/guides): fix many-to-many shortcut example — one hasMany declaration provides both access paths

  commitlint.config.js sets 'header-max-length': [2, 'always', 100] (error level), and CLAUDE.md § Commit Message Conventions names the config as canonical ("header ≤ 100 chars"). CI's pr.yml only lints the PR title, so this won't fail a check, but the convention is uniformly observed in practice — 0 of the last 300 commits on develop exceed 100 characters. Fix: reword the subject to ≤ 100 chars and force-push, e.g.:

  docs(web/guides): single hasMany with shortcut provides both access paths (74 chars)

• The fix commit e7d5f8ce2 remains conformant (84-char header, fix(model) type/scope, body explains the why, DCO sign-off matches the author email). The new commit's DCO sign-off also matches its author (claude[bot] <41898282+claude[bot]@users.noreply.github.com>).

Conventions / Cross-engine / Security — all clean

Unchanged from the prior review at e7d5f8ce2: safe builtins only (StructKeyExists, ListFirst, ListLast, ListAppend), public + $-prefix preserved on $expandThroughAssociations (Cross-Engine Invariant 7), ##3109 correctly escaped in the spec's describe string, fixture tables use the shared local.identityColumnType / local.storageEngine machinery, changelog fragment present with a valid fixed type, no direct CHANGELOG.md edit, no user-input paths touched.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: This head adds two substantive commits on top of the previously-reviewed state: a093c31dc closes both test-coverage gaps the earlier reviews carried forward (the explicit through= override form and the gate-true #449 rewrite path), and e0df81a6b removes the now-contradictory "shortcut is broken" caution Aside that arrived via the develop merge. All new code is correct, cross-engine-safe, and well-evidenced — but the blocking commitlint finding from the previous review is still unresolved: commit 095d36fe9's header remains 104 characters (> 100), unchanged at this head. Per the repo's commit rules that stays a blocking finding, so the verdict is request changes. It is resolvable without touching any code: reword the one subject line, or squash-merge the PR under its (conformant, 85-char) title. (Submitted as a comment-state review because GitHub rejects a changes-requested state on the bot's own PR — please treat it as request-changes.)

Correctness

No findings. Verified for the new content at this head:

• Explicit-override fixtures are wired correctly. Member.cfc declares hasMany(name="rosterSpots", modelName="MemberTeam", shortcut="squads", through="squad,rosterEntries"); the chain matches the dispatcher's contract in vendor/wheels/model/onmissingmethod.cfc:286-291 — ListFirst(through) ("squad") must be an association on the join model (MemberTeam.cfc adds belongsTo(name="squad", modelName="Team", foreignKey="teamid")) and ListLast(through) ("rosterEntries") must be an association on the far-side model (Team.cfc adds hasMany(name="rosterEntries", modelName="MemberTeam")). Walking it by hand: alice.squads() → findAll on Team, include="rosterEntries", where memberid = <alice> → Red, Blue — exactly what the spec asserts (ListSort(...) toBe "Blue,Red").
• The gate-true fixture exercises the preserved #449 branch for real. Team.squadMembers declares through="memberTeams,member" where memberTeams IS an association on Team, so $expandThroughAssociations("squadMembers") takes the IF-side at vendor/wheels/model/sql.cfc:1213-1215 and rewrites to "memberTeams(member)"; both nested segments resolve (memberTeams hasMany on Team, member belongsTo on MemberTeam). The row-count expectation (3) is right: each team carries exactly one join row, as the spec comment notes.
• The Aside removal is correct, not premature. The caution block removed from associations.mdx (formerly lines 232-234) says "Track the fix in [#3109]" — this PR is that fix, and the prose directly below it now documents the working single-declaration behavior, each path of which is asserted in hasManyShortcutSpec.cfc. Keeping the Aside would have shipped a self-contradicting page. The final file state is clean — no remaining shortcut/broken caution (grep over associations.mdx at this head).
• $expandThroughAssociations remains public with a string return (sql.cfc:1140), so the spec's direct string assertions (toBe("rosterSpots"), toBe("memberTeams(member)")) match the actual contract.

Tests

• Both carried-forward gaps are now closed. The prior reviews noted the gate-true path and the explicit-override form had no fixtures; this head adds direct unit assertions on the rewrite for all three shapes (conventional shortcut default, explicit override, this-model chain) plus end-to-end specs (plain method, include, far-side shortcut rows) for each. The commit body documents red-checks for both additions (1 fail + 3 errors pre-fix for the override form; both #449 specs fail with the if-branch disabled), satisfying the failing-first expectation.
• g = application.wo at the top of run() and bare model() calls in populate.cfm are both established suite patterns (e.g. vendor/wheels/tests/specs/view/html5FormHelpersSpec.cfc:10, populate.cfm:462 onward) — not findings.
• Fixture hygiene checks out: no name collisions in _assets/models/, the new tables reuse the shared local.identityColumnType / local.intColumnType / local.storageEngine machinery, and c_o_r_e_memberteams is prepended to the delete list ahead of c_o_r_e_members / c_o_r_e_teams (join table dropped first).

Docs

• Nit, carried forward and still stale: the PR body's checklist says "Framework Docs — left for bot-update-docs.yml", yet the PR now contains two guide commits touching web/sites/guides/src/content/docs/v4-0-0/basics/associations.mdx. The Test Plan section is similarly stale ("New spec alone (after fix): 4 pass" — the spec now contains 8 specs). Please refresh the PR body so the audit trail matches the diff.

Commits

• Blocking — commitlint header-max-length, unresolved from the previous review: commit 095d36fe9's header is 104 characters:

  docs(web/guides): fix many-to-many shortcut example — one hasMany declaration provides both access paths

  commitlint.config.js:27 sets 'header-max-length': [2, 'always', 100] (error level), and CLAUDE.md § Commit Message Conventions names the config as canonical. The previous review flagged exactly this and proposed a ≤100-char reword; this head adds commits on top instead of amending. Two acceptable resolutions: (1) reword the subject and force-push (now requires rebasing across the 5343717bb merge), or (2) squash-merge the PR — the PR title (fix(model): hasMany shortcut no longer breaks the association via include expansion, 85 chars) is conformant and would become the only commit header on develop. If maintainers intend (2), say so on the thread and this finding is satisfied at merge time.

• The two new substantive commits are conformant: a093c31dc (test(model): …, 85 chars, body explains the why and documents the red-checks) and e0df81a6b (docs(web/guides): …, 78 chars, body explains the develop-merge interaction). Both carry Signed-off-by: Peter Amiri <peter@alurium.com> matching the author email (DCO ✓). 5343717bb is a merge commit, which commitlint's default ignores exempt.

Conventions / Cross-engine / Security — all clean

• All-named arguments throughout the new fixtures (hasMany(name=..., modelName=..., shortcut=..., through=...)) — no mixed styles.
• Cross-engine: safe builtins only; no inline-closure constructor args, no Left(str, 0), no bracket-notation calls, no reserved-scope names (squad, teams, g are all safe); ##3109 / ##449 correctly escaped inside describe strings (the unescaped-# suite-killer is avoided); $expandThroughAssociations keeps public + $ prefix (Cross-Engine Invariant 7).
• Changelog fragment changelog.d/3109-hasmany-shortcut.fixed.md present with a valid fixed type; no direct CHANGELOG.md edit.
• No user-input paths touched; spec where strings are literal test data.