fix(dispatch): preserve receiver context in BoxLang invokeMethod (#2646)#2648
Conversation
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: Clean, minimal fix for a BoxLang-specific receiver-context bug. The core one-liner change is correct and well-motivated. No correctness or cross-engine blockers — only a test-fragility nit in the third spec, a missing CHANGELOG entry, and a missing cross-engine doc update.
Correctness
The fix is sound. Collapsing the two-statement dispatch:
// Before
local.method = arguments.object[arguments.methodName];
local.method();into a single expression:
// After — BoxLangAdapter.cfc:197
arguments.object[arguments.methodName]();correctly preserves the component receiver under BoxLang JS-style function dispatch. The root-cause analysis matches the observed failure mode (Public.cfc handlers calling $blockInProduction() after #2241).
The bracket-notation cross-engine gotcha from .ai/wheels/cross-engine-compatibility.md (Adobe CF crashes on obj["key"]() inside closures) does not apply here: this is a regular method body, not a closure, and BoxLangAdapter.cfc is only ever instantiated on BoxLang — Adobe never executes this code path.
Tests
Concern: third spec's catch condition is over-broad
vendor/wheels/tests/specs/dispatch/InvokeMethodSpec.cfc, lines 65-69:
if (FindNoCase("$blockInProduction", e.message)
|| FindNoCase("$privateHelper", e.message)
|| FindNoCase("not found", e.message)) {
threwReceiverLoss = true;The third arm (FindNoCase("not found", e.message)) matches any "file not found" or "template not found" error that has nothing to do with receiver loss — e.g. a view-path failure if congratulations.cfm is absent in a stripped test harness. The comment directly below says view-path errors are unrelated, but the condition contradicts that by treating them as failures.
Suggested tightening:
if (FindNoCase("$blockInProduction", e.message)
|| FindNoCase("$privateHelper", e.message)
|| REFindNoCase("Function \[\$[a-zA-Z]", e.message)) {
threwReceiverLoss = true;Minor concern in practice — congratulations.cfm exists in the repo — but the over-broad guard is a flaky-test footgun in stripped environments and the intent is clearer with the tightened pattern.
Observation: first two specs exercise the current engine adapter, not BoxLangAdapter directly
vendor/wheels/tests/specs/dispatch/InvokeMethodSpec.cfc, line 21:
var adapter = application.wheels.engineAdapter;On Lucee and Adobe, this resolves to the Lucee/Adobe adapter (invoke() in Base.cfc). The two specs pass trivially on non-BoxLang engines without touching the changed code path. The PR description acknowledges this and correctly identifies CI as the authoritative gate. Noted for transparency, not a blocker.
Docs
Missing CHANGELOG entry — no entry under [Unreleased]. This is a user-visible bug fix (internal routes 500-ing on BoxLang) and should be recorded there.
Missing cross-engine compatibility doc update — .ai/wheels/cross-engine-compatibility.md does not document the BoxLang receiver-loss gotcha. The existing doc covers the Adobe-CF inverse (bracket-notation crash inside closures), but the BoxLang trap — single-expression bracket-call required to preserve the receiver — is equally non-obvious and will be rediscovered by future contributors. Suggest adding a short entry mirroring the existing Adobe section.
Commits
Single commit: fix(dispatch): preserve receiver context in BoxLang invokeMethod (#2646). Type, scope, and subject all conform to commitlint.config.js. No issues.
Wheels Bot — Reviewer B (round 1)A's review is accurate and well-grounded. All three findings hold up against the actual code. The core fix is correctly identified as sound. Converging on changes needed. SycophancyNone detected. A submitted as False positivesNone detected.
Missed issuesNone detected. Spot-checks passed:
Verdict alignmentA's ConvergenceAligned. A identified three actionable items, all verified:
Joint recommendation: apply all three changes. |
- Tighten the third InvokeMethodSpec catch from FindNoCase("not found") —
which would false-positive on view-not-found or template errors — to
REFindNoCase("Function \[\$[a-zA-Z]"), which matches only the receiver-
loss signature this spec is meant to guard.
- Add a CHANGELOG.md [Unreleased] / Fixed entry describing the BoxLang
internal-route fix.
- Document the BoxLang receiver-loss trap in
.ai/wheels/cross-engine-compatibility.md alongside the existing Adobe
bracket-call section, with a Why + reference example, so future
contributors don't rediscover it.
No behavior change in the fix itself — these are review polish only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed Reviewer A in a690f86:
Reviewer A's observation about specs 1–2 passing trivially on non-BoxLang engines stands — the spec is a regression guard, and the CI matrix is the authoritative cross-engine check. Acknowledged in the PR description. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR correctly fixes the BoxLang receiver-loss bug in BoxLangAdapter.invokeMethod, and the second commit cleanly addresses the three nits from the previous round (tightened catch, CHANGELOG, cross-engine doc). The fix and the regression spec are sound. Two issues block merge: both commits are missing DCO Signed-off-by: trailers (a required status check), and the new cross-engine doc section labels invoke() as "ALSO RIGHT" on BoxLang when the PR author's own alternatives note says that parity is unconfirmed — a future contributor following that guidance could reintroduce the very bug this PR fixes.
Correctness
invoke() labeled "ALSO RIGHT" in BoxLang context, parity unconfirmed
.ai/wheels/cross-engine-compatibility.md, lines 80-81:
// ALSO RIGHT — the invoke() BIF (used by Lucee/Adobe via Base.cfc)
invoke(arguments.object, arguments.methodName);The PR description's "Alternatives considered" section explicitly says:
Delete the override entirely and fall through to
Base.cfc::invoke(...). Likely works on current BoxLang but couldn't confirm in this PR (Docker mount failed against the worktree path).
The reason BoxLangAdapter.cfc was written with an invokeMethod override in the first place is precisely because invoke() did not work reliably on BoxLang. Documenting it as "ALSO RIGHT" in the BoxLang section — without confirming it — could mislead a future contributor into replacing the single-expression bracket-call with invoke() and silently reintroducing the receiver-loss bug.
Suggested fix — either remove the invoke() example from this section, or annotate it clearly:
// ALSO RIGHT — the invoke() BIF (Lucee/Adobe only; BoxLang parity unconfirmed)
invoke(arguments.object, arguments.methodName);Or add a prose note:
Note:
invoke()is the Lucee/Adobe path viaBase.cfc. Its correctness on BoxLang has not been verified — until confirmed, prefer the single-expression bracket-call on BoxLang.
Commits
Both commits missing DCO Signed-off-by: trailers
CONTRIBUTING.md § DCO, line 81:
The DCO GitHub App runs as a required status check on every PR. It verifies that every commit in the PR carries a valid
Signed-off-by:trailer and blocks merge if any are missing.
Neither commit carries the trailer:
aea14281—fix(dispatch): preserve receiver context in BoxLang invokeMethod— noSigned-off-by:a690f862—docs: address review feedback from #2648 (Reviewer A)— noSigned-off-by:
Both need Signed-off-by: Peter Amiri <peter@alurium.com> (matching the configured git identity). The easiest fix if neither commit is yet merged upstream is an interactive rebase with git commit --amend -s on each, or git rebase --signoff HEAD~2.
Tests
Minor: caughtMessage captured but never asserted
vendor/wheels/tests/specs/dispatch/InvokeMethodSpec.cfc, lines 61 and 73:
var caughtMessage = "";
// ...
caughtMessage = e.message;caughtMessage is assigned but the only downstream assertion is expect(threwReceiverLoss).toBeFalse(). The captured message is never surfaced. In a failure scenario a maintainer would have to add a breakpoint to read it. Low-priority, but including it in the failure message would make the spec self-diagnosing:
expect(threwReceiverLoss).withMessage("Receiver-loss error thrown: " & caughtMessage).toBeFalse();(Or remove the variable entirely if it will never be used.)
Everything else
The core fix (arguments.object[arguments.methodName]() single expression), the fixture shape, the first two specs, the CHANGELOG entry, and the tightened REFindNoCase catch all look correct and clean. No cross-engine concerns fire — BoxLangAdapter.cfc is only ever instantiated on BoxLang, so the Adobe bracket-call restriction (closures only) does not apply here.
Wheels Bot — Address Review (round 1)Reviewer A + B converged on three changes at
No findings were skipped — the consensus and the human's commit are 1:1. A fresh Reviewer A run should fire on |
Internal Wheels routes (/wheels/info, /wheels/routes, /wheels/packages, ...)
were failing on BoxLang with "Function [$blockInProduction] not found".
Root cause: BoxLangAdapter.invokeMethod() extracted the handler as a function
reference into a local var and called it standalone:
local.method = arguments.object[arguments.methodName];
local.method();
In BoxLang's JS-like dispatch semantics that loses the component receiver,
so the handler's first call to $blockInProduction() — added to every
Public.cfc handler in #2241 — could not resolve the helper on its own
component.
Fix: collapse the lookup and call into a single bracket-call expression
(arguments.object[arguments.methodName]()). The expression-level dispatch
keeps the receiver bound, so in-component $-helpers resolve normally.
Lucee/Adobe were unaffected (they go through Base.cfc::invoke()).
Adds a fixture + spec under tests/specs/dispatch/ that exercises the
"public handler calls internal $helper" shape end-to-end through the
adapter. The spec passes on every engine; it is a regression guard so a
future refactor cannot reintroduce the receiver-loss pattern on BoxLang.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
- Tighten the third InvokeMethodSpec catch from FindNoCase("not found") —
which would false-positive on view-not-found or template errors — to
REFindNoCase("Function \[\$[a-zA-Z]"), which matches only the receiver-
loss signature this spec is meant to guard.
- Add a CHANGELOG.md [Unreleased] / Fixed entry describing the BoxLang
internal-route fix.
- Document the BoxLang receiver-loss trap in
.ai/wheels/cross-engine-compatibility.md alongside the existing Adobe
bracket-call section, with a Why + reference example, so future
contributors don't rediscover it.
No behavior change in the fix itself — these are review polish only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…-up) Reviewer A flagged a contradiction in the new cross-engine doc section: the PR description's "Alternatives considered" note explicitly says invoke()-on-BoxLang parity is unconfirmed, but the doc was labelling invoke() as "ALSO RIGHT" alongside the bracket-call. A future contributor following the doc could replace the override with invoke() and silently re-introduce the receiver-loss bug. - Drop the "ALSO RIGHT" invoke() example from the BoxLang section. - Add an explicit "On invoke()" callout: invoke() is the Lucee/Adobe path via Base.cfc, parity on BoxLang has NOT been verified, the override exists precisely because earlier BoxLang versions had invoke() gaps, and the override can only be removed after a BoxLang audit confirms parity. Also surface the receiver-loss error message in the third spec's assertion instead of stashing it in a never-read local. The spec now fails self-diagnosing — the actual error string lands in the test output instead of forcing a maintainer to add a breakpoint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
a690f86 to
f50179e
Compare
|
Addressed Reviewer A round 2 (force-pushed
|
Wheels Bot -- Reviewer B (round 1)A's review on this SHA is accurate and well-constructed. All three findings hold up against the diff and commit history. Converging on changes needed. SycophancyNone detected. A submitted as False positivesNone detected.
Missed issuesNone detected. Spot-checks on the rest of the diff came back clean:
Verdict alignmentA's ConvergenceAligned. A and B agree on all three findings. Two concrete changes are needed before merge:
Joint recommendation: apply changes 1 and 3 (bot-addressable), and the PR author signs the commits manually. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: All three items blocking merge in round 2 have been resolved: all commits now carry Signed-off-by: trailers, the "ALSO RIGHT" invoke() example has been removed from the BoxLang doc section and replaced with an accurate "unconfirmed parity" callout, and the third spec's receiverLossMessage is now surfaced directly in the assertion message rather than stashed in a dead local. The core one-liner fix and the regression spec remain correct and clean. Approving.
Correctness
No new concerns. The fix itself -- collapsing local.method = obj[name]; local.method() to arguments.object[arguments.methodName]() in BoxLangAdapter.cfc:197 -- is sound and unchanged since round 1. The bracket-call restriction for Adobe CF (closures only) does not apply here since invokeMethod is a regular method body, not a closure, and BoxLangAdapter.cfc is only ever instantiated on BoxLang.
Cross-engine
Round 2 flagged that the new cross-engine doc section labeled invoke() as "ALSO RIGHT" for BoxLang when the PR description explicitly said parity was unconfirmed. Commit 3 drops the code example entirely and adds an explicit prose callout:
Whether it preserves the receiver on BoxLang has not been verified -- the
invokeMethodoverride inBoxLangAdapter.cfcexists precisely because earlier BoxLang versions hadinvoke()parity gaps. Until a BoxLang run confirms the BIF binds the receiver, prefer the single-expression bracket-call on BoxLang.
That is accurate and will prevent a future contributor from unknowingly reintroducing the bug. The adjacent Adobe bracket-call section ("WRONG inside closures: split into two statements") and the new BoxLang section ("WRONG to split: single-expression") are not contradictory -- the Adobe restriction is closure-scoped and the BoxLang trap fires in regular method bodies. Both sections say which engine they apply to.
Commits
All three commits now carry Signed-off-by: Peter Amiri <peter@alurium.com>. DCO check will pass.
Commit messages conform to commitlint.config.js: types fix and docs are from the allowlist, no ALL-CAPS subjects, all headers under 100 chars.
Tests
Verified toBe(any expected, message = "") is the correct WheelsTest signature (vendor/wheels/wheelstest/system/Expectation.cfc:168). The third spec's updated assertion:
expect(receiverLossMessage).toBe(
"",
"Receiver-loss error thrown from Public.cfc::index: " & receiverLossMessage
);is valid and self-diagnosing on failure. No remaining concerns.
Wheels Bot — Address Review (round 2)Reviewer A + B converged on three findings at
No findings were skipped — the round-2 consensus and the current HEAD are 1:1. A fresh Reviewer A run should fire on |
Wheels Bot — Reviewer B (round 1)A's review on this SHA is accurate and well-grounded. All positive claims are verified against the diff and commit history. No sycophancy, no false positives, no missed issues. Converging on approve. SycophancyNone detected. A approved after three rounds of concrete blocking issues, each of which was resolved before A submitted. Every approval claim is cited to specific lines or verified behavior. False positivesNone detected.
Missed issuesNone detected. Spot-checks passed:
Verdict alignmentA's ConvergenceAligned on approve. A and B agree that all three blocking items from round 2 are addressed and no new concerns exist on this SHA. The PR is review-clean and ready for a human to mark as ready-for-review and merge. |
Summary
Function [$blockInProduction] not found#2646. Internal Wheels routes (/wheels/info,/wheels/routes,/wheels/packages,/wheels/guides, …) were 500ing on BoxLang withFunction [$blockInProduction] not found.BoxLangAdapter.invokeMethod()didlocal.method = obj[name]; local.method();— extracting the handler into a local var stripped the component receiver under BoxLang's JS-like dispatch, so the handler's first call to$blockInProduction()(introduced in fix(config): hard-block /wheels/* GUI surface in production (#2233) #2241) couldn't resolve.arguments.object[arguments.methodName]()). Keeping the lookup and call in one expression preserves the receiver. Lucee/Adobe were unaffected — they go throughBase.cfc::invoke().Why this just started failing
$blockInProduction()was added to everyPublic.cfchandler in #2241 (Apr 22, 2026). Before that PR, handlers only included a view file — they never made in-component calls — so the broken receiver dispatch was never exercised on BoxLang. Once handlers started calling internal$-helpers, the bug surfaced on every internal route.Test plan
vendor/wheels/tests/specs/dispatch/InvokeMethodSpec.cfcwith fixture atvendor/wheels/tests/_assets/dispatch/InvokeMethodFixture.cfc. The fixture mirrors thePublic.cfcshape (public handler calling a$-prefixed internal helper). Three specs:engineAdapter.invokeMethod()runs the handler and the internal helper (asserts both side-effect flags flip).Public.cfc::index(the only handler that doesn't call$blockInProduction) doesn't throw "function not found" — guards against any related receiver-loss regression./wheels/info,/wheels/routes,/wheels/packages— all render instead of throwing.Alternatives considered
Base.cfc::invoke(arguments.object, arguments.methodName). Likely works on current BoxLang but couldn't confirm in this PR (Docker mount failed against the worktree path). Kept the override as a smaller-blast-radius change; a follow-up could remove it after confirminginvoke()parity.Invoke(arguments.object, arguments.methodName)inside the override. Same end state as deleting the override — same risk profile, more code.🤖 Generated with Claude Code