Repository navigation
fix(global): copy arguments to struct in $header for Adobe CF 2023 compat - #2751
wheels-bot[bot] wants to merge 4 commits into
Conversation
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: PR #2751 applies a surgical one-branch-removed fix to `$header()` in `vendor/wheels/Global.cfc`, eliminating the Adobe CF 2023 regression where every request failed with `Failed to add HTML header` because `cfheader(attributeCollection="#arguments#")` rejects the `arguments` scope on that engine. The fix is correct, safe, and the new `headerSpec.cfc` follows established suite patterns. Only a minor style nit (multi-line comment block) keeps this from an outright approve — comment.
Correctness
The change is correct. Before the fix, `$header()` took two code paths:
statusTextpresent → copy `arguments` to plain struct, strip `statusText`, call `cfheader`statusTextabsent → call `cfheader(attributeCollection="#arguments#")` directly ← the path that fails on Adobe CF 2023
After the fix, the function always copies `arguments` to a plain struct and always strips `statusText` in one pass. Since `statusText` is absent from `arguments` in the common case, the for-loop simply skips it — no behavioral difference for callers that do not pass `statusText`.
Scanning all `$header()` callers in `vendor/wheels/` confirms no production caller passes `statusText`. `$setRequestStatusCode` in `rendering.cfc:771` calls `$header(statusCode = local.statusCode)` — the `local.statusText` variable computed at lines 764/768 is never forwarded to `$header()`. Stripping `statusText` unconditionally is therefore safe.
Cross-engine
No new cross-engine issues introduced. The struct-copy pattern was already proven on the `statusText`-present branch (added previously for Adobe CF 2025). This PR applies it to the only remaining code path. Lucee and BoxLang accepted the `arguments` scope directly and will continue to accept the plain-struct copy without change.
Tests
`vendor/wheels/tests/specs/global/headerSpec.cfc` covers four call shapes: `name`/`value`, `statuscode`, `name`/`value`/`charset`, and `statuscode`+`statusText` (the stripped case). The file follows the exact same structure as the neighboring `internalSpec.cfc` — `g = application.wo`, `$assert.notThrows(function(){...})`, arrow-function BDD sugar — confirming it uses established suite patterns. `$assert.notThrows` is the right assertion level here: the regression manifested as a thrown exception, and response headers cannot be inspected from within the test environment.
Docs
CHANGELOG updated under `[Unreleased] → ### Fixed`. Framework Docs, AI Reference Docs, and CLAUDE.md are delegated to `bot-update-docs.yml`, which is the established pattern for bot-authored PRs.
Commits
Single commit. Subject: fix(global): copy arguments to struct in $header for Adobe CF 2023 compat — 73 characters, valid type + scope, conventional-commit–compliant, accurate. DCO Signed-off-by: claude[bot] trailer is present.
Nits (non-blocking)
Multi-line comment block (vendor/wheels/Global.cfc, lines 113–117):
// Copy `arguments` into a plain struct before handing it to cfheader.
// Adobe CF 2023 rejects the arguments scope directly via attributeCollection
// ("Failed to add HTML header"); Lucee/BoxLang accept it but the uniform copy
// keeps every engine on one code path. Adobe CF 2025 also removed statusText,
// so strip it here.CLAUDE.md: "Never write multi-line comment blocks — one short line max." The WHY here is non-obvious enough to warrant a comment, but five lines exceeds the convention. Consider condensing before merge:
// Adobe CF 2023 rejects the arguments scope in attributeCollection; always copy to a plain struct.
// Strip statusText too — Adobe CF 2025 removed the attribute, and no production caller passes it.That preserves both the cross-engine rationale and the statusText context without the block. Non-blocking.
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The new commit (1e878ebc0) layers documentation onto the already-reviewed fix(global) commit: a new attributeCollection + arguments scope section in .ai/wheels/cross-engine-compatibility.md and invariant #10 in CLAUDE.md. Both additions are accurate and well-shaped. The one pre-existing nit — the 5-line comment block in Global.cfc flagged in the prior review — is still open but still non-blocking. Comment.
Correctness
No behavior changes in the new commit; it is documentation only. The prose in both files accurately describes the Adobe CF 2023 failure mode: the arguments scope is rejected by cfheader(attributeCollection=...) on that engine, the struct-copy pattern is the correct fix, and Lucee 6/7, BoxLang, and Adobe 2018/2021 all accept the plain-struct form without change.
CLAUDE.md invariant #10 correctly generalizes the lesson to "never pass raw arguments to attributeCollection". The .ai/wheels/cross-engine-compatibility.md section extends that to "built-in tags such as cfheader", which is the right level of generality for a reference doc.
Docs
.ai/wheels/cross-engine-compatibility.md — The new section follows the established pattern in the file: problem statement → WRONG/RIGHT code pair → explanation of why Adobe 2023 imposes a stricter type check → reference example back to the concrete fix site (Global.cfc::$header, #2741). Well-structured.
CLAUDE.md — Invariant #10 is a tight single-item addition after #9; it fits the existing numbered-list style and the subject is non-obvious enough to warrant inclusion. One sub-nit: the invariant text names only cfheader while the .ai/ doc hints the restriction applies to other attributeCollection-accepting built-ins as well. Not wrong — just more conservative than the reference doc. Non-blocking.
Commits
| Commit | Subject | Valid? |
|---|---|---|
e3c05e490 |
fix(global): copy arguments to struct in $header for Adobe CF 2023 compat |
✓ |
1e878ebc0 |
docs: document attributeCollection + arguments scope gotcha on Adobe CF 2023 |
✓ |
Both conform to commitlint.config.js: type from the valid allowlist, subject ≤ 100 chars, not ALL-CAPS, DCO Signed-off-by trailer present on both.
Pre-existing Nit (still open from previous review)
vendor/wheels/Global.cfc lines 113–117 — The 5-line comment block was flagged in the first review as exceeding CLAUDE.md's convention ("Never write multi-line comment blocks — one short line max"). The docs commit did not change Global.cfc, so the block remains. Non-blocking, but worth condensing before merge. The suggested two-line form from the previous review still applies:
// Adobe CF 2023 rejects the arguments scope in attributeCollection; always copy to a plain struct.
// Strip statusText too — Adobe CF 2025 removed the attribute, and no production caller passes it.
Wheels Bot — Reviewer B (round 1)A's technical analysis of the fix is accurate and thorough. One false positive in A's proposed resolution, one missed consistency issue in the tests, and a metadata bug in A's own marker. Converging on changes needed. SycophancyNone detected. A verified callers, traced the two code paths before and after, confirmed the struct-copy was already proven on the False positives
Missed issues
Verdict alignmentA's findings are concrete and actionable (file + lines + suggested replacement). "COMMENT with concrete actionable findings" is a valid trigger for ConvergenceAligned on changes needed. The consensus action is:
The underlying fix is correct. These are style-only changes. |
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Both changes are comment-only — no behavior change to Skipped (intentionally — not in consensus):
Test run note: The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on `approve` or the outer-loop cap (5 rounds) is reached. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: Commit 5a283ae63 correctly resolves the sole open nit from the two prior review rounds — the 5-line comment block in $header() is now a single line, and the 2-line comment in headerSpec.cfc is similarly condensed. The commit is comment-only and introduces nothing new. I have one new correctness concern to raise, pre-existing and not introduced by this PR, that should either be addressed here or tracked immediately as a follow-up: eight other Global.cfc wrappers still pass the raw arguments scope to attributeCollection and are therefore broken on Adobe CF 2023 by the same root cause this PR fixes. Comment.
Correctness
Prior nit — resolved. vendor/wheels/Global.cfc:113 is now:
// Adobe CF 2023 rejects the arguments scope as attributeCollection; copy to a plain struct and strip statusText (removed in Adobe CF 2025).One line, under the CLAUDE.md limit, preserves both the cross-engine rationale and the statusText context. vendor/wheels/tests/specs/global/headerSpec.cfc:33 is similarly a single line now. Both are correct.
New finding — pre-existing, not introduced here, but worth addressing.
vendor/wheels/Global.cfc contains eight other functions that still pass the raw arguments scope directly to attributeCollection:
line 61: cfimage(attributeCollection = arguments);
line 80: cfmail(attributeCollection = "#arguments#") { ... }
line 104: cfcache(attributeCollection = "#arguments#");
line 109: cfcontent(attributeCollection = "#arguments#");
line 200: cfdirectory(attributeCollection = "#arguments#");
line 205: cffile(attributeCollection = "#arguments#");
line 254: cflocation(attributeCollection = "#arguments#");
line 259: cfhtmlhead(attributeCollection = "#arguments#");
The .ai/wheels/cross-engine-compatibility.md section added by this PR correctly generalises the lesson to "built-in tags such as cfheader", which implies the fix should extend to all of the above. $location() (every redirect) and $content() (every file-download or content-type response) are called on nearly every non-trivial request path, so those two in particular are high-impact on Adobe CF 2023. $header() was catastrophic because it ran at boot/every-request — the others are not boot-level failures, but they will throw on Adobe CF 2023 as soon as a redirect or content response is issued.
Recommended resolution: either extend this PR with the same struct-copy pattern applied to the full list above, or open a tracking issue immediately and reference it from the CHANGELOG entry. Given CLAUDE.md invariant #10 now says "Never pass the raw arguments scope to cfheader(attributeCollection...)" — and the reference doc says "built-in tags such as cfheader" — the invariant should probably read "any built-in tag" rather than just cfheader to match the broader scope.
Docs
CLAUDE.md invariant #10 names only cfheader while .ai/wheels/cross-engine-compatibility.md correctly generalises to "built-in tags such as cfheader". These are slightly inconsistent. Consider aligning invariant #10 wording to reflect the broader rule:
// current
Never pass the raw `arguments` scope to `cfheader(attributeCollection="#arguments#")`.
// suggested
Never pass the raw `arguments` scope as `attributeCollection` to any built-in CFML tag.
Non-blocking on its own, but becomes load-bearing if the follow-up fix for the eight remaining functions is done separately.
Commits
| Commit | Subject | Valid? |
|---|---|---|
e3c05e490 |
fix(global): copy arguments to struct in $header for Adobe CF 2023 compat |
✓ |
1e878ebc0 |
docs: document attributeCollection + arguments scope gotcha on Adobe CF 2023 |
✓ |
5a283ae63 |
fix(global): address Reviewer A/B consensus findings (round 1) |
✓ (minor) |
5a283ae63's subject describes the reviewer process rather than the code change. Conventionally, commit messages should capture the "why" in terms of the code ("condense multi-line comment blocks in $header to one line per CLAUDE.md rule" would be clearer), but this is a non-blocking nit — the body is accurate and the type/scope/length all conform.
Wheels Bot -- Reviewer B (round 1)A correctly verified the round-1 consensus fix and raised a legitimate pre-existing finding about additional affected functions. One missed issue: the enumeration is incomplete -- four more functions pass the raw arguments scope to attributeCollection and were not listed. Not converging until the count is corrected. Sycophancy: None detected. A verified the prior nit before saying so and immediately raised a new substantive finding. False positives: None detected. Every function A named does pass the raw arguments scope to attributeCollection and is affected on Adobe CF 2023. Missed issues: A says "eight other functions" but four more exist in
A listed 8: cfimage (61), cfmail (80), cfcache (104), cfcontent (109), cfdirectory (200), cffile (205), cflocation (254), cfhtmlhead (259). Actual count: 12 call sites. Lines 83/90 (cfmailparam/cfmailpart) already use a local struct -- correctly excluded. Any tracking issue based on A list would miss the four above. Verdict alignment: COMMENT is correct for pre-existing issues not introduced by this PR. Recommendation to extend or open a tracking issue is appropriate. Convergence: Not aligned. A should confirm the corrected count (12 functions) and enumerate the full list so any follow-up tracking issue captures the true scope. The underlying analysis is sound; only the count needs updating. |
…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>
5a283ae to
b0b596c
Compare
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: PR #2751 has been rebased since the last review round: the vendor/wheels/Global.cfc fix and test file shipped via #2750; what remains here is the two-file documentation addition (.ai/wheels/cross-engine-compatibility.md and CLAUDE.md invariant #10). The docs are accurate, the code examples are correct, and the single open nit from round 3 — CLAUDE.md invariant #10 is worded more narrowly than the .ai/ reference doc — remains unresolved. Comment.
Correctness
Documentation-only commit. No behavior changes. The prose in both files accurately describes the Adobe CF 2023 failure mode and the struct-copy fix:
.ai/wheels/cross-engine-compatibility.md(line 238): correctly says "built-in tags such ascfheader" — appropriately general.CLAUDE.mdinvariant #10: says "Never pass the rawargumentsscope tocfheader(attributeCollection="#arguments#")" — correct, but scoped tocfheaderonly.
Docs
Open nit from round 3 (still unresolved): CLAUDE.md invariant #10 names cfheader specifically while the .ai/ section correctly generalises to "built-in tags such as cfheader". Round 3 also identified eight other Global.cfc wrappers (cfimage, cfmail, cfcache, cfcontent, cfdirectory, cffile, cflocation, cfhtmlhead) that still pass the raw arguments scope via attributeCollection. Those were intentionally deferred from this PR, but the narrower CLAUDE.md wording means a future developer reading the invariant will not know the restriction applies beyond cfheader. Consider aligning invariant #10 with the .ai/ doc:
Current:
10. **Adobe CF 2023 rejects the `arguments` scope as `attributeCollection`.** Never pass the raw `arguments` scope to `cfheader(attributeCollection="#arguments#")` — ...
Suggested:
10. **Adobe CF 2023 rejects the `arguments` scope as `attributeCollection`.** Never pass the raw `arguments` scope as `attributeCollection` to any built-in CFML tag (`cfheader`, `cfcontent`, `cflocation`, etc.) — ...
Non-blocking for merge, but load-bearing if the follow-up fix for the remaining eight Global.cfc wrappers is tracked separately rather than done here.
Commits
| Commit | Subject | Valid? |
|---|---|---|
b0b596ccd |
docs: document attributeCollection + arguments scope gotcha on Adobe CF 2023 |
✓ |
Type docs, no scope, subject under 100 chars, not ALL-CAPS. Conforms to commitlint.config.js.
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>
Rebase status + response to review rounds 1–2This PR has been rebased onto
The PR is now docs-only. The substantive feedback that still applies — Reviewer A's round-2 sub-nit and round-3 finding, plus Reviewer B's convergence noting that 12 (not 8) helpers were affected — is addressed in the new commit
CI status
Two reasonable paths forward — maintainer's call:
I haven't touched Reviewer findings that no longer apply
|
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: Commit `c9dd78c8e` resolves the sole open finding from round 4: CLAUDE.md invariant #10 now correctly names all 12 affected built-in CFML tag wrappers rather than only `cfheader`, and `.ai/wheels/cross-engine-compatibility.md` receives a matching WRONG/RIGHT block with a reference link to PR #2750. One new minor nit: the inline code in CLAUDE.md invariant #10 uses the escaped `##arguments##` form instead of the actual CFML code form `#arguments#`, inconsistent with the companion `.ai/` section. Non-blocking. Comment.
Correctness
Documentation-only commit; no behavior change. The content is accurate:
- `.ai/wheels/cross-engine-compatibility.md` line 238 correctly says both the string-interpolated form and the direct-struct form are affected, then shows a WRONG block with both, a RIGHT block with the struct-copy pattern, and a reference to #2750 for all 12 affected sites.
- CLAUDE.md invariant #10 now lists all 12 tag wrappers (`cfheader`, `cfcache`, `cfcontent`, `cfmail`, `cfdirectory`, `cffile`, `cflocation`, `cfhtmlhead`, `cfimage`, `cfdbinfo`, `cfinvoke`, `cfwddx`, `cfzip`), closing the narrowness finding from round 4.
The `$dbinfo()` per-call rebuild note preserved in the `.ai/` section is accurate and useful — `$dbinfo()` mutates `arguments` between its four `cfdbinfo` invocations, so a single upfront copy isn't sufficient.
Docs
Nit (non-blocking): `##arguments##` in CLAUDE.md invariant #10
CLAUDE.md (diff line, invariant #10):
Covers both the string-interpolated (`attributeCollection = "##arguments##"`) and direct-struct (`attributeCollection = arguments`) forms.
The double-hash `##` is the CFML template escape for a literal `#` character (it outputs the text `#arguments#`). In actual CFScript — the context shown in the companion WRONG block — you write single hashes:
cfheader(attributeCollection = "#arguments#"); // CFScript: evaluates argumentsThe `.ai/` doc at line 238 correctly uses single hashes:
Both the string-interpolated form (`attributeCollection = "#arguments#"`) ...
A developer reading CLAUDE.md invariant #10 who copy-pastes `"##arguments##"` would produce a literal string `#arguments#` rather than evaluating the scope — the opposite of what's being warned against. Suggested fix:
Covers both the string-interpolated (`attributeCollection = "#arguments#"`) and direct-struct (`attributeCollection = arguments`) forms.
Non-blocking for merge (the intent is clear from context), but worth correcting before this invariant is canonicalized.
Commits
| Commit | Subject | Valid? |
|---|---|---|
| `b0b596ccd` | `docs: document attributeCollection + arguments scope gotcha on Adobe CF 2023` | ✓ |
| `c9dd78c8e` | `docs: generalize attributeCollection gotcha to any built-in CFML tag` | ✓ |
Both conform to `commitlint.config.js`: type `docs` from the valid allowlist, subject ≤ 100 chars, not ALL-CAPS. DCO `Signed-off-by` trailers are present and match their respective commit authors (`claude[bot]` for `b0b596ccd`, `Peter Amiri peter@alurium.com` for `c9dd78c8e`).
…mple 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>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: Commit f1d372342 resolves the sole open finding from round 5 — CLAUDE.md invariant #10's inline code example now uses "#arguments#" (single hashes) instead of "##arguments##" (double hashes), matching the companion .ai/wheels/cross-engine-compatibility.md section at line 238 and correctly representing the CFML expression form to readers. Documentation-only change; no new issues. All findings across six review rounds are now resolved. Ready to merge.
Docs
The fix is precise and correct. The prior "##arguments##" form uses CFML's template escape for a literal # character — in a Markdown doc it renders to a reader as ##arguments##, not #arguments#, and someone copy-pasting it into CFScript would produce the string #arguments# rather than evaluating the scope, the opposite of what the invariant warns against.
Before (c9dd78c8e):
(`attributeCollection = "##arguments##"`)
After (f1d372342):
(`attributeCollection = "#arguments#"`)
The .ai/wheels/cross-engine-compatibility.md section at line 238 already had single hashes; CLAUDE.md now matches. The commit body captures the why faithfully — an exemplary one-line fix with full context.
Commits
| Commit | Subject | Valid? |
|---|---|---|
b0b596ccd |
docs: document attributeCollection + arguments scope gotcha on Adobe CF 2023 |
✓ |
c9dd78c8e |
docs: generalize attributeCollection gotcha to any built-in CFML tag |
✓ |
f1d372342 |
docs(claude): use single-hash CFML interpolation in invariant #10 example |
✓ |
All three: valid docs type, subject ≤ 100 chars, not ALL-CAPS, DCO Signed-off-by trailer present. f1d372342 is a human-authored commit (Peter Amiri <peter@alurium.com>) with a matching Signed-off-by trailer — clean.
Wheels Bot — Reviewer B (round 1)A's review is technically sound and the SycophancyNone detected. A's COMMENT verdict is proportionate for a docs-only PR; A did not approve prematurely and did not skip the correctness check on the new content. False positivesNone detected. A's Missed issuesCount discrepancy: both doc files say "12" but there are 13 wrappers.
Counting the list in each location: cfheader, cfcache, cfcontent, cfmail, cfdirectory, cffile, cflocation, cfhtmlhead, cfimage, cfdbinfo, cfinvoke, cfwddx, cfzip = 13 items, not 12. Verified against the #2750 merge commit ( Verdict alignmentCOMMENT is the correct category for a docs-only PR. A's characterisation of the ConvergenceNot aligned. A did not flag the 12-vs-13 count error, which is a concrete factual mistake in two places in the diff. Once A raises the count fix (or it is applied via address-review), the remaining open item is the |
|
Reviewer A round 5 ( |
…riant 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>
|
Reviewer B round 7 (12 → 13 wrapper count): fixed in |
Wheels Bot — Reviewer B (round 1)A's review is accurate and the core finding is sound. One factual inaccuracy in the review body (not the recommendation), no missed issues in the diff itself. Converging on changes needed. SycophancyNone detected. A verified the actual diff lines, identified a real inconsistency, and called COMMENT rather than approve. False positivesNone in the core finding. However, A's review text still states "eight other Missed issuesNone detected. The diff is two documentation files. A caught the only substantive problem: CLAUDE.md invariant 10 is narrower than the Verdict alignmentCOMMENT is the right verdict for a docs nit on a docs-only commit. Consistent with the finding. ConvergenceAligned on changes needed. The consensus action is a one-line edit to CLAUDE.md invariant 10: change the prohibition from |
|
Reviewer B round 1 against |
Wheels Bot -- Reviewer B (round 1)A's verification of the SycophancyNone detected. A verified the before/after diff directly, confirmed consistency with the companion False positivesNone detected. A's description of what Missed issues
Verdict alignmentA's COMMENTED verdict with "Ready to merge" is functionally appropriate for a docs-only PR -- the underlying analysis is sound and the commits are clean. The verdict is inconsistent only with A's own "all findings resolved" claim, which is undermined by the missed count error. ConvergenceAligned on changes needed. The one required action is:
That is the only open finding. The |
There was a problem hiding this comment.
Wheels Bot — Reviewer A response (round 1)
B's finding is correct. My prior review on f1d372342 was submitted at 03:52:33 — eight seconds before B's 12-vs-13 comment at 03:53:38 — so that finding was not accounted for. I concede B's point: both doc files still say "12" but enumerate 13 wrapper functions in their respective lists. No other concessions or defenses needed; B raised no false-positive claims against my findings.
Conceded points
- 12 vs 13 count is wrong in the current head. B correctly identified the discrepancy in
c9dd78c8e. Verified against the current committed head (f1d372342):CLAUDE.mdinvariant #10, line 49: "The 12 sites invendor/wheels/Global.cfc..." — but the preceding list contains 13 entries:cfheader / cfcache / cfcontent / cfmail / cfdirectory / cffile / cflocation / cfhtmlhead / cfimage / cfdbinfo / cfinvoke / cfwddx / cfzip..ai/wheels/cross-engine-compatibility.mdline 255: "patches all 12 affected wrappers..." — same 13-item list follows.
Thef1d372342commit only corrected the hash interpolation form and did not touch the count. The discrepancy is real and still present.
Defended points
None. B raised no false-positive claims against my findings.
Updated verdict
Still COMMENT — one factual correction remains: "12" → "13" in two places (CLAUDE.md invariant #10 and .ai/wheels/cross-engine-compatibility.md line 255). Both are single-character fixes with no structural changes to the docs. Once applied, all findings across the full exchange are resolved and the PR is ready to merge.
|
Reviewer A round 7 (concession) and Reviewer B round 7 ( |
Wheels Bot — Address Review (round 2)No code change required at this SHA. The latest
Verified at HEAD:
Skipped — not in any consensus and either rejected as bot-flow noise or already moot:
No commit pushed this round (nothing to address). The convergence loop should not iterate further: the next sensible step is a fresh Reviewer A run on HEAD |
Wheels Bot — Reviewer B (round 1)A's concession is substantively correct — the 12-vs-13 finding was real and A correctly accepted it. However A's verdict is stale: the fix A says still needs to be applied was already committed three minutes before A submitted this response. The current HEAD is clean and the PR is ready to approve. SycophancyNone detected. A did not approve prematurely; A conceded B's finding and maintained COMMENT rather than approving before the fix landed. False positives
Missed issuesNone detected. The diff touches only two documentation files. Code examples use the correct single-hash form ( Verdict alignmentA's verdict of COMMENT ("once applied, all findings are resolved") is self-contradicting given the current HEAD: the one remaining action A identifies is already done, which makes the correct verdict APPROVE. The underlying analysis — concede the count, confirm fix shape, no other findings — is sound; the verdict just hasn't caught up with ConvergenceAligned. All findings raised across the full exchange are addressed in the current HEAD: |
|
Superseded by #2755. This PR became docs-only after rebasing onto develop (PR #2750 merged the broader code fix for all 13 #2755 carries the identical content on a See full rationale in the rebase status comment. |
$header()previously passed theargumentsscope straight tocfheader(attributeCollection = "#arguments#"). Adobe ColdFusion 2023 rejects that shape withFailed to add HTML headerand aborts the request — every Wheels request on Adobe 2023 hit the error, including the test runner itself, so the compat-matrix job recorded0 pass / 0 fail / 0 errbecause no spec request got past application boot. Lucee, BoxLang, and earlier Adobe versions parse the same call without complaint.The function already had the struct-copy pattern on the
statusText-present branch (added when Adobe CF 2025 removed thestatusTextattribute). Applying that copy unconditionally — and continuing to stripstatusText— keeps every engine on one code path and removes the conditional branch.Adds
vendor/wheels/tests/specs/global/headerSpec.cfccovering the call shapes used across the framework: name/value, statuscode, name/value/charset, and statuscode+statusText. On Lucee the spec passes both before and after the fix (Lucee already accepts theargumentsscope), so locally it documents the cross-engine contract; the failure manifests on Adobe 2023 only and is verified by the compat-matrix job after merge.Fixes #2741
Pull Request
What does this PR do?
$header()now copies theargumentsscope into a plain struct before invokingcfheader(attributeCollection=…), so every supported engine — Lucee 6/7, Adobe CF 2018/2021/2023/2025, and BoxLang — sees a uniform input shape and Adobe 2023 stops throwingFailed to add HTML headeron every request.Where should the reviewer start?
vendor/wheels/Global.cfc— the$header()function (one branch, no caller updates).How should this be manually tested?
tools/test-matrix.sh adobe2023 sqliteonce the bot push lands — every spec request should now return JSON instead of the Wheels error page, andtotalPassshould match the Lucee 7 baseline. Re-checktools/test-matrix.sh adobe2025 sqliteto confirm Adobe 2025 stays green (the issue body flagged 2025 as pending).Any background context you want to provide?
Cross-engine
cfheaderparsing varies more than the docs suggest. Lucee 6/7 and BoxLang accept theargumentsscope asattributeCollection; Adobe 2018/2021 also accept it; Adobe 2023 rejects it with the cryptic "Failed to add HTML header" message; Adobe 2025 additionally removed thestatusTextattribute entirely. Folding all of that into one uniform code path (always copy, always stripstatusText) is simpler than maintaining engine-specific branches.What are the relevant tickets?
Fixes #2741.
Checklist
vendor/wheels/tests/specs/global/headerSpec.cfccovers the call shapes used across the framework. Passes locally on Lucee 7 (curl http://localhost:60007/wheels/core/tests?db=sqlite&format=json→ headerSpectotalPass: 4, totalFail: 0, totalError: 0); full-suite totals unchanged before/after the fix (3627 pass, 1 pre-existing fail, 40 pre-existing errors, 16 skipped). Adobe 2023 manifestation is what fails today and what the fix targets — verified post-merge via compat-matrix.bot-update-docs.ymlbot-update-docs.ymlbot-update-docs.yml[Unreleased]→### Fixedcompat-matrix.ymlonce the PR runs CI)