Skip to content

fix: green the compat matrix for BoxLang and Adobe CF 2023/2025 - #2817

Merged
bpamiri merged 5 commits into
developfrom
peter/fix-compat-matrix-boxlang-adobe
May 27, 2026
Merged

bpamiri merged 5 commits into
developfrom
peter/fix-compat-matrix-boxlang-adobe

Conversation

@bpamiri

@bpamiri bpamiri commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Gets the Wheels Compatibility Matrix green before v4.0.2. BoxLang, Adobe CF 2023, and Adobe CF 2025 were all red; Lucee 6/7 were green. Root-caused and fixed each, verified locally across all engines.

BoxLang (89 failures → 0)

All 89 failures (17 fail / 72 error) traced to one root cause: Global.cfc's pseudo-constructor seeded local.varKey = "", which BoxLang materializes as variables.local, shadowing the function-local local scope of every mixed-in $-helper (Migrator/Model local.appKey, etc.) → KeyNotFoundException: The key [APPKEY] was not found in the struct. Valid keys are ([VARKEY]). Lucee/Adobe keep local reserved to function scope, so they were unaffected. Extracted the UDF-promotion loop into $promoteIncludedGlobalsToThis() so its iterator uses a real function-local scope.

Adobe CF 2023 / 2025

  • Suite-crash (dominant cause of red): InvokeMethodSpec invoked Public.index(), rendering the congratulations welcome page straight into the test-runner response buffer. Adobe committed the buffer (HTTP 404 + ~1MB HTML prefix), corrupting the entire suite's JSON output. Wrapped the call in cfsavecontent.
  • RequestId middleware: wrote request.wheels.requestId through a parameter named request, which shadows the request scope on Adobe (anti-pattern New master #11). Moved the scope write into a helper with no request param.
  • TestClient: emitted a cfhttp POST with zero cfhttpparam tags for an empty body; Adobe rejects that. Always emit a (possibly empty) body param for POST/PUT/PATCH.
  • ParallelRunner.$collectFailures: relied on array-by-reference mutation, but Adobe passes arrays to functions by value, so the aggregated failure list stayed empty. Returns the array now.
  • $reincludeGlobals: re-included a file whose UDFs were already bound to application.wo, tripping Adobe's "Routines cannot be declared more than once". Evaluates the file in a throwaway GlobalIncludeLoader instance.
  • RewriteConfigInstallerSpec (2025 only): compared file content to a literal string; Adobe 2025's fileWrite/fileRead round-trip appends a newline. Compares to a normalized post-write baseline now.

Local verification (full suites)

Engine Result
BoxLang sqlite 3790/0/0, mysql 3801/0/0
Adobe 2023 mysql 3801/0/0 (HTTP 200, clean JSON)
Adobe 2025 mysql 3801/0/0 (HTTP 200, clean JSON)
Lucee 6 mysql 3801/0/0 (no regression)
Lucee 7 sqlite 3789/0/0 (no regression)

Test plan

  • Compatibility Matrix CI green for lucee6, lucee7, adobe2023, adobe2025, boxlang across all DBs (oracle soft-fail)
  • No regression on the standard test suite

🤖 Generated with Claude Code

BoxLang: Global.cfc's pseudo-constructor seeded `local.varKey = ""`, which
BoxLang materializes as `variables.local`, shadowing the function-local
`local` scope of every mixed-in `$`-helper (Migrator/Model `local.appKey`).
89 spec failures all traced to this one cause. Extracted the UDF-promotion
loop into `$promoteIncludedGlobalsToThis()` so its iterator uses a real
function-local scope.

Adobe CF 2023/2025:
- RequestId middleware wrote `request.wheels.requestId` through a parameter
  named `request`, which shadows the request scope on Adobe (anti-pattern #11).
  Moved the scope write into a helper with no `request` param.
- TestClient emitted a `cfhttp` POST with zero `cfhttpparam` tags for an empty
  body; Adobe rejects that. Always emit a (possibly empty) body param.
- ParallelRunner.$collectFailures relied on array-by-reference mutation; Adobe
  passes arrays to functions by value, so the aggregated failure list stayed
  empty. It returns the array now.
- $reincludeGlobals re-included a file whose UDFs were already bound to
  application.wo, tripping Adobe's "Routines cannot be declared more than
  once". Evaluates the file in a throwaway GlobalIncludeLoader instance.
- InvokeMethodSpec invoked Public.index(), rendering the congratulations page
  into the test-runner response buffer; Adobe committed it (HTTP 404 + ~1MB
  prefix), corrupting the whole suite's JSON. Captured with cfsavecontent.
  This was the dominant cause of Adobe red.
- RewriteConfigInstallerSpec compared content to a literal string; Adobe 2025's
  fileWrite/fileRead round-trip appends a newline. Compares to a normalized
  post-write baseline now.

Verified locally across all engines (full suites): BoxLang sqlite 3790/0/0 +
mysql 3801/0/0; Adobe 2023 & 2025 mysql 3801/0/0 (HTTP 200, clean JSON);
Lucee 6 & 7 no regression.

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

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@github-actions github-actions Bot added the docs label May 27, 2026

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This PR is a well-diagnosed, multi-engine compatibility sweep — six independent root causes, all traced to concrete engine behaviour, all fixed correctly. Local verification is thorough (3790–3801 pass, 0 fail/error across every engine). I have one minor correctness nit in TestClient.cfc, one docs gap, and one misleading commit scope; nothing rises to request-changes.


Correctness

TestClient.cfc — form-field path when sendAsJson=false and body is empty

vendor/wheels/wheelstest/TestClient.cfc lines 510–518

// new code
if (ListFindNoCase("POST,PUT,PATCH", arguments.method)) {
    if (!StructIsEmpty(arguments.body) && !variables.sendAsJson) {
        for (var fName in arguments.body) {
            cfhttpparam(type = "formfield", name = fName, value = arguments.body[fName]);
        }
    } else {
        cfhttpparam(type = "body", value = StructIsEmpty(arguments.body) ? "" : SerializeJSON(arguments.body));
    }
}

The intent is: for an empty-body form POST (sendAsJson=false, body={}), emit a bare type="body" value="" to satisfy Adobe CF's "requires at least one cfhttpparam for POST" constraint. That is correct. The logic is a bit surprising to read, though — the else branch fires for both JSON posts AND empty-body form posts, and the ternary inside decides which. A future reader might not see why an empty form post emits type="body" instead of zero cfhttpparam tags. The code is correct; a one-line comment next to the ternary would help:

// Empty form posts must still emit a body param (Adobe CF rejects
// POST/PUT/PATCH with zero cfhttpparam tags); JSON posts always use body.
cfhttpparam(type = "body", value = StructIsEmpty(arguments.body) ? "" : SerializeJSON(arguments.body));

Minor nit, no behaviour change needed.

GlobalIncludeLoader.cfc — exclusion list is fragile

vendor/wheels/GlobalIncludeLoader.cfc lines 35–38

for (var varKey in variables) {
    if (varKey != "loadFunctions" && IsCustomFunction(variables[varKey])) {
        fns[varKey] = variables[varKey];
    }
}

The component has exactly one declared method (loadFunctions) so the by-name exclusion is safe today. If a second method is ever added the exclusion list must be extended, which is easy to forget. A snapshot-diff pattern would be more robust:

var beforeVarKeys = StructKeyList(variables);
include "#arguments.file#";
for (var varKey in variables) {
    if (!ListFindNoCase(beforeVarKeys, varKey) && IsCustomFunction(variables[varKey])) {
        fns[varKey] = variables[varKey];
    }
}

This is low-risk as written; flagging as a nit, not a blocker.


Conventions

No violations found. All six changes follow Wheels conventions:

  • $promoteIncludedGlobalsToThis() is correctly declared public with $ prefix in Global.cfc (anti-pattern #7 — mixin helpers must be public).
  • $writeRequestId() is correctly declared private in RequestId.cfc (not a mixin, so private is right here; the request param-shadow fix matches anti-pattern #11 exactly).
  • $collectFailures() returning rather than mutating aligns with the Adobe CF array-by-value rule (CLAUDE.md cross-engine invariant #6).

Cross-engine

All fixes are grounded in documented engine behaviour. Confirming against .ai/wheels/cross-engine-compatibility.md:

  • BoxLang variables.local pollution from a pseudo-constructor local.varKey assignment is the same class of bug as the "method-local local shadowed by component-scope local" pattern. Moving the loop into $promoteIncludedGlobalsToThis() is the correct mitigation.
  • Adobe CF array-by-value in function arguments is documented; returning the array is the right fix for $collectFailures().
  • Adobe CF request scope shadowing by a same-named parameter is anti-pattern #11; the $writeRequestId() helper side-steps it cleanly.
  • Adobe CF "Routines cannot be declared more than once" on re-include is resolved by the GlobalIncludeLoader throwaway instance.

No regressions on Lucee 6/7 or BoxLang are expected from these changes; the PR's local verification table confirms this.


Tests

The existing InvokeMethodSpec.cfc wraps the leaky-output call in cfsavecontent — correct and minimal fix. The file was already a regression spec; no new spec needed.

The RewriteConfigInstallerSpec.cfc change reads a baseline after the write rather than comparing against the literal write string — correct for Adobe 2025's trailing-newline round-trip.

Missing: there is no unit test for the new GlobalIncludeLoader.cfc or for the changed $promoteIncludedGlobalsToThis() path. A minimal spec verifying that a .cfm file's functions are returned and the component's own methods are not would improve confidence. Not blocking for a compat-matrix sweep, but worth a follow-up issue.


Docs

CHANGELOG.md is not updated. This PR fixes real user-visible bugs that should appear in the [Unreleased] section:

  • RequestId middleware was silently writing to the passed struct instead of the request scope on Adobe CF 2023/2025 — any downstream code reading request.wheels.requestId for tracing would find it missing on those engines.
  • ParallelRunner.$collectFailures was returning an empty failure list on Adobe CF — parallel test runs reported zero failures even when specs actually failed.

These are meaningful regressions for Adobe users and deserve at least a bullet each under ### Fixed in CHANGELOG.md.


Commits

Single commit: fix(test): green the compat matrix for BoxLang and Adobe CF 2023/2025

The type (fix) and subject are correct. The scope (test) is misleading: the changes touch production framework files (vendor/wheels/Global.cfc, vendor/wheels/GlobalIncludeLoader.cfc, vendor/wheels/middleware/RequestId.cfc, vendor/wheels/wheelstest/ParallelRunner.cfc) that are not test-only code. fix: green the compat matrix … (no scope) or fix(compat): … would be more accurate. Per CLAUDE.md the scope is "optional and unrestricted" — dropping or changing it avoids the ambiguity. Not a hard rule violation, just a clarity nit.

Header length is 74 chars — within the 100-char limit. Subject is not ALL-CAPS. Conforms otherwise.


wheels-bot[bot]
wheels-bot Bot previously approved these changes May 27, 2026

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: Follow-up review of the new head commit (SHA 0be659982), which adds the CHANGELOG entry flagged as missing in the previous review. The documentation gap is now resolved; the code changes were already reviewed as correct. Approving.


Docs

The previous review noted CHANGELOG was missing for this PR's fixes. The new commit docs(changelog): note compat-matrix BoxLang/Adobe fixes in [Unreleased] (#2817) adds a thorough, accurate bullet under ### Fixed in [Unreleased]. The entry names every root cause (BoxLang variables.local shadowing, Adobe cfsavecontent crash, RequestId scope-shadow anti-pattern #11, TestClient empty-body cfhttp, ParallelRunner array-by-value, GlobalIncludeLoader throwaway instance, RewriteConfigInstallerSpec newline round-trip), cites the local verification table, and references the PR number. Gap closed.


Commits

Both commits carry Signed-off-by: Peter Amiri <peter@alurium.com> — DCO satisfied.

The original nit from the previous review stands: fix(test) scope is slightly misleading since several of the changed files are production framework code (Global.cfc, GlobalIncludeLoader.cfc, RequestId.cfc), not test-only code. The new docs(changelog): commit uses the correct type and scope. Neither is a hard commitlint violation; both subjects are within the 100-char limit and not ALL-CAPS.


Residual nits (carry-over, non-blocking)

Two minor observations from the previous review remain open for future cleanup if desired:

  1. GlobalIncludeLoader.cfc line 35 excludes loadFunctions by name: varKey != "loadFunctions". If a second method is ever added to the component the exclusion list must be extended manually. A snapshot-diff pattern (capture keys before the include, iterate new keys only) would be self-maintaining. Not a correctness issue today.

  2. TestClient.cfc lines 510-518: the logic is correct, but the else branch serves double duty (JSON posts and empty-body form posts) and a future reader may not see why an empty form body emits type="body" value="" instead of zero params. A one-line comment would help. Not a behaviour issue.

Neither item warrants holding the PR.

@wheels-bot

wheels-bot Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-grounded. The cross-engine analysis is correct, conventions assessment is right, and the two correctness nits are genuine (though non-blocking). One false positive: A's CHANGELOG finding was accurate when A reviewed at SHA 532f5766f8d6aec71856be0c47daf8cf8f9cd6c9, but a follow-up commit (0be659982, the current head) has since added the CHANGELOG entry. No convergence blocker; the concern is resolved.

Sycophancy

None detected. A used COMMENTED state, explicitly declined to approve, and listed concrete issues rather than waving the PR through on the strength of the local-verification table alone.

False positives

  • CHANGELOG claim: A states CHANGELOG.md is not updated. The current diff (head 0be659982...) includes a ### Fixed bullet that documents all six root causes in detail. A reviewed against SHA 532f5766f... — the CHANGELOG was added in a subsequent commit. Not a wrong read of the code A saw, but the finding is now moot and should not hold up the PR.

No other false positives. The TestClient logic trace is correct (the else branch fires for both empty-body form POSTs and JSON posts, with the ternary picking the right value in each case). The array-by-value analysis for $collectFailures is correct. The request-scope-shadowing call-out for RequestId.cfc matches anti-pattern #11 exactly.

Missed issues

None substantive. Two minor things worth noting but neither blocks alignment:

  • A's suggested snapshot-diff fix for GlobalIncludeLoader.cfc is slightly better than stated: the beforeVarKeys snapshot must be captured before the include call, which A's suggested code does correctly. Worth confirming if that code is adopted as-written.
  • The commit scope (test) is flagged as misleading — correct — but CLAUDE.md explicitly says scope is "optional and unrestricted", so this is not a rule violation, only an accuracy concern. A's characterization is fine.

Verdict alignment

A's de facto verdict — no blockers, two minor nits, CHANGELOG missing (now addressed) — is consistent with the COMMENTED state and with the quality of the diff. Approving without first noting the nits and the CHANGELOG gap would have been sycophantic; A avoided that.

Convergence

Aligned. The CHANGELOG concern (A's only actionable finding) is resolved in the current head. The remaining nits (TestClient comment, GlobalIncludeLoader exclusion-list fragility, commit scope) are all non-blocking style observations. A's effective recommendation is "merge with optional follow-up improvements" — I agree. Recommend approve.

@wheels-bot

wheels-bot Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot -- Reviewer B (round 2)

A's follow-up review is accurate. Approval traces to the CHANGELOG gap being closed by 0be6599. No sycophancy, false positives, or missed issues.

Sycophancy: None. A linked approval to prior blocking finding being resolved, not an unqualified LGTM.

False positives: None. DCO confirmed (Signed-off-by: Peter Amiri). fix(test) scope nit is accurate (production files in diff). GlobalIncludeLoader exclusion-list nit is real but non-blocking. TestClient else-branch observation correct.

Missed issues: None. Cross-engine invariants 7 and 11 checked: promoteIncludedGlobalsToThis is correctly public; cfsavecontent in InvokeMethodSpec is in a function body not a catch; GlobalIncludeLoader iterates local in a real function not pseudo-constructor; writeRequestId param is requestId not a reserved scope; no migration/DI/security changes present.

Verdict alignment: APPROVED is consistent with findings -- prior blocker resolved, remaining nits are non-blocking carry-overs.

Convergence: Aligned. No blockers on this SHA. A and B recommend merge. Nits (GlobalIncludeLoader exclusion list, TestClient readability) documented for optional follow-up.

OuterTransactionSignalSpec's cleanup queryExecute hardcoded
cfsqltype="cf_sql_integer" for the row id. CockroachDB's default
unique_rowid() primary key is a ~60-bit value that overflows a 32-bit
CF_SQL_INTEGER on Adobe CF ("Invalid data <id> for CFSQLTYPE
CF_SQL_INTEGER"); Lucee/BoxLang auto-promote so they never saw it. This was
the sole remaining Adobe red after the compat-matrix fixes — every other DB
(mysql/postgres/sqlserver/sqlite) passed. cf_sql_bigint binds correctly on
every engine and database. Verified: Adobe 2023 model suite on CockroachDB
826/0/0.

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

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
wheels-bot[bot]
wheels-bot Bot previously approved these changes May 27, 2026

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: Follow-up review of the two new head commits (e98f1da6f and b3d6389ed). The first adds the cf_sql_bigint fix for CockroachDB/Adobe CF integer overflow in OuterTransactionSignalSpec; the second amends the CHANGELOG to include it. Both are clean and correct. The PR is now complete: all compat-matrix root causes fixed, all CHANGELOG entries present, DCO satisfied on every commit. Approving.


Correctness

OuterTransactionSignalSpec.cfc — cf_sql_bigint fix is accurate

vendor/wheels/tests/specs/model/OuterTransactionSignalSpec.cfc line 68

// before
{ id = { value = tag.id, cfsqltype = "cf_sql_integer" } }

// after
{ id = { value = tag.id, cfsqltype = "cf_sql_bigint" } }

CockroachDB's unique_rowid() primary key is a ~60-bit value. Adobe CF's JDBC layer enforces the declared cfsqltype at bind time, so cf_sql_integer (32-bit) throws "Invalid data <id> for CFSQLTYPE CF_SQL_INTEGER" on large IDs. Lucee and BoxLang auto-promote numeric types and never surfaced the error. cf_sql_bigint binds correctly on every supported engine and database. The fix is correct and the commit body documents the observed error message, the engine-specific reason, and the verification result (Adobe 2023 model suite on CockroachDB: 826/0/0). No issues.


Docs

The docs(changelog): commit appends the CockroachDB/cf_sql_bigint entry to the existing compat-matrix CHANGELOG bullet under ### Fixed. The combined entry now covers all seven root causes addressed in this PR. The CHANGELOG is complete.


Commits

Both new commits are clean:

  • fix(test): bind cleanup id as cf_sql_bigint for CockroachDB on Adobe CF — type fix, scope (test) is accurate (the only changed file is vendor/wheels/tests/specs/model/OuterTransactionSignalSpec.cfc). Subject is 68 chars, not ALL-CAPS. Commit body fully explains the root cause, affected configuration, and verified result. Conforms to commitlint.config.js.
  • docs(changelog): add CockroachDB cf_sql_bigint fix to compat-matrix e… — correct type and scope. Within 100-char limit.

All four PR commits carry Signed-off-by: Peter Amiri <peter@alurium.com>. DCO satisfied on every commit.


Residual nits (carry-over from prior reviews, non-blocking)

Neither item was introduced by the new commits:

  1. GlobalIncludeLoader.cfc line 35 excludes loadFunctions by name — if a second method is ever added the exclusion must be extended. A snapshot-diff pattern (capture keys before the include, iterate new keys only) would be self-maintaining. Not a correctness issue today.
  2. TestClient.cfc lines 510-518: the else branch serves double duty (JSON posts and empty-body form posts). The code is correct; a single-line comment (as suggested in the prior review) would help a future reader. Not a behaviour issue.

@wheels-bot

wheels-bot Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's follow-up review of the two new commits is accurate and tight. The cf_sql_bigint analysis is correct, the CHANGELOG assessment is correct, commit hygiene checks out, and approval is traced to specific evidence rather than a blanket LGTM. No sycophancy, no false positives, no missed issues on these incremental changes.

Sycophancy

None detected. A explicitly scoped the review to the two new head commits (e98f1da6f and b3d6389ed), linked approval to all prior blockers being resolved and the new commits being clean, and carried forward the non-blocking nits from prior rounds rather than silently dropping them.

False positives

None detected. I verified A's claims against the diff:

  • OuterTransactionSignalSpec.cfc line 68: the change from cf_sql_integer to cf_sql_bigint is confirmed in the diff. CockroachDB's unique_rowid() is a ~60-bit value; 32-bit CF_SQL_INTEGER overflows on Adobe CF at bind time. cf_sql_bigint is universally supported across all CFML engines and databases. A's engine-specific reasoning (Lucee/BoxLang auto-promote, Adobe enforces the declared type at JDBC bind) is accurate.
  • Commit scope (test): the only file changed in e98f1da6f is OuterTransactionSignalSpec.cfc, which is a test file. A correctly assessed the scope as accurate for this commit (in contrast to the prior-round nit on 532f5766, where production files were also in scope — that was a different commit).
  • CHANGELOG completeness: the combined entry now covers all seven root causes. A's count is correct.
  • DCO: all four commits carry Signed-off-by: Peter Amiri. Confirmed.

Missed issues

None. The two new commits touch only a test file and CHANGELOG. No production code paths, no cross-engine compat surface, no migration or security changes. The residual nits A carried over (GlobalIncludeLoader exclusion-list fragility, TestClient comment) remain non-blocking and are correctly characterized as such.

Verdict alignment

A's APPROVED state is consistent with the findings: prior blocker (CHANGELOG gap) was resolved in an earlier head commit; the two incremental commits under review are correct and complete.

Convergence

Aligned. A and B agree: the PR is review-clean on this SHA. All root causes fixed, all CHANGELOG entries present, DCO satisfied, no outstanding blockers. Recommend merge.

@github-actions

Copy link
Copy Markdown
Contributor

Wheels Test Results

     31 files  +    10    6 748 suites  +2 410   26m 11s ⏱️ + 11m 6s
104 825 tests +37 442  104 490 ✅ +37 785  328 💤 +112  2 ❌  - 94  5 🔥  - 361 
106 617 runs  +38 082  106 282 ✅ +38 426  328 💤 +112  2 ❌  - 94  5 🔥  - 362 

For more details on these failures and errors, see this check.

Results for commit b3d6389. ± Comparison against base commit 4a6bf7e.

Addresses non-blocking review nits on #2817:
- GlobalIncludeLoader.loadFunctions now keeps only variables-scope keys that
  appeared after the include (diffed against a snapshot taken before it),
  rather than excluding its own method by name. Adding a sibling method later
  can no longer silently leak it into the returned set. Behavior is unchanged
  today — verified global subset 114/0/0 on Lucee 7, BoxLang, and Adobe 2023
  (reloadGlobalsSpec exercises this path).
- TestClient: clarifying comment on why the else branch emits a body param for
  both JSON posts and empty-body form posts.

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

bpamiri commented May 27, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough review. Addressed the actionable nits in aa8438523:

  • GlobalIncludeLoader exclusion-list fragility (A + B): switched from the by-name loadFunctions exclusion to a snapshot-diff — variables keys are captured before the include and only newly-appeared UDFs are returned, so a future sibling method can't silently leak. Behavior is unchanged today; verified the global subset (which runs reloadGlobalsSpec, the loader's integration path) at 114/0/0 on Lucee 7, BoxLang, and Adobe 2023.
  • TestClient else-branch readability (A): added the one-line comment clarifying the branch covers both JSON posts and empty-body form posts.

Deferred / noted:

  • Dedicated GlobalIncludeLoader unit spec: leaving as the follow-up you both suggested — the loader is already exercised end-to-end by reloadGlobalsSpec (verified on Adobe), and the snapshot-diff now makes own-method exclusion structural rather than by-name.
  • Commit scope (test): fair point — the original commit touched production files. Not rewriting published history for it; later commits use accurate scopes (fix(test) for the cockroachdb test-only change, refactor/no-scope here).
  • CHANGELOG: already added in 0be659982 (B confirmed).

On the "Wheels Test Results" check: the matrix's per-engine jobs all pass — that red is the JUnit aggregate counting BoxLang + Oracle (2 fail / 5 error), which are pre-existing Oracle JDBC connection-lifecycle issues (ORA-17009, "Connection is closed") in the #2663 soft-fail bucket. These fixes actually improved BoxLang+Oracle from 22f/72e → 2f/5e; Oracle now fully passes on Lucee 6/7 and Adobe 2023.

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This single follow-up commit (aa8438523) closes both residual nits from the previous review rounds — the snapshot-diff fix in GlobalIncludeLoader and the clarifying comment in TestClient. No new issues introduced. The PR is complete and I'm approving.


Correctness

GlobalIncludeLoader.loadFunctions — snapshot-diff approach is correct

vendor/wheels/GlobalIncludeLoader.cfc lines 24-44 (new form)

var beforeVarKeys = StructKeyList(variables);
include "#arguments.file#";
// ...
for (var varKey in variables) {
    if (!ListFindNoCase(beforeVarKeys, varKey) && IsCustomFunction(variables[varKey])) {
        fns[varKey] = variables[varKey];
    }
}

Since GlobalIncludeLoader is always instantiated fresh (new wheels.GlobalIncludeLoader().loadFunctions(arguments.file) in Global.cfc), variables at the snapshot point contains only the component's own compiled methods. The post-include diff captures exclusively what the included file declared. The self-maintenance guarantee holds: a second method added to GlobalIncludeLoader will be in beforeVarKeys and cannot leak into the returned set. Behavior is unchanged on all engines, consistent with the commit body's verification report (114/0/0 on Lucee 7, BoxLang, Adobe 2023 against reloadGlobalsSpec).

TestClient.cfc — clarifying comment added, behavior unchanged

The three-line comment on the else branch now documents why both JSON posts and empty-body form posts share the branch. Code path is identical to the previously reviewed form; this is documentation only.


Commits

Single commit: refactor: snapshot-diff UDF detection in GlobalIncludeLoader (#2817)

  • Type refactor is accurate — no behavior change, pure structural improvement.
  • No scope — acceptable per CLAUDE.md "scope is optional."
  • Subject is 55 chars, not ALL-CAPS. Within the 100-char limit.
  • Body documents what changed, why, and the verification result. Conforms to commitlint.config.js.
  • Signed-off-by: Peter Amiri <peter@alurium.com> — DCO satisfied on all five PR commits.

All prior review findings are resolved. The PR addresses seven independent compat-matrix root causes, all verified across every supported engine and database. Nothing remains open.

@wheels-bot

wheels-bot Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review of commit aa8438523 is accurate and tight. The snapshot-diff analysis is correct, the TestClient assessment is correct, commit hygiene checks out, and APPROVED is traced to specific evidence. No sycophancy, no false positives, no missed issues.

Sycophancy

None detected. A linked approval to a concrete structural verification of the snapshot-diff logic, not a blanket LGTM.

False positives

None detected. I verified each claim against the diff:

  • Fresh instantiation: Global.cfc calls new wheels.GlobalIncludeLoader().loadFunctions(arguments.file) — confirmed. The snapshot at the top of loadFunctions therefore sees only the component's own compiled methods.
  • Self-maintenance guarantee: Any sibling method added later will exist in variables before the include and will therefore be in beforeVarKeys, excluded by !ListFindNoCase(beforeVarKeys, varKey). Correct.
  • Dual-engine loop: Lucee adds included UDFs to local; Adobe adds them to variables. Both loops remain, unchanged in behavior. The snapshot-diff only tightens the variables side filter. Correct.
  • TestClient change is documentation only: Confirmed — the three new comment lines precede an unchanged cfhttpparam call; no logic was altered.
  • DCO: All five PR commits carry Signed-off-by: Peter Amiri <peter@alurium.com>. Confirmed in diff headers.
  • Commit type refactor: Accurate — no behavior change, pure structural improvement. Subject 55 chars, within 100-char limit. Scope omitted, acceptable per CLAUDE.md.

Missed issues

None. The two files touched (GlobalIncludeLoader.cfc and TestClient.cfc) are narrow and self-contained. No production code paths were altered — behavior is unchanged on all engines, consistent with the commit body's 114/0/0 verification on Lucee 7, BoxLang, and Adobe 2023.

Verdict alignment

APPROVED is consistent with the findings: all prior blockers (CHANGELOG gap, seven root causes) were resolved in earlier commits; aa8438523 closes the two non-blocking nits cleanly with no regressions.

Convergence

Aligned. A and B agree: the PR is review-clean on this SHA. All root causes fixed, CHANGELOG complete, DCO satisfied, snapshot-diff refactor correct, no outstanding blockers. Recommend merge.

@bpamiri
bpamiri merged commit e07358f into develop May 27, 2026
6 checks passed
@bpamiri
bpamiri deleted the peter/fix-compat-matrix-boxlang-adobe branch May 27, 2026 11:57
@bpamiri bpamiri mentioned this pull request May 27, 2026
4 tasks
wheels-bot Bot pushed a commit that referenced this pull request May 27, 2026
Add a "Compatibility matrix restored" section to the 4.0.2 release
post covering the BoxLang and Adobe CF 2023/2025 compatibility
restoration shipped in #2817 — Reviewer A flagged this as a headline
CHANGELOG deliverable that was silent in the post, and Reviewer B
explicitly converged on it as the one finding warranting action.

- web/content/blog/posts/wheels-4-0-2-released.md: new H2 between
  "Native apt and yum repositories" and "Smaller fixes" summarizing
  the BoxLang `variables.local` shadow root cause, the Adobe CF
  2023/2025 response-already-committed cascade, and the five
  companion Adobe-specific traps fixed alongside.

Not addressed (out of consensus):
- RPM `~80 MB` approximation: A and B both rated as appropriately
  low-severity; no action.
- Missing Signed-off-by on the original commit: not in B's
  "warrants action" list, and amending another author's commit
  on a shared branch is out of scope.

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
bpamiri added a commit that referenced this pull request May 27, 2026
* docs(web/blog): add Wheels 4.0.2 release post

Publishes the Wheels 4.0.2 release announcement to blog.wheels.dev.
4.0.2 (tagged v4.0.2) centers on shared-database migration
reconciliation — orphan-version detection, `migrate doctor`, and
`forget`/`pretend` commands — plus native signed apt/yum repos.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(web/blog): address Reviewer A/B consensus findings (round 1)

Add a "Compatibility matrix restored" section to the 4.0.2 release
post covering the BoxLang and Adobe CF 2023/2025 compatibility
restoration shipped in #2817 — Reviewer A flagged this as a headline
CHANGELOG deliverable that was silent in the post, and Reviewer B
explicitly converged on it as the one finding warranting action.

- web/content/blog/posts/wheels-4-0-2-released.md: new H2 between
  "Native apt and yum repositories" and "Smaller fixes" summarizing
  the BoxLang `variables.local` shadow root cause, the Adobe CF
  2023/2025 response-already-committed cascade, and the five
  companion Adobe-specific traps fixed alongside.

Not addressed (out of consensus):
- RPM `~80 MB` approximation: A and B both rated as appropriately
  low-severity; no action.
- Missing Signed-off-by on the original commit: not in B's
  "warrants action" list, and amending another author's commit
  on a shared branch is out of scope.

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

* docs(web/blog): address Reviewer A/B consensus findings (round 2)

- Fix item-count error in the 4.0.2 compatibility-matrix section of
  web/content/blog/posts/wheels-4-0-2-released.md: "five further
  Adobe-specific traps" → "six further", matching the six-item list
  in the parenthetical (request-scope shadowing, empty-body cfhttp
  POSTs, array-by-value mutation in ParallelRunner, double-include in
  $reincludeGlobals, fileWrite/fileRead roundtrip on Adobe 2025,
  cf_sql_integer overflow on CockroachDB PKs).

Reviewers A and B converged on this as the only finding for the round;
the same miscount appears in CHANGELOG.md but is out of scope for this
docs-only PR.

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 Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant