[bpamiri]

Summary

Resolves #2568 — core test suite crashes on Adobe ColdFusion 2023/2025 with java.lang.ArrayStoreException: coldfusion.compiler.ASTcffunction. Every database matrix cell (mysql/postgres/sqlserver/cockroachdb/oracle/sqlite) on adobe2023 and adobe2025 has returned HTTP 500 for the last 5+ weekly compat-matrix runs.

Root cause: vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc passed 12 inline keyFunction = function(req) { ... } literals as named arguments to new wheels.middleware.RateLimiter(...). Adobe CF's bytecode generator (ExprAssembler.invokeNew → generateSetVarCode → assignStatement) rejects function-AST nodes in that array slot. The failure fires from getComponentMetadata() in TestBox, and because Adobe CF eagerly compiles every CFC in the bundle directory before running, one bad spec crashes the entire core test suite for that engine.

Fix: Hoist all 12 inline closures into var keyFn = ...; declarations above the new call. Matches the existing workaround in SessionStrategySpec.cfc. Identical to the unmerged May 3 commit (2c329d4cb on the abandoned branch claude/loving-chaplygin-fa1988) — this PR cherry-picks just the surgical RateLimiterSpec portion and adds documentation so the trap doesn't recur.

Related Issue

Closes #2568

Type of Change

• Bug fix
• New feature
• Enhancement to existing feature
• Documentation update
• Refactoring

Feature Completeness Checklist

• DCO sign-off — every commit carries Signed-off-by: Peter Amiri <peter@alurium.com>
• Tests — this PR is a test fix; existing assertions cover behavior unchanged
• Framework Docs — N/A (no public API change; user-facing rate-limiter docs unchanged)
• AI Reference Docs — new section in .ai/wheels/cross-engine-compatibility.md documenting the trap with WRONG/RIGHT examples and the ExprAssembler why
• CLAUDE.md — added to "Known cross-engine gotchas" list
• CHANGELOG.md — entry under [Unreleased] → Fixed
• Test runner passes — pending compat-matrix verification on adobe2023/adobe2025 (manually dispatched on this branch: run 25710121789)

Test Plan

• PR CI green (Lucee 7 + SQLite + Browser)
• Manual compat-matrix dispatch on this branch green on adobe2023 + adobe2025 across all databases — direct verification this fix resolves the reported error
• Lucee 6/7 and BoxLang remain green (no behavior change expected; both compile both forms identically)
• Issue Core tests fail on Adobe ColdFusion with coldfusion.compiler.ASTcffunction error in TestBox metadata loading #2568 reporter (zainforbjs) can verify Adobe CF test runs no longer crash

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR correctly fixes the Adobe CF ASTcffunction ArrayStoreException crash by hoisting all 12 inline keyFunction = function(...) literals in RateLimiterSpec.cfc out of the new wheels.middleware.RateLimiter(...) constructor calls, and adds accurate cross-engine documentation. The fix is mechanically sound and Lucee/BoxLang-safe. Two issues block merge: a missing DCO Signed-off-by: trailer in the commit (required CI status check) and a missing CHANGELOG.md entry required by the PR template. One minor nit about inline comment placement.

---

Correctness

No issues. All 12 keyFunction sites now pass a pre-declared var keyFn reference instead of an inline literal, matching the precedent already set in SessionStrategySpec.cfc. A grep of the final file confirms zero remaining keyFunction = function( patterns. The documentation additions in .ai/wheels/cross-engine-compatibility.md (lines 67–88) and CLAUDE.md are accurate and well-placed — the why/how/reference examples are all present.

---

Conventions

Nit — comment present on only one of 12 hoisted sites.

vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc, lines 491–492 (added):

// Adobe CF: hoist the function literal out of `new` to dodge
// ASTcffunction ArrayStoreException at compile time.
var keyFn = function(req) { return longKey; };

The other 11 hoisted sites are uncommented. Since the pattern is now documented globally in .ai/wheels/cross-engine-compatibility.md and CLAUDE.md, the inline comment is not strictly required — and inconsistently applying it to only one site is mildly confusing. Suggestion: either remove the inline comment entirely (the docs are the canonical reference) or add a single file-level comment near the top of the relevant describe block covering all sites. Not blocking.

---

Commits

Blocking — DCO Signed-off-by: trailer is absent.

git show 67248c47fd22a8ef3a0f4771e2dbbd0e41aca931 --format="%B" | grep "Signed-off-by"
→ (empty)

The commit ends with Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> but carries no Signed-off-by: Peter Amiri <peter@alurium.com> trailer. Per CONTRIBUTING.md § DCO and the PR template checklist item 1, every commit must carry this trailer. The DCO GitHub App is a required status check and will block merge. Fix with:

git commit --amend -s   # appends Signed-off-by automatically from git config
git push --force-with-lease

The rest of the commit message is well-formed: type fix, scope test, subject ≤ 100 chars, not ALL-CAPS, body explains the why clearly.

---

Docs

Minor — no CHANGELOG.md entry under [Unreleased].

The PR template (.github/pull_request_template.md, line 26) requires a CHANGELOG entry for every PR. Fixing a crash on adobe2023/adobe2025 that caused every database to return HTTP 500 on those engines is notable enough to warrant a Fixed entry, even though it is test-only. Suggested addition under the Fixed subsection of [Unreleased]:

- Fix Adobe CF `ASTcffunction ArrayStoreException` crash in `RateLimiterSpec` caused by
  inline function literals as named constructor arguments; hoist pattern now documented in
  cross-engine compatibility guide (#2568, #2599)

The PR description also replaced the standard template checkboxes (DCO, Tests, AI Ref Docs, CLAUDE.md, CHANGELOG, test runner) with a freeform summary, making completeness harder to verify at a glance.

---

[bpamiri]

@wheels-bot — addressing the three items.

1. DCO sign-off (blocking) — fixed.

$ git log -1 --format='%B' | tail -2
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

Amended via git commit --amend -s --no-edit and force-pushed with --force-with-lease. New SHA: b74577d0d.

2. CHANGELOG.md entry (minor) — added.

Entry placed at the top of [Unreleased] → Fixed, with context covering the ExprAssembler.invokeNew mechanism, the eager-compile blast radius, the workaround, and the cross-references to the documentation updates. (#2568, #2599).

3. PR description template — updated.

Reformatted to follow .github/pull_request_template.md structure: Summary / Related Issue (Closes #2568) / Type of Change / Feature Completeness Checklist / Test Plan. Checkboxes marked per-item with reasoning for the N/A entries (Framework Docs — no public API change; Test runner — pending compat-matrix verification on the manually-dispatched run on this branch).

4. Inline comment nit — keeping as-is.

The comment at line 491–492 matches the precedent in commit 2c329d4cb (the May 3 RateLimiterSpec fix on the abandoned claude/loving-chaplygin-fa1988 branch). It sits at the first hoist site so someone reading the spec top-to-bottom encounters the rationale once before the pattern repeats. The cross-engine doc is the canonical reference for the global pattern, but the inline comment is a local guard rail for anyone debugging this specific file without that context. Removing 4 lines to silence inconsistency would lose the rationale anchor; making it file-level moves it away from the first occurrence. Both alternatives are weaker than the existing precedent.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is sound. The DCO finding is confirmed, the correctness claims hold up against the diff, and the nit about comment inconsistency is accurate. One minor overstatement on the CHANGELOG requirement, no false positives on the code itself, and no missed issues.

Sycophancy

None detected. Every claim in the Correctness section is backed by diff evidence. The 12-site count is accurate.

False positives

Minor — CHANGELOG requirement overstated. A cites the PR template's checklist as requiring a CHANGELOG entry, but the checklist header reads "All items must be checked for new features and enhancements." This is a bug-fix PR, not a feature or enhancement. The CHANGELOG requirement as stated is not strictly mandatory per the template. That said, the ### Fixed subsection under [Unreleased] in CHANGELOG.md is populated with other test/fix entries, and a crash that took down both Adobe CF engines across all databases is notable enough that adding an entry is reasonable regardless — A's suggestion is pragmatically sound even if the "required" framing is a stretch.

No false positives on the code claims. A grep of other spec files confirms no remaining new Component(namedArg = function(){...}) patterns anywhere in vendor/wheels/tests/specs/ — the fix scope is complete.

Missed issues

None detected. All 12 hoisted sites are in RateLimiterSpec.cfc. SessionStrategySpec.cfc was already clean. No other affected files exist.

Verdict alignment

CHANGES_REQUESTED is correct. The DCO Signed-off-by: trailer is confirmed absent from commit 67248c47, and the DCO GitHub App is a required status check that will block merge. The verdict is consistent with the findings.

Convergence

Aligned. A's CHANGES_REQUESTED verdict stands: the DCO fix is the hard blocker (contributor needs to amend and force-push), a CHANGELOG entry under ### Fixed is a reasonable addition, and the single-site inline comment should be removed for consistency. No disagreement on any code-level claim.

[bpamiri]

Merge conflict resolved.

#2597 landed on develop just before my rebase and also added an entry to [Unreleased] → Fixed, so CHANGELOG.md collided. Rebased onto origin/develop, kept both entries (mine on top, #2597's directly below), preserved the Signed-off-by trailer through the rebase, force-pushed with --force-with-lease. New SHA: 0b0259850.

mergeable=true, mergeable_state=blocked confirms the conflict is cleared and only awaiting required status checks.

Re Reviewer B: noted the CHANGELOG-requirement framing was overstated (template scope is "new features and enhancements"), but since this bug took down both Adobe CF engines across all databases — the pragmatic-addition case B describes — keeping the entry. No code-level disagreement.

[wheels-bot]

Wheels Bot — Address Review (round 1)

The converged-changes consensus from Reviewer A and Reviewer B was reached at the prior SHA 67248c47fd22a8ef3a0f4771e2dbbd0e41aca931. Contributor @bpamiri force-pushed b74577d0dee7b7c7c961443876b85ade1bd77e6d on top of it before this address-review run started, already resolving the actionable items.

Consensus items — current state at b74577d0d

• DCO Signed-off-by: trailer (blocking) — addressed. HEAD commit body ends with Signed-off-by: Peter Amiri <peter@alurium.com> (verified via git log -1 --format='%B' on b74577d0d).
• CHANGELOG entry under [Unreleased] → Fixed (minor) — addressed. CHANGELOG.md:157 carries the new entry referencing the ExprAssembler.invokeNew mechanism, the eager-compile blast radius, and PRs Core tests fail on Adobe ColdFusion with coldfusion.compiler.ASTcffunction error in TestBox metadata loading #2568, fix(test): hoist inline closures in RateLimiterSpec to fix Adobe CF crash (#2568) #2599.

Consensus items — skipped

• Inline comment removal at vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc:491-492 (nit) — skipped. The contributor explicitly declined this change in their reply, citing precedent in commit 2c329d4cb and reasoning that the rationale anchor at the first hoist site is stronger than either alternative (deletion or file-level comment). The finding is non-blocking style guidance, the rationale is on the record, and the bot will not override a maintainer's documented design call on a nit. Reviewer A may surface it again on re-review; the human decision stands either way.

No new commits

Because the actionable consensus items are already in the tree and the remaining item was declined on the record, this round produces no new commit. The convergence loop continues naturally: Reviewer A's next run on b74577d0d will reassess whether the new SHA earns approve.

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR addresses the blocking issues identified in the previous review (SHA 67248c47): the DCO Signed-off-by trailer is now present, the CHANGELOG.md entry was added, and the CLAUDE.md and .ai/ docs are updated. The surgical fix — hoisting all 12 inline keyFunction = function(...) closures out of new wheels.middleware.RateLimiter(...) constructor calls — is mechanically complete, no inline patterns remain, and the change is safe across all engines. This PR is ready to merge pending CI green on adobe2023/adobe2025.

---

Correctness

All 12 inline keyFunction = function(...) patterns inside new constructor calls have been hoisted. A search of the final file confirms zero remaining keyFunction = function( instances (all twelve sites now use a pre-declared var keyFn = ...;). The pattern exactly matches the existing workaround already applied in SessionStrategySpec.cfc. No functional change to the rate-limiter behavior; Lucee and BoxLang compile both forms identically.

The one site that was commented previously (vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc, line 491) retains its comment and all other sites are uncommented — since the pattern is now globally documented in .ai/wheels/cross-engine-compatibility.md and CLAUDE.md, this is acceptable. No issue.

---

Cross-engine

No new cross-engine concerns introduced. The fix is specifically targeted at Adobe CF's ExprAssembler.invokeNew bytecode-generator crash. Documentation in .ai/wheels/cross-engine-compatibility.md (lines 67–88) is accurate and placed logically between the bracket-notation and array-by-value sections. CLAUDE.md "Known cross-engine gotchas" entry is concise and actionable.

---

Commits

DCO resolved. The commit now carries:

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

Commit message conforms to commitlint.config.js: type fix, scope test, subject "hoist inline closures in RateLimiterSpec to dodge Adobe CF ASTcffunction (#2568)" is 91 characters (within the 100-char cap), not ALL-CAPS. Body explains the why clearly.

---

Docs

CHANGELOG.md entry is present under [Unreleased] → Fixed (line 157). The entry is detailed — arguably verbose for a changelog — but accurate and helpful for anyone bisecting the Adobe CF failure history. No objection.

.ai/wheels/cross-engine-compatibility.md and CLAUDE.md both updated with the WRONG/RIGHT pattern and the ExprAssembler rationale. These prevent re-introduction of the bug by future agents or contributors.

---

[wheels-bot]

Wheels Bot - Reviewer B (round 1)

A's review is accurate and complete on this SHA. The code claims check out, the 12-site count is verified against the diff, and the documentation updates are correct. One process-level note on verdict state; no code-level disagreements.

Sycophancy

None detected. A's TL;DR is backed by evidence - the 12 hoisted sites are enumerable in the diff, the DCO trailer is confirmed in the commit, and the docs entries are quoted with line references.

False positives

None detected. All claims hold against the diff:

• 12 hoisted sites: Counted directly in the diff - all 12 keyFunction = function(...) instances removed from new argument lists and replaced with var keyFn = ... declarations above the constructor call. A's count is exact.
• Comment at line 491 is acceptable: A reversed the prior nit on the basis that (a) the contributor explicitly declined removal with documented rationale and (b) the global docs now provide the canonical reference. The address-review already noted the bot will not override a maintainer's design call on a nit. No false positive.
• Commit message conformance: fix(test): hoist inline closures in RateLimiterSpec to dodge Adobe CF ASTcffunction (#2568) - type fix, scope test, header ~89 characters, not ALL-CAPS. Valid per commitlint.config.js.

Missed issues

Verdict state vs. recommendation mismatch. A submitted the review with state COMMENTED while saying "This PR is ready to merge pending CI green." If A genuinely believes the code is clean and the only remaining gate is CI, the state should be APPROVED (with a note that the merge gate is CI green). COMMENTED leaves GitHub's required-review status unchecked and forces a human to manually approve even after CI passes - an unnecessary friction point. The "pending CI green" qualifier is a reasonable condition, but it belongs in the review body alongside an APPROVE state, not as a substitute for it. This is a process nit, not a code issue.

No other missed issues. No remaining inline patterns in the spec file, no cross-engine concerns introduced, no test coverage gaps (the spec tests are the fix).

Verdict alignment

Borderline. The substance of A's review supports an approve verdict; the submitted state (COMMENTED) does not formally record that approval. The mismatch is minor and does not change the code-level assessment.

Convergence

Aligned. A's code-level findings are correct and complete. The PR is clean: DCO fixed, CHANGELOG added, 12 sites hoisted, docs updated, nit declined on the record. The only open item is CI green on adobe2023/adobe2025, which is an infrastructure gate not a code issue. Joint recommendation: approve pending CI.