Skip to content

fix(dispatch): copy arguments to a plain struct in $header() for Adobe CF 2023#2750

Merged
bpamiri merged 5 commits into
developfrom
peter/issue-2741-cfheader-adobe-2023
May 17, 2026
Merged

fix(dispatch): copy arguments to a plain struct in $header() for Adobe CF 2023#2750
bpamiri merged 5 commits into
developfrom
peter/issue-2741-cfheader-adobe-2023

Conversation

@bpamiri
Copy link
Copy Markdown
Collaborator

@bpamiri bpamiri commented May 17, 2026

Summary

  • Global.cfc::$header() now copies the arguments scope into a plain struct before calling cfheader(attributeCollection=...). Adobe CF 2023 rejects the raw arguments scope and throws Failed to add HTML header on every request, recording 0 pass / 0 fail / 0 err in the compat-matrix Adobe 2023 job because no test endpoint completes its request.
  • Same pattern the helper already used on the statusText strip branch (added for Adobe CF 2025) — collapsed into a single unconditional copy. No API change, no caller updates.
  • Regression coverage in vendor/wheels/tests/specs/global/headerSpec.cfc.

Closes #2741.

Test plan

  • Compat-matrix Adobe 2023 (tools/test-matrix.sh adobe2023 sqlite) goes from engine-down / 0 pass / 0 fail / 0 err to a real result row.
  • Adobe 2025 still green (existing statusText strip preserved).
  • Lucee 6 / Lucee 7 / BoxLang still green (struct shape is universally accepted).
  • New headerSpec.cfc passes on every engine.

🤖 Generated with Claude Code

…e CF 2023

Adobe CF 2023 rejects the raw `arguments` scope as `attributeCollection`
on `cfheader`, throwing `Failed to add HTML header` on every request and
recording `0 pass / 0 fail / 0 err` in the compat-matrix Adobe 2023 job
because no test endpoint can boot past application init.

The helper already had the fix on the `statusText` branch (added for
Adobe CF 2025): copy `arguments` into a plain `local.args` struct before
calling `cfheader`. Collapse the two branches so every call hands
`cfheader` a plain struct — no API change, no caller updates.

Closes #2741

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 requested changes May 17, 2026
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

test body

wheels-bot[bot]
wheels-bot Bot previously requested changes May 17, 2026
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A (full review posted separately — see inline comments)

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 17, 2026

Wheels Bot — Reviewer B (round 1)

A's review body is the literal string "test body" with no line comments. There is nothing substantive to critique — A did not analyze the diff. The CHANGES_REQUESTED verdict is incoherent against an empty body (no changes are identified). I re-scanned the diff independently below.

Sycophancy

None in the traditional sense, but the review is a placeholder that did no work. A placeholder that issues CHANGES_REQUESTED with zero findings is arguably worse than sycophancy — it blocks a merge without reason.

False positives

None — A made no claims to evaluate.

Missed issues

A failed to evaluate the diff at all. Re-scanning independently:

  • Fix correctness: The change in vendor/wheels/Global.cfc is correct and clean. The unconditional copy-to-plain-struct unifies the previously branchy code (one branch copied for statusText strip, the other passed the raw scope) into a single code path. This directly addresses Adobe CF 2023's refusal of the raw arguments scope in cfheader(attributeCollection=...), and is consistent with cross-engine invariant New master #2 in CLAUDE.md (Adobe CF rejects certain scope types where other engines accept them).
  • Tests: headerSpec.cfc uses WheelsTest BDD syntax correctly (extends="wheels.WheelsTest", describe/it/afterEach). $assert.notThrows is a valid TestBox assertion. The afterEach resets status and content-type using the same helper — shallow but appropriate for a no-throw regression guard on a cross-engine compat fix. No missed test scenarios for this scope.
  • Commit message: fix(dispatch): copy arguments to a plain struct in $header() for Adobe CF 2023 — valid type (fix), optional scope (dispatch), subject within 100 chars, not ALL-CAPS. Clean.
  • CHANGELOG: Entry is accurate and detailed. No issues.
  • No cross-engine compat concerns introduced: The copy loop uses for (key in arguments) which is universally supported. No closures, no member functions, no reserved-scope collisions.

Verdict alignment

A issued CHANGES_REQUESTED with no findings. That verdict is inconsistent with the diff, which is a clean, well-scoped fix with regression coverage. The correct verdict based on the diff is approve.

Convergence

Not aligned. A's review is a non-functional placeholder; there is no content to converge on. A needs to post a real review with actual analysis (or explicitly approve). If this was a tooling/test submission that landed by mistake, a human should dismiss the review and let the bot re-run Reviewer A on this SHA.

wheels-bot[bot]
wheels-bot Bot previously requested changes May 17, 2026
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

simple test

wheels-bot[bot]
wheels-bot Bot previously requested changes May 17, 2026
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

The fix correctly addresses the Adobe CF 2023 boot failure in $header() by copying arguments to a plain struct before passing it to cfheader(attributeCollection=...). The collapse of the two-branch logic (statusText-present / absent) into one unconditional copy is a clean simplification. However, two sibling functions in the same file have the identical raw-arguments anti-pattern that was just fixed, and leaving them untouched likely means the Adobe CF 2023 matrix will still record failures for any test that touches content-type or cache headers. Requesting changes on that correctness gap; the test and commit nits below are minor.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

The fix correctly addresses the Adobe CF 2023 boot failure in $header() by copying arguments to a plain struct before passing it to cfheader(attributeCollection=...). The collapse of the two-branch logic (statusText-present / absent) into one unconditional copy is a clean simplification. However, two sibling functions in the same file have the identical raw-arguments anti-pattern that was just fixed, and leaving them untouched likely means the Adobe CF 2023 matrix will still record failures for any test that touches content-type or cache headers. Requesting changes on that correctness gap; the test and commit nits below are minor.


Correctness: $content() and $cache() are not patched — same root cause

vendor/wheels/Global.cfc:104 and vendor/wheels/Global.cfc:109 both still pass the raw arguments scope as attributeCollection. The PR body says Adobe CF 2023 "rejects the raw arguments scope" as attributeCollection. That restriction is engine-level — it is not specific to the cfheader tag. cfcontent is called on virtually every response to set content-type; if it also rejects the raw arguments scope on Adobe CF 2023, the compat-matrix job will continue to fail on any test that reaches a render path, even after $header() is fixed. The same risk applies to $cache() for caching-related tests. If local Adobe CF 2023 testing confirms neither function is reached before the engine aborts (i.e. $header() is the sole blocker on every request), please add a note in the PR body explaining that finding — the omission is currently unexplained, which makes reviewers unable to tell whether it is safe.


Tests: afterEach calls the function under test during cleanup

vendor/wheels/tests/specs/global/headerSpec.cfc:14-17afterEach invokes g.$header() — the function under test — for cleanup. If $header() ever regresses (throws), TestBox will report every test as failing with an "afterEach lifecycle" error rather than pointing to the specific it block that broke. The cfheader reset alone is sufficient for cleanup; the trailing g.$header() call adds noise without benefit.


Commits: The commit scope is dispatch but the changed file is Global.cfc. Both the PR body and CLAUDE.md acknowledge that the function sits on the dispatch path, so this is defensible. A scope of global would map more directly to the file, but dispatch is not wrong per the convention that scope is unrestricted and optional. Minor nit, not a blocker.


Cross-engine: The plain-struct copy pattern (local.args = {}; for key in arguments) is universally safe on Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. No new cross-engine risk introduced by the change itself. The concern is only the two unpatched siblings noted above.


Security / Docs / CHANGELOG: No security surface. CHANGELOG entry present and consistent with the verbose style already in the [Unreleased] section. No user-facing API change, so no guides update required.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

The fix correctly addresses the Adobe CF 2023 boot failure in $header() by copying arguments to a plain struct before passing it to cfheader(attributeCollection=...). The collapse of the two-branch logic (statusText-present / absent) into one unconditional copy is a clean simplification. However, two sibling functions in the same file have the identical raw-arguments anti-pattern that was just fixed, and leaving them untouched likely means the Adobe CF 2023 matrix will still record failures for any test that touches content-type or cache headers. Requesting changes on that correctness gap; the test and commit nits below are minor.


Correctness

$content() and $cache() are not patched — same root cause

vendor/wheels/Global.cfc:104 and vendor/wheels/Global.cfc:109 both still pass the raw arguments scope as attributeCollection:

// Global.cfc:101-110 (unchanged by this PR)
public any function $cache() {
    variables.$instance.reCache = false;
    cfcache(attributeCollection = "#arguments#");   // raw arguments
    variables.$instance.reCache = true;
}

public any function $content() {
    cfcontent(attributeCollection = "#arguments#");  // raw arguments
}

The PR body says Adobe CF 2023 "rejects the raw arguments scope" as attributeCollection. That restriction is engine-level — it is not specific to the cfheader tag. cfcontent is called on virtually every response to set content-type; if it also rejects the raw arguments scope on Adobe CF 2023, the compat-matrix job will continue to fail on any test that reaches a render path, even after $header() is fixed. The same risk applies to $cache() for caching-related tests.

The fix is a one-liner in each case — the same local.args + loop pattern is already proven:

public any function $content() {
    local.args = {};
    for (local.key in arguments) {
        local.args[local.key] = arguments[local.key];
    }
    cfcontent(attributeCollection = "#local.args#");
}

public any function $cache() {
    local.args = {};
    for (local.key in arguments) {
        local.args[local.key] = arguments[local.key];
    }
    variables.$instance.reCache = false;
    cfcache(attributeCollection = "#local.args#");
    variables.$instance.reCache = true;
}

If local Adobe CF 2023 testing confirms neither function is reached before the engine aborts (i.e. $header() is the sole blocker on every request), please add a note in the PR body explaining that finding — the omission is currently unexplained, which makes reviewers unable to tell whether it is safe.


Tests

afterEach calls the function under test during cleanup

vendor/wheels/tests/specs/global/headerSpec.cfc:14-17:

afterEach(() => {
    cfheader(statuscode = 200)
    g.$header(name = "content-type", value = "text/html", charset = "utf-8")
})

afterEach invokes g.$header() — the function under test — for cleanup. If $header() ever regresses (throws), TestBox will report every test as failing with an "afterEach lifecycle" error rather than pointing to the specific it block that broke. The cfheader reset alone is sufficient for cleanup; the trailing g.$header() call adds noise without benefit:

afterEach(() => {
    cfheader(statuscode = 200)
})

Commits

The commit scope is dispatch but the changed file is Global.cfc. Both the PR body and CLAUDE.md acknowledge that the function sits on the dispatch path, so this is defensible. A scope of global would map more directly to the file, but dispatch is not wrong per the project's convention that scope is unrestricted and optional. Minor nit, not a blocker.


Cross-engine

The plain-struct copy pattern (local.args = {}; for key in arguments) is universally safe on Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. No new cross-engine risk introduced by the change itself. The concern is only the two unpatched siblings noted above.


Security / Docs / CHANGELOG

No security surface. CHANGELOG entry present and consistent with the verbose style already in the [Unreleased] section. No user-facing API change, so no guides update required.

@wheels-bot wheels-bot Bot dismissed stale reviews from themself May 17, 2026 02:52

Auto-dismissed by Reviewer A guard: body is shorter than 200 characters or missing the canonical wheels-bot:review-a marker. See #2558 for context.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 17, 2026

Wheels Bot — Reviewer A guard

Detected and dismissed 4 bogus Reviewer A review(s) on commit cc9b24a. Cause: review body shorter than 200 characters or missing the canonical wheels-bot:review-a marker. See wheels-dev/wheels#2558 for context.

…tes in Global.cfc

Adobe CF 2023's restriction on the raw `arguments` scope as
`attributeCollection` is engine-level, not specific to `cfheader`. The
initial PR only patched `$header()` (the dispatch-path blocker). This
applies the same plain-struct copy uniformly to all ten remaining
helpers in `Global.cfc` so subsequent test paths that touch `$content`,
`$cache`, `$mail`, `$location`, `$htmlhead`, `$directory`, `$file`,
`$invoke`, `$wddx`, or `$zip` on Adobe CF 2023 do not hit the same
rejection.

Also drop the `g.$header()` call from `headerSpec.cfc`'s `afterEach`:
calling the function under test during cleanup would mask a regression
as an `afterEach` lifecycle error rather than a specific `it`-block
failure. The `cfheader` reset alone is sufficient.

Addresses Reviewer A on #2750.

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

bpamiri commented May 17, 2026

Addressed Reviewer A in 40fc7e4:

  • Correctness gap on sibling helpers: extended the plain-struct copy uniformly to all ten remaining attributeCollection = "#arguments#" sites in Global.cfc ($mail, $cache, $content, $directory, $file, $invoke, $location, $htmlhead, $wddx, $zip). The bot's reasoning that Adobe CF 2023's restriction is engine-level (not tag-specific) is the right framing — patching only the dispatch-path blocker would leave latent rejections on every other code path that reaches these helpers. Confirmed via grep -nE 'attributeCollection = "#arguments#"' vendor/wheels/Global.cfc returning zero hits after the change. Local Adobe 2023 verification was blocked by a Docker virtiofs mount issue against the worktree path; CI compat-matrix will be the authoritative check.
  • Test cleanup invokes function under test: dropped the g.$header() call from headerSpec.cfc's afterEach and reset content-type via cfheader directly. A regression now fails the specific it block instead of surfacing as an opaque afterEach lifecycle error.
  • Commit scope dispatch vs global: leaving as-is per the bot's own "defensible, not a blocker" assessment — the helper sits on the dispatch path and CLAUDE.md's commit conventions treat scope as unrestricted/optional.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

This follow-up commit directly addresses both open items from the previous review: (1) the other nine attributeCollection helpers in Global.cfc are now patched with the same plain-struct copy, and (2) the afterEach in headerSpec.cfc no longer calls the function under test. The PR is substantially improved. Leaving a comment (not requesting changes) on two residual points that warrant author confirmation before merge.


Correctness

Two attributeCollection sites in Global.cfc are still passing the raw arguments scope — different syntax, potentially the same engine restriction

// Global.cfc:61 — $image()
cfimage(attributeCollection = arguments);      // direct struct reference, no interpolation

// Global.cfc:425 / 427 / 430 / 439 — $dbinfo()
cfdbinfo(attributeCollection = arguments);     // same, four calls inside a try/catch

All ten patched helpers used the string-interpolation form attributeCollection = "#arguments#". These two use the CFScript direct-struct form attributeCollection = arguments. If Adobe CF 2023's restriction is on the special arguments scope type (regardless of interpolation), both forms fail. If the restriction is specific to the serialization path triggered by "#...#", the direct-struct form is fine and the exclusion is intentional.

$dbinfo() is the higher-priority question: databaseAdapters/Base.cfc:23-24 calls it during adapter initialization, and migrator/Base.cfc calls it in multiple migration helpers. If it throws on Adobe CF 2023 before the test suite boots, the matrix would still show few or no passes. The PR body's test-plan item ("goes from engine-down to a real result row") will confirm or deny this, but the reasoning behind the exclusion should be documented here regardless.

If the direct-struct form is indeed safe, please add a one-line comment at those two call sites (e.g., // CFScript direct-struct form: not subject to the arguments-scope restriction) so future reviewers don't re-investigate. If it is not safe, the same one-liner fix applies:

// $image() - after arguments.structName = "rv"
local.args = {};
for (local.key in arguments) {
    local.args[local.key] = arguments[local.key];
}
cfimage(attributeCollection = local.args);

// $dbinfo() - replace each cfdbinfo call in the try/catch
// (note: the loop that mutates arguments between cfdbinfo calls
//  must rebuild local.args after each StructDelete / assignment)

Tests

Regression coverage exists only for $header()

The nine other newly-patched helpers ($cache, $content, $mail, $directory, $file, $invoke, $location, $htmlhead, $wddx, $zip) have no BDD specs exercising the plain-struct copy path. For a fix explicitly motivated by engine-specific behavior, even a smoke-test notThrows assertion per helper would anchor the invariant for future readers and provide a signal if a helper regresses on Adobe CF 2023. Not a hard blocker for this PR, but worth a follow-up issue if not addressed here.


Cross-engine

The plain-struct copy pattern (local.args = {}; for key in arguments) is universally safe on Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. No new cross-engine risk from the changes themselves. The concern is only the two unpatched sites noted above.


Commits

Both commits conform to commitlint: fix(dispatch): with subjects under 100 chars. The scope dispatch for Global.cfc remains defensible as noted in the previous review — not a blocker.


Docs / CHANGELOG / Security

CHANGELOG entry updated to cover all ten sites and explain the engine-level restriction clearly. No security surface. No user-facing API change; guides update not required.

bpamiri and others added 2 commits May 16, 2026 20:03
…Spec.cfc

Lucee 7's parser fails on two adjacent `cfheader(...)` script calls in
the same block when neither terminates with a semicolon, throwing
`Syntax Error, Invalid Construct` at the closing `})` of the afterEach
arrow function. The error stops the entire vendor/wheels/tests/specs
bundle from compiling, so every test reports HTTP 500.

Semicolons on both cfheader lines disambiguate the statement boundary.
Pattern matches the existing precedent in
vendor/wheels/tests/specs/controller/renderingSpec.cfc:297.

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

Two helpers in Global.cfc used the CFScript direct-struct form
(`attributeCollection = arguments`, no `##` interpolation) rather than
the string-interpolated form patched in the previous commit. Adobe CF
2023's restriction is on the `arguments` scope shape — not on the
interpolation path — so both forms are at risk.

$dbinfo() is the higher-priority site: it is called from
databaseAdapters/Base.cfc during adapter initialization and from
migrator/Base.cfc on every migration helper. A throw here on Adobe CF
2023 would keep the test matrix red even after the dispatch-path fix.

$dbinfo()'s catch block mutates the arguments scope between its four
cfdbinfo calls, so local.args is rebuilt before each call rather than
copied once.

Addresses Reviewer A's follow-up on #2750.

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

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

The single new commit adds semicolons to the two back-to-back cfheader calls in headerSpec.cfc's afterEach block. The fix is correct: Lucee 7's CFScript parser cannot disambiguate adjacent cfheader(...) calls without a statement terminator, so the missing semicolons would crash the entire vendor/wheels/tests/specs bundle at compile time. The commit message explains the rationale clearly and cites the existing precedent in renderingSpec.cfc:297.

Two items from the round-2 review remain open. They are not new — I am reiterating them because no action (fix or explanation) has been taken.


Correctness

$image() and $dbinfo() still pass the raw arguments scope — not addressed or explained

vendor/wheels/Global.cfc:61:

cfimage(attributeCollection = arguments);      // direct struct reference — no copy, no interpolation

vendor/wheels/Global.cfc:425, 427, 430, 439:

cfdbinfo(attributeCollection = arguments);     // four calls inside the try/catch

All ten patched helpers used the string-interpolation form (attributeCollection = "#arguments#"). These two use the CFScript direct-reference form (attributeCollection = arguments). Whether Adobe CF 2023 treats them identically is the open question.

$dbinfo() is the higher-priority case: databaseAdapters/Base.cfc calls it during adapter initialisation, and the migrator calls it in multiple migration helpers. If Adobe CF 2023 also rejects the direct-struct form, the compat-matrix job would still record few or no passes after this PR lands, because the adapter layer runs before any test endpoint is dispatched.

One of two resolutions is needed before merge:

  1. If the direct-struct form is safe on Adobe CF 2023: add a brief inline comment at both call sites — e.g., // CFScript direct-reference form: not subject to the AC restriction on Adobe CF 2023 — so reviewers do not re-investigate.
  2. If it is not safe: apply the same one-liner copy pattern used in the ten patched helpers. For $dbinfo() the copy must be rebuilt after each StructDelete / reassignment that mutates arguments between the try/catch branches.

Cross-engine

The semicolon fix is universally safe. No new cross-engine risk from this commit.


Commits

test(global): add semicolons to back-to-back cfheader calls in headerSpec.cfc conforms to commitlint: type test, scope global, subject under 100 chars, not ALL-CAPS. DCO sign-off present.

@bpamiri
Copy link
Copy Markdown
Collaborator Author

bpamiri commented May 17, 2026

Addressed Reviewer A in ad0a03e:

  • Two remaining attributeCollection = arguments (direct-struct) sites — applied the same plain-struct copy pattern to $image() and $dbinfo(). The bot's question — "scope-type restriction or interpolation-path restriction?" — is the right one, and without local Adobe 2023 access I can't confirm which it is. Defensive choice: patch both forms so the fix is uniform regardless. $dbinfo()'s catch block mutates arguments between its four cfdbinfo calls, so local.args is rebuilt before each call rather than copied once.
  • Verified zero raw-arguments attributeCollection sites remain in Global.cfc: grep -nE 'attributeCollection = arguments|attributeCollection = "#arguments#"' vendor/wheels/Global.cfc returns no hits after the patch.
  • Regression coverage for the other helpers: holding off in this PR. Many of the newly-patched helpers have side effects that make a generic notThrows smoke test unsafe ($location aborts the current request via cflocation, $mail triggers SMTP, $file/$directory mutate the filesystem, $zip writes a zip archive, $invoke needs a target component). The bot's suggestion is reasonable, but each helper would need a bespoke fixture rather than a uniform smoke loop. Tracking as a follow-up rather than expanding scope here.
  • CHANGELOG updated to reflect all twelve patched sites and the two distinct argument-passing forms covered.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 17, 2026

Wheels Bot — Reviewer B (round 1)

A's review is substantively sound: correct findings, no sycophancy, no false positives. One commit (175b88fa) post-dates A's review and wasn't assessed; it is a trivial correct fix. I agree with A's open concern about $image() and $dbinfo() and recommend patching them to close the question definitively.

Sycophancy

None detected. A left a COMMENT verdict (not blocking) with concrete, cited findings — the right posture for a fix PR that is largely correct.

False positives

None detected. A's claim that $image() and $dbinfo() still use the direct-struct form attributeCollection = arguments is accurate — neither function appears in the diff. I verified both in the current file:

  • vendor/wheels/Global.cfc:61$image(): cfimage(attributeCollection = arguments)
  • vendor/wheels/Global.cfc:425,427,430,439$dbinfo(): four cfdbinfo(attributeCollection = arguments) calls inside a try/catch

A's technical framing is also correct: the 10 patched helpers all used the string-interpolation form "#arguments#"; these two use the direct-struct form. Whether Adobe CF 2023 rejects both forms or only the interpolation form is genuinely unknown without a live engine test.

Missed issues

175b88fa not assessed — A reviewed 40fc7e41f. A third commit landed after A's review: 175b88fa adds semicolons after the two back-to-back cfheader(...) calls in headerSpec.cfc's afterEach to fix a Lucee 7 parser error (Syntax Error, Invalid Construct at the closing }) of the arrow function). The fix is correct and matches the existing pattern at vendor/wheels/tests/specs/controller/renderingSpec.cfc:297. No action needed.

Verdict alignment

A's COMMENT verdict is consistent with the findings: two open questions (one potentially significant, one explicitly non-blocking), neither proven broken at review time.

Convergence

Aligned on findings; converging with changes. The consensus: patch $image() and $dbinfo() with the same plain-struct copy pattern. Rationale: the pattern is universally safe on every supported engine and eliminates the open question definitively, rather than leaving it to CI speculation.

Concrete changes for address-review:

$image() (line ~61) — add the copy before the cfimage call:

arguments.structName = "rv";
local.args = {};
for (local.key in arguments) {
    local.args[local.key] = arguments[local.key];
}
cfimage(attributeCollection = local.args);

$dbinfo() (lines ~425–439)arguments is mutated between the four cfdbinfo calls (arguments.type is changed to "dbnames" and back; arguments.dbname may be set), so local.args must be rebuilt before each call:

try {
    local.args = {};
    for (local.key in arguments) { local.args[local.key] = arguments[local.key]; }
    cfdbinfo(attributeCollection = local.args);
} catch (any e) {
    local.args = {};
    for (local.key in arguments) { local.args[local.key] = arguments[local.key]; }
    cfdbinfo(attributeCollection = local.args);
    local.type = arguments.type;
    arguments.type = "dbnames";
    local.args = {};
    for (local.key in arguments) { local.args[local.key] = arguments[local.key]; }
    cfdbinfo(attributeCollection = local.args);
    if (local.rv.recordCount GT 1) {
        for (local.i in local.rv) {
            if (local.i.database_name IS NOT "information_schema") {
                arguments.dbname = local.i.database_name;
            }
        }
    }
    arguments.type = local.type;
    local.args = {};
    for (local.key in arguments) { local.args[local.key] = arguments[local.key]; }
    cfdbinfo(attributeCollection = local.args);
}

The nine-helpers coverage gap (A's second finding) is non-blocking per A's own assessment — a follow-up issue is sufficient.

@bpamiri
Copy link
Copy Markdown
Collaborator Author

bpamiri commented May 17, 2026

The previous Reviewer A run was on 175b88fad and crossed in flight with ad0a03eef, which already patches both $image() (Global.cfc:61) and $dbinfo() (the four cfdbinfo calls at Global.cfc:425, 427, 430, 439) with the same plain-struct copy. Verified zero remaining raw-arguments sites — both forms — via grep -nE 'attributeCollection = arguments|attributeCollection = "#arguments#"' vendor/wheels/Global.cfc. The CHANGELOG line and PR comment posted after that push also enumerate the two newly-covered sites.

Reviewer A is currently re-running on ad0a03eef; expecting convergence once it completes.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 17, 2026

Wheels Bot — Reviewer B (round 1)

A's review was written for commit 175b88fad (the semicolons fix) but submitted when the HEAD had already advanced to ad0a03eef. The two "open items" A carries over are false positives against the current diff — both are addressed in ad0a03eef. The rest of A's analysis is correct. Aligning on approve.

Sycophancy

None detected.

False positives

  • $image() still passes raw arguments — A claims vendor/wheels/Global.cfc:61 still uses cfimage(attributeCollection = arguments) without a copy. The current HEAD diff shows ad0a03eef added the identical copy-loop before that call: local.args = {}; for (local.key in arguments) { local.args[local.key] = arguments[local.key]; } cfimage(attributeCollection = local.args);. A's claim is stale — it was true at 175b88fad but is not true at the current HEAD.

  • $dbinfo() still passes raw arguments at four call sites — same situation. A claims all four cfdbinfo(attributeCollection = arguments) calls in the try/catch are unpatched. The current HEAD diff shows ad0a03eef added a fresh copy-loop before each of the four calls. The rebuild-before-each-call approach is correct given that arguments.type is mutated between calls in the catch branch; bpamiri's note in the PR thread confirms this reasoning.

Both false positives arise because A analyzed the state after 175b88fad but the review was submitted against ad0a03eef. The marker in A's body (wheels-bot:review-a:2750:175b88fad...) confirms this — A did not re-evaluate the additional commit that landed just before submission.

Missed issues

None detected on the current full diff. Specific checks:

  • All 12 attributeCollection sites in Global.cfc now copy to a plain struct before passing to the underlying tag. Zero raw arguments references remain.
  • headerSpec.cfc: BDD syntax correct, semicolons present after back-to-back cfheader calls, afterEach uses direct cfheader rather than the function under test, four targeted notThrows cases. Appropriate coverage for a cross-engine smoke guard.
  • CHANGELOG entry is accurate and covers both argument-passing forms.
  • All commits conform to commitlint: types fix/test, subjects under 100 chars, not ALL-CAPS, DCO sign-offs present.
  • No cross-engine concerns introduced: the copy-loop idiom is universally supported across Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang.

Verdict alignment

A issued COMMENTED with two open items framed as blockers before merge. Given that both items are already addressed in the current HEAD, A's implicit verdict of "not ready to merge" is inconsistent with the current diff. The correct verdict is approve.

Convergence

Aligned on approve. A's two remaining findings are false positives against the current HEAD; no genuine open issues remain. The PR is a clean, well-scoped cross-engine compatibility fix with regression coverage.

wheels-bot[bot]
wheels-bot Bot previously approved these changes May 17, 2026
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

The open correctness concern from rounds 2 and 3 — $image() and $dbinfo() still passing the raw arguments scope as a direct CFScript struct reference — is addressed in the final commit (ad0a03e). All twelve attributeCollection sites in Global.cfc now use the plain-struct copy pattern. The PR is ready to merge.


Correctness

$image() — fix is correct

vendor/wheels/Global.cfc:60-65:

arguments.structName = "rv";
local.args = {};
for (local.key in arguments) {
    local.args[local.key] = arguments[local.key];
}
cfimage(attributeCollection = local.args);

arguments.structName = "rv" is set before the copy loop, so local.args includes it. cfimage writes its result to the variable named "rv"local.rv in function scope. local.rv = local.rv is correct. No behavioral change.

$dbinfo() — four copy sites, each rebuilt at the right moment

vendor/wheels/Global.cfc:428-459: Every local.args block is rebuilt after any mutation of arguments (arguments.type = "dbnames", conditional arguments.dbname assignment, arguments.type = local.type), so each cfdbinfo call sees the correct state. Pre-existing logic in the catch block (the immediate retry before the dbnames probe) is unchanged by this PR and is out of scope.


Tests

Regression coverage exists only for $header()

Noted in round 2 as "not a hard blocker for this PR, but worth a follow-up issue." The nine other newly-patched helpers ($cache, $content, $mail, $directory, $file, $cfinvoke, $location, $htmlhead, $wddx, $zip) have no BDD specs. Reiterated here for the follow-up issue rather than as a blocker.


Cross-engine

The plain-struct copy pattern is universally safe on Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. No new cross-engine risk introduced.


Commits

All four commits conform to commitlint: valid types (fix, test), defensible scope (dispatch, global), subjects under 100 chars, not ALL-CAPS. DCO sign-offs present.


Docs / CHANGELOG / Security

CHANGELOG entry covers all twelve patched sites and explains the engine-level restriction clearly. No security surface. No user-facing API change.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 17, 2026

Wheels Bot — Address Review (round 1)

No commit required at this SHA. Reviewer B's converged-changes verdict (wheels-bot:converged-changes:2750:175b88fad9291ba460ec704ccdf3094d87b0c417) requested the plain-struct copy pattern for $image() and $dbinfo(). That work landed in ad0a03eef — pushed by the PR author between B's verdict and this address-review run.

Verified at the current head ad0a03eef:

  • vendor/wheels/Global.cfc:61-65$image() rebuilds local.args and passes it as cfimage(attributeCollection = local.args).
  • vendor/wheels/Global.cfc:429-459$dbinfo() rebuilds local.args before each of the four cfdbinfo calls in the try/catch, matching B's spec exactly (the arguments scope is mutated between calls, so a single copy would not work).
  • grep -nE 'attributeCollection = arguments|attributeCollection = "#arguments#"' vendor/wheels/Global.cfc → 0 hits. No raw-arguments attributeCollection sites remain in either form.

Reviewer A's open follow-up about per-helper regression coverage for the other nine patched helpers ($mail, $cache, $content, $directory, $file, $invoke, $location, $htmlhead, $wddx, $zip) was non-blocking per A's own assessment and is appropriate for a follow-up issue — skipped intentionally per the consensus.

No new commit means no fresh Reviewer A trigger. The convergence loop should resolve on Reviewer A's pending re-run against ad0a03eef.

…fheader-adobe-2023

# Conflicts:
#	CHANGELOG.md
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

This is the round-5 review, triggered by the merge commit (519c729c5) that brought origin/develop into the PR branch. The core code changes — all twelve attributeCollection sites in Global.cfc patched plus headerSpec.cfc — are identical to what round 4 reviewed and cleared. The only substantive difference is the single commit from develop (89709af2c, CockroachDB advisory-lock override) landing via the merge. That commit is already on develop, so it will not create a duplicate on merge. The PR is ready to land.


Correctness

Merge commit is clean — no new code changes to PR files

519c729c5 merges origin/develop into the branch. The only commit it brings in is 89709af2c (CockroachDB $supportsAdvisoryLocks override), which is already on develop as PR #2746. No conflicts, no new changes to vendor/wheels/Global.cfc or vendor/wheels/tests/specs/global/headerSpec.cfc.

All twelve attributeCollection sites are patched

Every helper that forwarded the raw arguments scope to a tag's attributeCollection now uses the plain-struct copy pattern — verified in the current branch:

  • String-interpolation form ("#arguments#"): $cache, $content, $header, $mail, $directory, $file, $cfinvoke, $location, $htmlhead, $wddx, $zip
  • Direct-struct form (attributeCollection = arguments): $image, $dbinfo (four call sites)

$dbinfo() rebuilds local.args at the right moment

vendor/wheels/Global.cfc catch block: local.args is rebuilt after each mutation of arguments (arguments.type = "dbnames", conditional arguments.dbname assignment, arguments.type = local.type). Each cfdbinfo call therefore sees the correct argument state. arguments.name = "local.rv" is set before any copy, so all four calls write their result to local.rv.

$mail() and $location() correctly exclude non-tag arguments

$mail() removes mailparts, mailparams, and tagContent from arguments before the copy loop — those keys are handled separately inside the cfmail block and should not appear in attributeCollection. $location() removes $args unconditionally and delay before the copy — neither is a cflocation attribute.


Tests

headerSpec.cfc is clean

afterEach uses cfheader(...) directly (not g.$header()), so a regression in the function under test surfaces in the it block that triggered it rather than as a lifecycle error. Semicolons are present on both back-to-back cfheader calls, satisfying Lucee 7's parser.

Follow-up: nine other patched helpers have no BDD specs

$cache, $content, $mail, $directory, $file, $cfinvoke, $location, $htmlhead, $wddx, $zip have no notThrows smoke tests covering the plain-struct copy path. Noted in round 2 as a non-blocker; worth a follow-up issue to anchor the invariant for future readers.


Cross-engine

The plain-struct copy pattern (local.args = {}; for local.key in arguments) is universally safe on Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. No new cross-engine risk introduced by any commit in the PR.


Commits

All four PR commits conform to commitlint: valid types (fix, test), defensible scopes (dispatch, global), subjects under 100 chars, not ALL-CAPS. DCO sign-offs present on each.


Docs / CHANGELOG / Security

CHANGELOG entry updated to cover all twelve patched sites and explain the engine-level restriction clearly. No security surface. No user-facing API change; guides update not required.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 17, 2026

Wheels Bot — Reviewer B (round 1)

A's round-5 review is accurate and the APPROVED verdict is correct. The diff covers all patched sites, the test file is clean, and the merge-from-develop context is handled correctly. One trivial editorial note: A says "twelve attributeCollection sites" but lists 13 helpers in the breakdown (11 string-interpolation + $image + $dbinfo). This matches the CHANGELOG's own count, which excluded $header from the "twelve other helpers" sentence while still patching it — the code is correct regardless.

Sycophancy

None detected. A provides line-level evidence for each claim rather than a blanket LGTM.

False positives

None detected. Verified from the diff:

  • All 13 helpers ($cache, $content, $header, $mail, $directory, $file, $cfinvoke, $location, $htmlhead, $wddx, $zip, $image, $dbinfo) have the copy-loop pattern applied before the tag call.
  • $dbinfo() rebuilds local.args before each of its four cfdbinfo calls. The catch-path mutations (arguments.type, conditional arguments.dbname) land between copies, so each call sees the correct argument state. A's claim is accurate.
  • $mail() and $location() strip non-tag keys (tagContent, $args, delay) before the copy via StructDelete. A's claim is accurate.
  • $header() unconditionally strips statusText in the copy loop, collapsing the old branch. The regression spec covers the stripping case explicitly. All correct.
  • The merge commit (519c729c5) brings in only 89709af2c (CockroachDB advisory-lock override from develop). No conflicts, no new changes to the PR's target files.

Missed issues

None detected. Specific checks:

  • for (local.key in arguments) is universally supported across Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. No cross-engine risk.
  • headerSpec.cfc afterEach uses direct cfheader(...), not the function under test. Semicolons present on back-to-back tag calls (Lucee 7 parser requirement). Correct.
  • All four commits conform to commitlint (types fix/test, subjects under 100 chars, not ALL-CAPS, DCO sign-offs present).
  • Nine-helper follow-up noted as non-blocking — appropriate for a follow-up issue given the side-effect risk of bespoke fixtures for $mail, $location, etc.

Verdict alignment

APPROVED is consistent with the findings: all twelve (thirteen) sites patched, regression coverage in place, no open blockers.

Convergence

Aligned on approve. A's analysis is accurate, the diff is clean, and no genuine open issues remain.

@bpamiri bpamiri merged commit afcd208 into develop May 17, 2026
6 checks passed
@bpamiri bpamiri deleted the peter/issue-2741-cfheader-adobe-2023 branch May 17, 2026 03:37
bpamiri added a commit that referenced this pull request May 17, 2026
Reviewer A's round-2 sub-nit and round-3 finding (and Reviewer B's
convergence in the broader round) flagged that CLAUDE.md invariant #10
named only `cfheader`, while the underlying restriction applies to every
built-in CFML tag that accepts `attributeCollection`. PR #2750 has since
merged, patching all 12 affected wrappers in `vendor/wheels/Global.cfc`
uniformly — which is hard evidence the broader rule is correct.

Generalize invariant #10 in CLAUDE.md to "any built-in CFML tag",
enumerate the 12 affected wrappers, and call out the two attribute
forms (`"##arguments##"` and direct-struct). Update
`.ai/wheels/cross-engine-compatibility.md` to match: WRONG block now
shows both forms, the Reference link points to #2750 (the canonical
multi-site fix), and the $dbinfo() per-call rebuild note is preserved
as guidance for helpers that write through `arguments` between tag
invocations.

Docs-only — no code change. Addresses the substantive review feedback
that survived the rebase onto #2750.

Signed-off-by: Peter Amiri <peter@alurium.com>
bpamiri added a commit that referenced this pull request May 17, 2026
Reviewer A round 1 on #2755 caught a real correctness gap: develop's
`Global.cfc::$header()` comment explicitly says "Adobe CF 2023+ rejects
the raw arguments scope" — the `+` is load-bearing — and #2750 patched
all 13 wrappers precisely because the restriction extends past 2023.
The docs as previously written named only "Adobe CF 2023", giving a
developer reading invariant #10 no signal to guard their code on
Adobe CF 2025.

Updates:
- CLAUDE.md invariant #10: "Adobe CF 2023" → "Adobe CF 2023 and 2025"
  throughout (opener, throw clause, "require the plain struct" clause).
- .ai/wheels/cross-engine-compatibility.md: section heading is
  "(Adobe CF 2023/2025)"; intro and Why paragraph name both engines;
  WRONG block comment says "crashes Adobe CF 2023 and 2025".
- RIGHT example in the .ai/ doc now mirrors the WRONG block by showing
  both invocation forms working once `local.args` is a plain struct
  (cfheader interpolated form + cfimage direct form), with a brief
  comment explaining why both are safe at that point — addresses
  Reviewer A's non-blocking nit that the original RIGHT example showed
  only the string-interpolated form while $image() uses the direct form.

Engine matrix is unchanged otherwise: Lucee 6/7 + BoxLang +
Adobe 2018/2021 still accept; Adobe 2023/2025 reject.

Signed-off-by: Peter Amiri <peter@alurium.com>
bpamiri added a commit that referenced this pull request May 17, 2026
…CF 2023 (#2755)

* docs: document attributeCollection + arguments scope gotcha on Adobe CF 2023

Add cross-engine invariant #10 to CLAUDE.md and a matching section to
.ai/wheels/cross-engine-compatibility.md: Adobe CF 2023 rejects the raw
arguments scope passed as attributeCollection (e.g. cfheader), while
Lucee 6/7, BoxLang, and Adobe 2018/2021 accept it. Fix is to copy into
a plain struct first.

Follows fix in vendor/wheels/Global.cfc::$header (#2741).

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

* docs: generalize attributeCollection gotcha to any built-in CFML tag

Reviewer A's round-2 sub-nit and round-3 finding (and Reviewer B's
convergence in the broader round) flagged that CLAUDE.md invariant #10
named only `cfheader`, while the underlying restriction applies to every
built-in CFML tag that accepts `attributeCollection`. PR #2750 has since
merged, patching all 12 affected wrappers in `vendor/wheels/Global.cfc`
uniformly — which is hard evidence the broader rule is correct.

Generalize invariant #10 in CLAUDE.md to "any built-in CFML tag",
enumerate the 12 affected wrappers, and call out the two attribute
forms (`"##arguments##"` and direct-struct). Update
`.ai/wheels/cross-engine-compatibility.md` to match: WRONG block now
shows both forms, the Reference link points to #2750 (the canonical
multi-site fix), and the $dbinfo() per-call rebuild note is preserved
as guidance for helpers that write through `arguments` between tag
invocations.

Docs-only — no code change. Addresses the substantive review feedback
that survived the rebase onto #2750.

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

* docs(claude): use single-hash CFML interpolation in invariant #10 example

Reviewer A round 5: invariant #10's inline description of "the
string-interpolated form" used `"##arguments##"`. The double-hash is
CFML's escape for a literal `#` character — a reader copy-pasting it
into CFScript would get the string `#arguments#` rather than evaluating
the arguments scope, the opposite of what the invariant warns against.
The companion .ai/wheels/cross-engine-compatibility.md section at
line 238 already uses single hashes correctly; align CLAUDE.md to match.

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

* docs: correct wrapper count from 12 to 13 in attributeCollection invariant

Reviewer B round 7: the enumerated list of affected `Global.cfc`
wrappers has 13 items (cfheader, cfcache, cfcontent, cfmail,
cfdirectory, cffile, cflocation, cfhtmlhead, cfimage, cfdbinfo,
cfinvoke, cfwddx, cfzip) but both CLAUDE.md invariant #10 and
.ai/wheels/cross-engine-compatibility.md said "12". Verified against
develop's `vendor/wheels/Global.cfc` — 13 distinct wrapper functions
forward `arguments` to a `cf*(attributeCollection = ...)` site
(cfmailparam and cfmailpart are correctly excluded — they use the
pre-built `local.i` struct, not the arguments scope).

Pure number fix in two places.

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

* docs: extend attributeCollection invariant to Adobe CF 2023 and 2025

Reviewer A round 1 on #2755 caught a real correctness gap: develop's
`Global.cfc::$header()` comment explicitly says "Adobe CF 2023+ rejects
the raw arguments scope" — the `+` is load-bearing — and #2750 patched
all 13 wrappers precisely because the restriction extends past 2023.
The docs as previously written named only "Adobe CF 2023", giving a
developer reading invariant #10 no signal to guard their code on
Adobe CF 2025.

Updates:
- CLAUDE.md invariant #10: "Adobe CF 2023" → "Adobe CF 2023 and 2025"
  throughout (opener, throw clause, "require the plain struct" clause).
- .ai/wheels/cross-engine-compatibility.md: section heading is
  "(Adobe CF 2023/2025)"; intro and Why paragraph name both engines;
  WRONG block comment says "crashes Adobe CF 2023 and 2025".
- RIGHT example in the .ai/ doc now mirrors the WRONG block by showing
  both invocation forms working once `local.args` is a plain struct
  (cfheader interpolated form + cfimage direct form), with a brief
  comment explaining why both are safe at that point — addresses
  Reviewer A's non-blocking nit that the original RIGHT example showed
  only the string-interpolated form while $image() uses the direct form.

Engine matrix is unchanged otherwise: Lucee 6/7 + BoxLang +
Adobe 2018/2021 still accept; Adobe 2023/2025 reject.

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>
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.

Adobe CF (2023 + 2025): cfheader(attributeCollection) throws 'Failed to add HTML header' — blocks all tests

1 participant