Skip to content

fix(compat): burn down four compat-matrix root causes (#3302) - #3365

Merged
bpamiri merged 12 commits into
developfrom
fix/3302-matrix-burndown
Aug 5, 2026
Merged

fix(compat): burn down four compat-matrix root causes (#3302)#3365
bpamiri merged 12 commits into
developfrom
fix/3302-matrix-burndown

Conversation

@bpamiri

@bpamiri bpamiri commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Burns down 4 of the 18 distinct root causes behind the 73 failing legs in the compatibility matrix, plus a fifth BoxLang parity fix. Refs #3302, which is explicitly blocked on this debt.

Three of the four are live framework bugs, not test defects. The matrix has been reporting them for weeks; nothing acted on them because compat-matrix.yml is continue-on-error: true and does not run on PRs.

Root causes

# Root cause Legs Engines
1 wheels.Public helpers never reach this 17 lucee6, adobe2023, adobe2025
2 Adobe discards typed default parameters 11 adobe2023, adobe2025
3 camelCase view file unreachable after path lowercasing 11 adobe2023, adobe2025
4 LocalDisk.put() corrupts stored content 5 adobe2025
5 $parseInsertColumnList engine fork 5 boxlang

1 — Public.cfc helper visibility

$init() does include "/wheels/public/helpers.cfm" inside a function body. The 27 UDFs that include declares reach variables but never this on Lucee 6, Adobe 2023 and Adobe 2025. Lucee 7 and BoxLang do promote them, which is why the split stayed invisible. Every helper in that file is declared public, and the framework's own views reach them through variables — so only an external caller is affected.

Global.cfc already solves exactly this for its own /app/global/functions.cfm include. Public.cfc never got the same treatment. Calls the inherited $scanAndPromoteIncludedGlobals() after the include.

The raw scan is deliberate: $promoteIncludedGlobalsToThis() memoizes its promote-list per class in application scope, and the entry for wheels.Public is written by the pseudo-constructor before $init() runs the include — so the memoized path would replay a stale pre-include key list and promote nothing.

2 — Adobe discards typed default parameters

Probed directly on Adobe 2025:

string default = ""   ->  arguments key is  STRING    (name swallowed)
string default        ->  arguments key is  STRING    (name swallowed)
default = ""          ->  arguments key is  DEFAULT   (correct)

Adobe binds the type token as the parameter name and drops default, so the declared default value never materializes. Explicitly-passed values still arrive as a separate key, which is what makes it so quiet — every call site that passes default= works, and only the declared default vanishes.

Not limited to the float() spec that caught it: uniqueidentifier() is declared string default = "newid()" and has been emitting DDL with no DEFAULT clause on Adobe for as long as it has existed. The type keyword is dropped from all 24 default declarations under vendor/wheels/.

3 — camelCase view file

$generateIncludeTemplatePath() ends with return LCase(local.rv), so a camelCase view filename is unreachable on a case-sensitive filesystem. Adobe enforces that; Lucee and BoxLang resolve the mismatch anyway.

_groupRow.cfm (added with #3152) was the only camelCase .cfm in the entire repository — one fixture, 11 legs. Renamed to match the convention, with ViewFileNamingGuardSpec so the next one fails on every engine instead of only the two Adobe legs.

LCase() itself is unchanged — it is load-bearing for controller-folder normalization and predates the rebrand, so rewriting it inside a burn-down is the wrong risk. The consequence for applications shipping a camelCase partial is real and undocumented; flagged separately rather than silently changed here.

4 — LocalDisk corrupts stored content

Byte-level probe on Adobe 2025: FileWrite(path, "hello world") puts 12 bytes on disk — 104 101 108 108 111 32 119 111 114 108 100 10. A trailing 0x0A. put() / get() stopped round-tripping and any binary payload came back one byte long. Decodes to binary before writing.

5 — $parseInsertColumnList fork

The non-BoxLang branch drops the comma delimiters when it runs on BoxLang (id,name,age -> idnameage). BaseProbe hard-codes $isBoxLangEngine() to false, so the unit spec drove exactly that branch on the boxlang legs. Collapsing the fork removes both the engine-dependent behaviour and the test-double trap; the regex form also preserves spaces inside quoted identifiers such as [order date], which ReplaceList stripped.

Verification

Run Result
lucee7 + sqlite, full core suite 4760 pass / 0 fail / 0 error, 353 bundles
adobe2023 — security / view / migrator / database 290/290, 581/581, 292 pass 0 fail, 63/63
adobe2025 — security / view / migrator / database / storage 290/290, 581/581, 292 pass 0 fail, 63/63, 27/27

Verification is on sqlite. Root causes 2 and 3 are adapter-independent code paths, so the remaining per-database legs follow from the same fix, but they are not separately claimed here.

Not yet verified on lucee6 or boxlang — both images are still building locally. Root cause 1's five lucee6 legs and root cause 5's five boxlang legs are expected to clear but are unconfirmed. Please hold merge until the matrix run on this branch confirms them, or until I post those results.

Also included

Two new cross-engine invariants in CLAUDE.md (typed default parameters; Adobe 2025 FileWrite trailing LF), plus two workflow notes that cost real time here: Adobe serves cached compiled classes, so ?reload=true does not pick up an edited .cfc — it needs a container restart, and until you know that a correct fix reads as a failed one. And directory= on the core-test endpoint turns a ~19-minute CI round-trip into ~5 seconds.

Still open on #3302

13 root causes / 24 legs: the BoxLang sendmail blank-line and Evaluate() pair, the cockroachdb transaction pairs, and the adobe2023 + Oracle tail. Continuing in follow-ups.

🤖 Generated with Claude Code

Four of the eighteen distinct root causes behind the 73 failing legs in
the compatibility matrix. Three were live framework bugs that the matrix
had been reporting for weeks without anyone able to act on them, because
compat-matrix.yml is continue-on-error and does not run on PRs.

Public.cfc — helpers.cfm is included inside $init(), and the UDFs that
include declares reach `variables` but not `this` on Lucee 6, Adobe 2023
and Adobe 2025 (Lucee 7 and BoxLang promote them, which hid the split).
Every helper there is declared `public`, so an external caller should be
able to reach it. Calls the inherited $scanAndPromoteIncludedGlobals()
after the include — the same fix Global.cfc already applies to its own
/app/global/functions.cfm include. Uses the raw scan rather than
$promoteIncludedGlobalsToThis(), whose per-class memo is written by the
pseudo-constructor before this include runs and would replay a stale
pre-include key list. 17 legs.

Migrator/adapters — Adobe parses `<type> default` as a parameter named
after the type keyword and discards `default`, so declared default values
never materialize. This was not limited to the float() spec that caught
it: uniqueidentifier() is declared `string default = "newid()"` and has
been emitting DDL with no DEFAULT clause on Adobe. Drops the type keyword
from all 24 `default` parameter declarations under vendor/wheels. 11 legs.

Views — $generateIncludeTemplatePath() lowercases the whole resolved path,
so a camelCase view file is unreachable on a case-sensitive filesystem.
Adobe enforces that; Lucee and BoxLang resolve it anyway. _groupRow.cfm
was the only camelCase .cfm in the repo. Renamed to match the convention,
with ViewFileNamingGuardSpec so the next one fails on every engine rather
than only on the two Adobe legs. 11 legs.

LocalDisk — Adobe 2025's FileWrite() appends a trailing 0x0A to simple
values, so put()/get() stopped round-tripping and binary payloads came
back one byte long. Decodes to binary before writing. 5 legs.

$parseInsertColumnList — collapses an engine fork whose non-BoxLang branch
dropped the comma delimiters when it ran on BoxLang, which is exactly what
BaseProbe's hard-coded boxlangMode=false made it do. The unified regex form
also preserves spaces inside quoted identifiers. 5 legs.

Verified: full core suite on lucee7 + sqlite is 4760 pass / 0 fail /
0 error. security, view, migrator, database and storage directories are
green on both adobe2023 and adobe2025.

Refs #3302

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

@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

TL;DR — This PR burns down four (plus one BoxLang-parity) root causes behind the compat-matrix failures: wheels.Public helper visibility, Adobe's typed-default-parameter swallow, the camelCase view-path trap, LocalDisk binary-write corruption, and the $parseInsertColumnList engine fork. I reviewed every hunk against the actual code paths and could not refute a single fix — each is well-reasoned, matches existing prior art, and (where behavior changed) is covered by a spec. Verdict: comment, one step short of approve solely because the author flags that two of the five target engines (lucee6, boxlang) are still unverified — see the caveat below. No correctness, cross-engine, or security defect fired.

Correctness

All five fixes verified sound against the source:

  • Public.cfc:24 — calling the raw $scanAndPromoteIncludedGlobals() rather than the memoizing $promoteIncludedGlobalsToThis() is correct: Global.cfc:4696-4733 writes the per-class promote-list during the pseudo-constructor (before $init() runs the include), so the memoized path would replay a stale pre-include key set. The raw scan at Global.cfc:4741 re-reads variables live. Same fix as Global.cfc's own /app/global/functions.cfm promotion.
  • LocalDisk.cfc:42-43IsBinary() guard + CharsetDecode(content,"utf-8") round-trips cleanly with get() at line 55, which already returns FileReadBinary. The empty-string and binary paths both behave.
  • databaseAdapters/Base.cfc:164-183 — collapsing the $isBoxLangEngine() fork to the single regex form is behavior-preserving for Lucee/Adobe and fixes the delimiter-drop on BoxLang; the quoted-identifier space preservation is a genuine improvement over ReplaceList.

Conventions

  • Dropping string from the default parameter (24 sites) is safe: no adapter declares implements=\"...DatabaseMigratorAdapterInterface\" (only DatabaseModelAdapterInterface and DatabaseInterfaceSpec reference it), so the interface-signature edit at DatabaseMigratorAdapterInterface.cfc:57 triggers no runtime enforcement. A repo-wide grep confirms no typed default params remain under vendor/wheels/ outside tests.

Cross-engine

  • The two new invariants (#17 typed-default, #18 Adobe-2025 FileWrite trailing-LF) and the ?reload=true/directory= workflow notes in CLAUDE.md are accurate and match the code they describe. Good documentation of hard-won gotchas.
  • Verification gap (hold-merge caveat, not a code defect): the PR body states lucee6 (root cause 1, 5 legs) and boxlang (root cause 5, 5 legs) are not yet confirmed on this branch. For a change whose entire purpose is cross-engine correctness, please confirm the matrix on both before merging — this is why I'm leaving a comment rather than an approval.

Tests

  • AdapterIdentityTemplateSpec.cfc now loops boxlangMode over [false, true] and asserts identical output; BaseProbe.$isBoxLangEngine() returns that flag, so both branches are genuinely exercised — a reintroduced fork fails here. The added quoted-identifier case pins the space-preservation improvement.
  • ViewFileNamingGuardSpec.cfc is a sound structural guard; its scope note honestly limits it to the framework's own view trees.
  • The LocalDisk fix is covered by the existing StorageSpec.cfc:99-103 round-trip (ToString(content) toBe \"hello world\") — the exact assertion the Adobe-2025 trailing-LF bug broke. No new spec needed.

Docs

  • Four changelog.d/*.fixed.md fragments, correct <slug>.<type>.md shape, no direct CHANGELOG.md edit. The view-rename fix (root cause 3) ships no fragment, which is defensible — it changes no user-facing framework behavior (test-fixture rename + guard), and the app-facing camelCase limitation is explicitly deferred rather than silently changed.

Commits

  • fix(compat): burn down four compat-matrix root causes (#3302) — valid conventional-commit header (~62 chars), DCO Signed-off-by: present. Subject says "four" while the diff also carries the fifth BoxLang-parity fix, but the body frames it as "four … plus a fifth," so this is intentional, not a defect.

Clean, disciplined burn-down. My only ask is the pre-merge lucee6/boxlang matrix confirmation the author already called out.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Test Results

     32 files   10 024 suites   23m 22s ⏱️
132 573 tests 132 170 ✅ 397 💤 6 ❌
134 505 runs  134 102 ✅ 397 💤 6 ❌

For more details on these failures, see this check.

Results for commit 5417e51.

♻️ This comment has been updated with latest results.

bpamiri added 6 commits August 4, 2026 11:58
…3302)

Adobe rejects a nested cftransaction whose isolation level differs from
its parent's. CockroachDBTransactionSpec's updateAll case wrapped a
Wheels call — which opens its own transaction with isolation
"read_committed" via invokeWithTransaction — in a raw `transaction
action="begin"` that declared none, so both Adobe legs threw. Lucee and
BoxLang do not enforce the rule, and the spec is CockroachDB-gated, which
is why it presented as a cockroachdb-only failure.

The knock-on is the more interesting half. invokeWithTransaction sets
request.wheels.transactions[connectionArgs] = true BEFORE opening the
tag, and the tag itself sits outside the try/catch that would reset it.
When `transaction action="begin"` throws, that marker is never cleared,
so every subsequent invokeWithTransaction in the same request takes the
"alreadyopen" path and silently skips its own transaction. The whole core
suite runs in one request, so a single throwing begin disabled model
transaction handling for everything after it — which is exactly why
OuterTransactionSignalSpec's rollback assertion reported 10 rows instead
of 9 on the same two legs. Fixing the isolation clears both.

Verified on adobe2025 + cockroachdb: database directory 90/90, and the
full core suite 4771 pass / 1 fail — the remaining failure being
JobClassRoundTripSpec, which reproduces identically on 8bc8304 with
these changes stashed and is therefore pre-existing, not a regression.
It was previously masked by this very bug.

Refs #3302

Co-authored-by: Peter Amiri <peter@alurium.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
BoxLang ships no `Evaluate()` BIF — a probe on 1.11.0 returns "The method
Evaluate does not exist" — so every expression that reached the built-in
branch of `$evaluateExpression()` came back as the error string rather
than a result. Five legs, one per database.

`executeStatement()` on the BoxLang runtime is the faithful equivalent:
like `Evaluate` it takes the whole expression string, so neither branch
has to re-parse the argument list, and the two paths stay
behaviourally identical. Function calls resolve at runtime, so the
BoxLang-only `getBoxRuntime()` name never has to compile on Lucee or
Adobe; the branch is selected on `server.boxlang`, which no other
engine defines.

Refs #3302

Co-authored-by: Peter Amiri <peter@alurium.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…ding it (#3302)

`sendEmail` writes CRLFCRLF between the text and HTML parts, and the
spec matched that byte sequence literally. BoxLang's `cffile` write
normalizes CRLF to LF on the way to disk — a byte-level probe showed a
10-byte "AAA\r\n\r\nBBB" payload landing as 8 bytes — so the needle was
never found on any of the five boxlang legs while Lucee and Adobe
passed. The failure message was the giveaway: needle and haystack
printed identically because the difference was invisible.

Normalize line endings before matching. The encoding of a debug
artifact's line breaks is the engine's business; the blank line
separating the two bodies is the behaviour under test.

Refs #3302

Co-authored-by: Peter Amiri <peter@alurium.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…ws (#3302)

494ea50 stopped CockroachDBTransactionSpec from throwing on Adobe.
This fixes the mechanism that turned that one throw into a second,
unrelated failure several bundles later, so the next spec that fails to
open a transaction cannot do the same thing again.

`invokeWithTransaction()` sets
`request.wheels.transactions[connectionArgs]` to true *before* it opens
the `cftransaction`, and the tag sits outside the try/catch that resets
the marker. So whenever the begin tag itself throws — an unsupported
isolation level, Adobe's nested-isolation-mismatch rule, a dead
connection — the marker stayed true and every subsequent
`invokeWithTransaction` in the same request took the "alreadyopen"
branch and ran with no transaction at all. Silent, and it does not
recover until the request ends. The whole core suite runs in one
request, which is how a single throwing begin in
CockroachDBTransactionSpec went on to fail OuterTransactionSignalSpec's
rollback assertion with "Expected [9] but received [10]". Two legs,
adobe2023 and adobe2025 on cockroachdb.

Wrapping the tag in its own try/catch clears the marker on that path
too. Resetting twice is harmless: the inner catch already clears the
same flag before it rethrows.

TransactionMarkerResetSpec pins it with an invalid isolation level,
which is the portable way to make the begin tag fail on every engine.

Verified: full core suite on lucee7 + sqlite, 4761 pass / 0 fail /
0 error across 354 bundles.

Refs #3302

Co-authored-by: Peter Amiri <peter@alurium.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…Lang (#3302)

`equalize()` is a cascade of type-pair checks that each return true on a
match, with `actual.equals(expected)` at the bottom as an
object-identity catch-all. Nothing kept simple values out of that
catch-all, so every *unequal* pair of numbers or strings reached it. On
Lucee and Adobe that was harmless — the boxed Java `.equals()` returns
false, the right answer by accident. On BoxLang a simple value resolves
`.equals()` to the DateTime member function and throws, so a plain
numeric mismatch was reported as an ERROR reading "Can't cast [2] to a
DateTime" rather than a FAILURE reading "Expected [2] but received [0]".

This only fires on the unhappy path, which is why it has been invisible:
it does not cause failures, it misdescribes them. Every BoxLang
numeric-or-string mismatch in the suite has been reporting the wrong
reason. The channel `DatabaseAdapterSpec` maxRows leg on boxlang +
postgres/cockroachdb is the current instance; the real mismatch behind
it is still open.

Two simple values that did not match the checks above are unequal, so
say so before the object fallback. `eq` is looser than `.equals()`
(numeric coercion, case-insensitive strings), so anything `eq` rejects
`.equals()` would have rejected too — no verdict changes on any engine.

Refs #3302

Co-authored-by: Peter Amiri <peter@alurium.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…ture (#3302)

The rename spec hand-writes `CREATE TABLE c_o_r_e_migrator_versions
(version VARCHAR(25), core_level INT NOT NULL DEFAULT 1)`. Oracle
rejects that column-constraint order with ORA-03076 "unexpected item
DEFAULT in a column definition or inline constraint" — the SQL standard
puts the default clause before the column constraints, and Oracle
enforces it while MySQL, PostgreSQL, SQL Server, SQLite and H2 accept
either order. One leg, adobe2023 + oracle, the only engine/database
pair that runs Oracle at all.

`DEFAULT 1 NOT NULL` parses everywhere, so this is a straight
reordering rather than an engine branch. Migrator.cfc already emits the
Oracle-safe order for the tracking table it creates itself
(`NUMBER DEFAULT 1 NOT NULL`); only the fixture had drifted.

Refs #3302

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

@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

TL;DR — This is a re-review at head 2e3215f98. The prior review already cleared the original four-root-cause burn-down at 6dc0251a; since then six commits have landed adding a genuine framework bug fix (open-transaction marker reset in invokeWithTransaction) and five BoxLang / Adobe / Oracle test-parity fixes. I read every new hunk against the actual code paths and could not refute a single one — each matches established repo prior art and, where framework behavior changed, is covered by a spec. Verdict: comment, one step short of approve solely because the author explicitly asks to hold merge until the lucee6/boxlang matrix confirms on this branch (see caveat). No correctness, cross-engine, or security defect fired.

Correctness

  • vendor/wheels/model/transactions.cfc:69-85 — the new outer try around transaction action="begin" correctly closes the leak: the marker is set true at :51 before the tag opens, and a throw from the begin tag itself (bad isolation, Adobe nested-isolation mismatch, dead connection) previously skipped the reset. Success path still clears via local.closeTransaction at :100-102; the inner catch (:76-80) and outer catch (:82-85) both reset the same request-scoped flag and rethrow, so the documented double-reset is idempotent and harmless. Importantly the resets write request.wheels.transactions[...] — request scope, not local. — so cross-engine invariant 11 (BoxLang discards local. writes inside catch) does not bite.
  • vendor/wheels/wheelstest/system/Assertion.cfc:1296-1298 — the early return false for two non-matching simple values is placed after the numeric (:1274) and simple-value (:1283) equality checks, and before the query/UDF/XML/array/object branches, all of which require non-simple operands. So it only short-circuits the object-oriented .equals() fallback for genuinely-unequal simple values — behavior-preserving on every engine and it stops BoxLang from resolving .equals() to the DateTime member function. Mixed simple-vs-object comparisons still fall through correctly (both operands must be simple to trigger the early return).
  • vendor/wheels/Test.cfc:771-772getBoxRuntime().executeStatement() is guarded by StructKeyExists(server, "boxlang") and getBoxRuntime appears nowhere else (grep confirms), so the BoxLang-only symbol resolves at runtime and never has to compile on Lucee/Adobe. The server.boxlang probe is well-established prior art (Global.cfc:2474/2494/2500, Base.cfc:152, populate.cfm:485).

Cross-engine

  • CockroachDBTransactionSpec.cfc:57 adds isolation="read_committed" to the outer cftransaction so the nested invokeWithTransaction opens with a matching level — correctly targets Adobe's "nested cftransaction must specify same isolation level as the parent" rule, which Lucee/BoxLang don't enforce.
  • migratorSpec.cfc:315 reorders INT DEFAULT 1 NOT NULL for Oracle (ORA-03076 on the reversed form); portable on MySQL/PG/MSSQL/SQLite/H2.
  • miscellaneousSpec.cfc:497-506 normalizes CRLF->LF before the blank-line assertion — sound BoxLang cffile-normalization parity fix; Replace(...,"all") is cross-engine safe.

Tests

  • TransactionMarkerResetSpec.cfc directly exercises the marker-reset fix and correctly follows cross-engine invariant 11 (struct state accessed without the local. prefix inside catch). It self-guards: if an engine silently accepts the bogus isolation level, state.threw is false and the test fails loudly with an explanatory message rather than producing a false pass — a good failure mode.

Docs

  • Three new changelog.d/3302-*.fixed.md fragments (evaluate-expression, crdb-isolation, transaction-marker-reset), correct <slug>.<type>.md shape, no direct CHANGELOG.md edit. The test-only parity fixes (assert simple-value, migrator DEFAULT order, sendEmail blank line) ship no fragment, which is defensible — they change no user-facing framework behavior.

Commits

  • All six new commits are valid conventional-commit headers under 100 chars, not ALL-CAPS, each carrying (#3302). fix(test): ... on Test.cfc infrastructure and fix(model): ... on transactions.cfc are appropriately typed; scope is unrestricted so test/model/assert/controller/migrator all pass commitlint.

Verification caveat (hold-merge, not a code defect)

Per the PR body, lucee6 and boxlang legs remain unconfirmed on this branch. Two of the new commits (fix(test): evaluate built-in expressions via the BoxLang runtime, test(assert): report unequal simple values ...) exist specifically to clear boxlang legs, so please confirm the matrix on lucee6 + boxlang before merging — matching the author's own request to hold. That unverified-engine gap is why this is a comment rather than an approval; the diff itself is clean.

…base (#3302)

`cleanup(maxRows=)` builds its candidate SELECT through `$applyRowBound`
and pairs it with a list-parameter DELETE, and the whole pass sits
inside a catch that logs to `wheels_channels` and returns 0. The five
existing `$applyRowBound` tests only compare strings — nothing sends the
rewritten SQL to a database, and nothing exercises the DELETE at all. So
when an engine/database pair rejects either statement, the only symptom
is a wrong row count: `Expected [2] but received [0]` on boxlang +
postgres and cockroachdb, with the reason visible nowhere the suite can
reach.

This runs both statements without the catch, so the database's own error
is what the suite reports. It asserts against the id the bounded SELECT
returned rather than the row it inserted, because ORDER BY createdAt ASC
takes the oldest expired row in the table and that need not be ours.

`$detectDatabaseType()` becomes public with the `$` prefix — the same
internal-naming convention its caller `$applyRowBound` already uses, and
for the same reason — so the spec can name the dialect the bounded pass
actually chose in its failure messages.

The underlying boxlang + postgres/cockroachdb defect is still open; this
is what will name it. Local Docker was unavailable this session (the
daemon died on every attempt to bring up a BoxLang container), so this
diagnosis goes through CI rather than a container probe.

Verified: full core suite on lucee7 + sqlite, 4762 pass / 0 fail /
0 error across 354 bundles — one more than the previous run, which is
the spec added here being discovered.

Refs #3302

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

@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

TL;DR — Burns down four compat-matrix root causes plus a BoxLang parity fix for #3302, three of which are live framework bugs (Public.cfc helper visibility, Adobe-swallowed typed default params, LocalDisk byte corruption, transaction-marker leak). I re-derived each root cause against source, tried to refute each fix, and every one holds up: the changes are correct, complete across all code paths, and backed by existing or new specs. Verdict: comment — no correctness, cross-engine, or security finding fires; one out-of-scope docs nit is the only thing worth noting. Conservative first-pass comment (blocks nothing); the author has already flagged lucee6/boxlang matrix confirmation as pending, which is the right gate for merge.

Correctness

Verified, no findings:

  • vendor/wheels/model/transactions.cfc:69-84 — the new outer try/catch resets request.wheels.transactions[local.connectionArgs] when transaction action="begin" throws before the inner block. All three paths are covered: begin-throws → outer catch resets + rethrows; inner-throws → inner resets + rethrows through the (harmless) outer reset; success → local.closeTransaction resets at :100. Double-reset is idempotent. Correct.
  • vendor/wheels/storage/drivers/LocalDisk.cfc:40IsBinary(content) ? content : CharsetDecode(content, "utf-8") round-trips cleanly because get() already returns FileReadBinary() (:53). On Lucee the written bytes are identical to the pre-change FileWrite(string), so no regression; the fix only changes Adobe 2025's trailing-0x0A behaviour. Correct.
  • vendor/wheels/wheelstest/system/Assertion.cfc:1293 — the isSimpleValue(actual) && isSimpleValue(expected) → return false guard is placed after the numeric-equal and simple-equal return true branches, so it only fires on a genuine mismatch and correctly short-circuits before the object .equals() fallback. Correct.

Cross-engine

Verified, no findings:

  • vendor/wheels/Test.cfc:772getBoxRuntime().executeStatement(...) is behind a StructKeyExists(server, "boxlang") runtime guard, and CFML resolves function names at call time, so the BoxLang-only symbol never has to compile on Lucee/Adobe. Correct — no prior art in the repo, but the mechanism is sound.
  • The default-parameter retyping is complete and consistent: every declaration under vendor/wheels/ (Migration.cfc, TableDefinition.cfc, Abstract.cfc, MySQLMigrator.cfc, SQLiteMigrator.cfc) is now untyped, and DatabaseMigratorAdapterInterface.cfc:57 was updated to match so the implements contract stays intact. git grep confirms no typed default param remains in the migrator/adapter tree. Correct.
  • vendor/wheels/Public.cfc:22$scanAndPromoteIncludedGlobals() (inherited from Global.cfc:4741) iterates variables, promotes custom functions absent from this, which is exactly the post-include state needed. The comment's rationale for not using the memoizing $promoteIncludedGlobalsToThis() wrapper is accurate. Correct.

Tests

Solid coverage — no findings:

  • Root cause 1 (Public.cfc) is exercised by existing specs that call createObject("component","wheels.Public").$init() then invoke $-helpers on this (InvokeMethodSpec, CliEndpointHardeningSpec, DevAssetServingSpec).
  • New guards are well-designed: ViewFileNamingGuardSpec (structural, fails on every engine instead of only the two Adobe legs), TransactionMarkerResetSpec (portable invalid-isolation trigger, self-documenting assertion messages, correctly uses a non-local.-prefixed struct per cross-engine invariant 11), and AdapterIdentityTemplateSpec now pins both boxlangMode flags to the same expectation.
  • DatabaseAdapterSpec's live-DB test defensively asserts against the id the bounded SELECT actually returned rather than the inserted row, and cleans up its own channel — resilient to leftover rows from prior bundles.

Docs

One minor, out-of-scope nit (non-blocking):

  • vendor/wheels/migrator/CLAUDE.md:12 still shows public any function string(string columnNames, any limit, string default, boolean allowNull) as the canonical helper shape. This PR establishes cross-engine invariant 17 (typed default breaks on Adobe) and untypes every real declaration — the doc example now demonstrates the anti-pattern the PR just eliminated. Not touched by this diff, so it's a follow-up, but worth folding in since this file is the reference agents read when adding migrator helpers.

Commits

All eight commits conform to commitlint.config.js (valid types, lowercase subjects, ≤100 chars, #3302 refs). fix(test): declare matching isolation… touches only a spec so test(...) would read more naturally, but fix is a valid type and this is not a violation.

Nicely done — the inline comments and CLAUDE.md invariants make each fix auditable, and the "Adobe serves cached compiled classes / ?reload=true does not pick up edits" note is exactly the kind of workflow trap worth codifying. Holding merge on the pending lucee6/boxlang matrix run is the right call.

#3302)

Named by the probe added in 65a5104, which is the whole reason that
commit exists:

    Method org.postgresql.jdbc.PgPreparedStatement.setLargeMaxRows
    is not yet implemented.

`cleanup(maxRows=)` pushes the row bound into dialect SQL (TOP / FETCH
FIRST / LIMIT) *and* set the driver-level `maxrows` query option "as
belt-and-braces". On BoxLang that option reaches the PostgreSQL driver
as `setLargeMaxRows()`, which pgjdbc has never implemented, so the
candidate SELECT threw, the catch-all swallowed it, and `cleanup()`
reported 0 rows deleted on both PostgreSQL and CockroachDB — which share
that driver. Expired `wheels_events` rows were never being reclaimed on
either database under BoxLang. Two legs.

The option is now used only when `$applyRowBound` applied nothing, which
it signals by returning the statement unchanged. There it is the only
bound on the read and worth its portability risk; everywhere else the
SQL already bounds the result and the option is pure downside.

This is a repeat, not a new discovery. `JobWorker.$claimNext` bounds the
identical way and carries a NOTE — "Avoid the maxrows option — BoxLang +
PostgreSQL throws when setMaxRows() is called on the JDBC
PreparedStatement" — added when it hit this. `cleanup()` copied the
dialect-SQL half of that pattern and kept the option anyway.

Two `maxrows` callers remain, `Job.$processQueue` and three in
JobWorker. All four run green on boxlang + postgres today, so they are
not the same failure and I have not touched them; worth a look at
whether the difference is real or luck.

Verified: full core suite on lucee7 + sqlite, 4762 pass / 0 fail /
0 error across 354 bundles. The boxlang + postgres/cockroachdb legs go
to CI — Docker died on every attempt to raise a container locally today.

Refs #3302

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

@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

TL;DR — Re-review at head 9a3cff91f. Since the last reviewed commit (2e3215f98) two commits landed: 65a51047f adds a live-database exercise of the channel bounded-cleanup pass, and 9a3cff91f drops the redundant driver-level maxrows option that pgjdbc's unimplemented setLargeMaxRows() was throwing on under BoxLang + PostgreSQL/CockroachDB. Both are correct, well-commented, tested, and carry a changelog fragment. The wider four-root-cause burn-down was cleared in earlier rounds and I re-spot-checked the highest-risk framework paths (Public.cfc, LocalDisk.put(), transactions.cfc) — all sound. Verdict: comment. No blocking finding; the only reason I'm not approving is that the two engines these last commits actually target (boxlang + postgres/cockroachdb) are unverified locally — the author flagged the same gap and asked to hold merge for the CI matrix.

Correctness

The 9a3cff91f fix keys the driver-option fallback on local.candidateSql == local.candidateSelect (vendor/wheels/channel/DatabaseAdapter.cfc:186). I confirmed this is a reliable "no dialect rewrite happened" signal against $applyRowBound (:377): every applied branch mutates the string (SELECT TOP n, FETCH FIRST, LIMIT n), and only the default/unknown-dialect and maxRows <= 0 branches return arguments.sqlText unchanged. The bounded pass only runs under if (arguments.maxRows > 0) (:162), so Int(arguments.maxRows) is always positive when the option is set — no maxrows: 0/negative footgun.

The CockroachDB path is handled correctly, which is the subtle part: $detectDatabaseType() (:342) matches CockroachDB's database_productname of "PostgreSQL" via FindNoCase("postgre", ...) and returns "postgresql", so the candidate SELECT gets LIMIT n appended and the driver option is not set — exactly the outcome the fix needs on both databases that share pgjdbc.

Tests

65a51047f's new it("runs the bounded-pass statements against the live database") closes the real gap — the pre-existing five $applyRowBound cases only string-compare and never send SQL to a database, so a driver-level rejection presented only as cleanup()'s swallowed 0. The spec correctly asserts against candidates.id[1] rather than the inserted row (vendor/wheels/tests/specs/channel/DatabaseAdapterSpec.cfc), since ORDER BY createdAt ASC may pick an older leftover, and cleans up its own test.livebound row by channel at the end. Its option handling was updated in 9a3cff91f to mirror cleanup()'s conditional exactly (:263), so the double no longer diverges from the code under test.

Docs

Changelog fragment changelog.d/3302-channel-cleanup-driver-row-bound.fixed.md is a proper .fixed.md fragment (no direct CHANGELOG.md [Unreleased] edit). Eight fragments total, all well-formed.

Commits

Both new commits conform to commitlint.config.js — valid types (fix, test), unrestricted scopes, subjects under 100 chars and not ALL-CAPS, and the bodies explain the why thoroughly (the pgjdbc setLargeMaxRows() provenance and the JobWorker.$claimNext prior art). DCO sign-off present.

Non-blocking note

Verification is lucee7 + sqlite only (4762 pass / 0 fail); the boxlang + postgres/cockroachdb legs these commits fix could not be run locally (Docker down) and go to CI. This is the author's own stated hold-for-merge condition, not a defect in the diff — mirroring it as the reason this is a comment rather than an approval. Worth confirming the matrix on this branch before merge.

`insertAll()` could never insert more than one row into an Oracle table
with an identity or sequence-backed primary key.

    ORA-00001: unique constraint violated on table C_O_R_E_BULKITEMS
    columns (ID) ... row with column values (ID:1) already exists
    ORA-00001: ... C_O_R_E_AUTHORS ... (ID:21) already exists

Two records, both ID:1. Three records, all ID:21. In a multitable
insert Oracle evaluates row defaults once per row of the DRIVING QUERY
and shares the result across every INTO clause — and the driving query
was `SELECT 1 FROM dual`, exactly one row. So the identity default was
evaluated once and handed to every record in the batch.

`INSERT ALL` was chosen for #2745, where the standard multi-row VALUES
tuple list tripped `ORA: returning clause is not allowed with INSERT
and Table Value Constructor` — cfquery sets RETURN_GENERATED_KEYS on
INSERTs and the driver turns that into a RETURNING clause. That fix was
right about the constraint it addressed and wrong about this one.

`INSERT INTO t (cols) SELECT ? ... FROM dual UNION ALL SELECT ? ...`
satisfies both: not a table value constructor, and N driving rows, so
the identity advances per record. It is also the shape `$upsertSQL`
directly below already uses for its MERGE source, down to aliasing only
the first branch.

The SQL-shape spec asserted `INSERT ALL` and one INTO per record —
the mechanism, not the requirement — which is why it stayed green
through this. It now pins one statement, no tuple list, and one
`FROM dual` source row per record. That last assertion is the one that
would have caught it: the old form had three INTO clauses and one
driving row.

Also adds TimestampRoundTripSpec, aimed at the remaining five Oracle
legs (RateLimiterDatabaseSpec x3, SchemaEnrichmentSpec x2) — all of
them consistent with a stored timestamp not reading back as a date. It
asserts the disjunction `$secondsSince()` actually relies on: a CFML
date, or epoch milliseconds. Writing it turned up that Lucee 7 +
sqlite-jdbc returns `1785873308685` from a cf_sql_timestamp write —
the epoch-millisecond case `$secondsSince` documents but nothing
pinned. Unlike the five symptom legs, its failure message prints the
value.

Verified: full core suite on lucee7 + sqlite, 4763 pass / 0 fail /
0 error across 355 bundles. The Oracle behaviour itself is verified by
the compat matrix, which is the only place an Oracle instance exists —
local Docker was unavailable all session.

Refs #3302

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

@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

TL;DR — Re-review at head 5d085c384. Since the last reviewed commit (9a3cff91f) one commit landed: the Oracle insertAll fix that swaps the INSERT ALL ... SELECT 1 FROM dual multitable form for INSERT INTO t (cols) SELECT ... FROM dual UNION ALL ..., plus TimestampRoundTripSpec. The change is correct and the SQL-shape spec was tightened to pin the requirement (one driving row per record) rather than the old mechanism. One minor Docs nit: this user-facing bug fix is the only #3302 root cause in the PR without a changelog.d/ fragment. Nothing blocking. Verdict: comment — this PR also explicitly asks that merge be held until the compat matrix confirms the Oracle and BoxLang legs, which local review cannot substitute for.

Correctness

No defects found in the new commit. The rewritten $bulkInsertSQL (vendor/wheels/databaseAdapters/Oracle/OracleModel.cfc:330-356) is structurally identical to the already-shipping $upsertSQL MERGE source directly below it (:381-397) — same SELECT ? AS col ... FROM dual UNION ALL shape and the same alias-only-the-first-branch rule:

if (local.r == arguments.batchStart) {
    ArrayAppend(local.sql, " AS " & $quoteIdentifier(arguments.columns[local.p]));
}

The columns / validProperties parallel-index assumption (arguments.columns[local.p] inside a loop over validProperties) is a pre-existing invariant shared with $upsertSQL, not introduced here.

One item to confirm on the matrix (I can't evidence it either way from here, so it is not a change request): whether cfquery's implicit RETURN_GENERATED_KEYS on INSERT ... SELECT is tolerated by the Oracle driver the way it is on the MERGE path. The commit body already flags Oracle as matrix-verified-only, and the parallel $upsertSQL shape works, so the risk is low — but it is the one thing local sqlite verification (4763 pass) does not exercise. This is exactly why the PR asks to hold merge for the matrix run.

Tests

Good. oracleBulkInsertSqlSpec.cfc was correctly re-pointed from asserting the mechanism (INSERT ALL, "one INTO per record") to asserting the requirement: one INSERT INTO, no table value constructor, and — the assertion that would have caught #3302 — one FROM dual source row per record (vendor/wheels/tests/specs/model/oracleBulkInsertSqlSpec.cfc:94-104). TimestampRoundTripSpec pins the $secondsSince() date-or-epoch-milliseconds disjunction directly and prints the offending value, which the five downstream symptom legs do not. Table c_o_r_e_bulkitems and its createdat column exist in vendor/wheels/tests/populate.cfm:398, matching the spec's INSERT.

Minor (non-blocking) test-hygiene note: TimestampRoundTripSpec cleans up with a trailing DELETE, but an expectation failure throws before it runs, leaving a TS-ROUNDTRIP row behind. The leading DELETE makes re-runs self-healing, so this is cosmetic — a transaction { ... transaction action="rollback" } wrapper would make it airtight if you touch the file again.

Docs

Every other #3302 root cause in this PR ships a changelog.d/ fragment (8 present: 3302-adobe-typed-default-param.fixed.md, 3302-localdisk-binary-write.fixed.md, 3302-public-component-helper-visibility.fixed.md, etc.). The newest commit's fix is equally user-facing — insertAll() could never insert more than one row into an Oracle table with an identity/sequence PK — but has no fragment. Suggest adding changelog.d/3302-oracle-bulk-insert-per-record.fixed.md with the bullet, consistent with the fragment system CLAUDE.md mandates over direct CHANGELOG.md edits.

Commits

fix(model): give Oracle's bulk insert one source row per record (#3302) — conforms to commitlint.config.js (valid fix type, model scope, subject well under 100 chars, DCO sign-off present). The body explains the why (Oracle evaluates row defaults once per driving-query row) precisely.

bpamiri added a commit that referenced this pull request Aug 5, 2026
…t-fail check, honest summary (#3366)

* ci: compat-matrix safe-slice hardening for #3302

Three workflow-plumbing fixes in compat-matrix.yml, shippable before the
continue-on-error flip (which stays gated on PR #3365 merging plus one
green dispatch on develop):

- run-tests: per-leg zero-test guard — an HTTP 200/417 leg reporting
  totalSpecs below 4000 (suite runs ~4,700) now emits ::error:: and sets
  the leg fail flag, so a compile-wiped leg fails loudly instead of
  rendering as a pass. SOFT_FAIL_DBS is respected.
- publish-results: fail_on: nothing on publish-unit-test-result-action,
  so oracle soft-fail debt stops pinning a red aggregate 'Wheels Test
  Results' check to innocent dispatch SHAs. Annotations, PR comments and
  artifacts are unchanged; leg gating stays in the tests job.
- test-matrix-summary: zero-test legs render as ':warning: N tests'
  instead of a checkmark, soft-fail DB failures render as :warning:, and
  the Oracle column is annotated as soft-fail with a footnote.

The rustcfml job is untouched (intentionally informational).

Refs #3302

Signed-off-by: Peter Amiri <petera@pai.com>

* ci: align per-engine summary with the zero-test guard

Review follow-up for the #3302 safe slice: the per-engine step summary
still rendered a compile-wiped leg (0 failures, 0 testcases) as a pass
while the run-tests guard in the same job failed it with ::error::.
Read totalSpecs alongside the failure count and render sub-floor legs
as ':warning: N tests (zero-test guard)', mirroring the matrix grid.

Refs #3302

Signed-off-by: Peter Amiri <petera@pai.com>

---------

Signed-off-by: Peter Amiri <petera@pai.com>
Co-authored-by: Peter Amiri <petera@pai.com>

@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

TL;DR — Re-review at head 815d118b6. This head is a merge commit (Merge branch 'develop' into fix/3302-matrix-burndown, parents 5d085c384 + 3b7199c6d); the only change since the last reviewed commit (5d085c384) is the clean develop catch-up merge — no conflict-resolution hunks (git show on the merge equals the diff vs its first parent), no new PR-authored code. The net PR diff against develop remains the four-root-cause compat burn-down (+ BoxLang parity), which I re-verified below. No blocking findings. Verdict: comment, consistent with the prior five passes.

Correctness

Re-read the four framework fixes at this head; each is sound as written:

  • vendor/wheels/storage/drivers/LocalDisk.cfc:42put() now decodes non-binary content with CharsetDecode(content, "utf-8") before FileWrite, and get() (:55) already returns FileReadBinary. The put/get pair is byte-symmetric, so the Adobe-2025 trailing-0x0A corruption is fixed without changing the round-trip contract on other engines.
  • vendor/wheels/model/transactions.cfc:57-84 — the new outer try/catch resets request.wheels.transactions[local.connectionArgs] = false when transaction action="begin" itself throws (bad isolation / dead connection) before the inner block is entered. Without it the open marker stayed true and every later invokeWithTransaction silently took the "already open" no-transaction path. Double-reset is idempotent (inner catch clears the same flag).
  • vendor/wheels/migrator/TableDefinition.cfc — dropping the string type token from the 24 default params is the documented fix for Adobe binding the type keyword as the parameter name. The parameter was already named default (a legal identifier here), so no other engine changes behavior; the two intentional outlier defaults (float default="", uniqueidentifier default="newid()") are preserved with in-line notes.
  • vendor/wheels/Public.cfc:23$scanAndPromoteIncludedGlobals() (the raw scan, not the memoizing wrapper) after the helpers.cfm include mirrors Global.cfc's own /app/global/functions.cfm handling; the comment correctly explains why the memoized path would replay a stale pre-include key list.

Cross-engine

  • vendor/wheels/Test.cfc:770 and vendor/wheels/databaseAdapters/Base.cfc:167-183 are the BoxLang-parity fixes. The StructKeyExists(server, "boxlang") guard on getBoxRuntime().executeStatement(...) keeps the BoxLang-only symbol from having to compile on Lucee/Adobe (runtime resolution), and collapsing the $isBoxLangEngine() fork in $parseInsertColumnList to a single regex form removes both the ReplaceList comma-drop bug and the BaseProbe test-double trap. vendor/wheels/wheelstest/system/Assertion.cfc:1296 short-circuits two unequal simple values before the object .equals() fallback, which on BoxLang resolves to the DateTime member fn and threw. All three are correct and land as two new invariants in CLAUDE.md.
  • View rename _groupRow.cfm -> _grouprow.cfm + ViewFileNamingGuardSpec addresses the LCase(local.rv) case-sensitivity path without touching the load-bearing LCase() call. Good scoping.

Tests

Every root cause ships a spec (TransactionMarkerResetSpec, TimestampRoundTripSpec, ViewFileNamingGuardSpec, DatabaseAdapterSpec, oracleBulkInsertSqlSpec, AdapterIdentityTemplateSpec, CockroachDBTransactionSpec). Coverage exercises the fixed paths, not just the happy case.

Commits

All PR commits conform to commitlint.config.js (fix(model), test(channel), fix(compat), chore, …). The head merge commit is a standard merge and is exempt from the header rules.

Notes (non-blocking)

  • The PR body asks to hold merge until the matrix confirms lucee6 (root cause 1) and boxlang (root cause 5) — those legs were unverified at authoring time. That's a merge-gating concern for a maintainer, not a code finding; please confirm the matrix run on this branch (or the author's posted results) before merging.

No Correctness / Cross-engine / Security finding fires, and no prior wheels-bot review is in an active CHANGES_REQUESTED state (all five are COMMENTED), so this stays a comment.

@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

TL;DR — Re-review at head 5417e51e0. This head is a merge commit (Merge branch 'develop' into fix/3302-matrix-burndown, parents 815d118b6 + 11952f330); the only on-diff change since the previously-reviewed 815d118b6 is vendor/wheels/tests/specs/model/queryBuilderSpec.cfc, pulled in from develop via the already-merged #3375. The underlying burn-down — four compat-matrix root causes plus a BoxLang parity fix, three of them live framework bugs — remains as vetted in the prior rounds: sound, well-commented, spec-backed, and each carrying a changelog fragment. Verdict: comment (no blocking findings; conservative default while lucee6/boxlang matrix confirmation is still pending per the PR body).

Correctness

  • The one new change (vendor/wheels/tests/specs/model/queryBuilderSpec.cfc:283-287, :296-300) replaces count() terminals with .get() + expect(result.recordcount).toBe(1):
    // Terminal must be non-aggregate: Postgres/CockroachDB reject COUNT(*) ... FOR UPDATE.
    var result = model("author").forUpdate().where("lastName", "Djurner").get();
    expect(result.recordcount).toBe(1);
    Correct — SELECT COUNT(*) ... FOR UPDATE is rejected by Postgres/CockroachDB, and .get() still exercises the forUpdate() chain-entry dispatch these specs are pinning. This landed independently as #3375 and is merged; nothing to change here.
  • Spot-checked the core framework fixes against the cited files and they hold up:
    • LocalDisk.put() (vendor/wheels/storage/drivers/LocalDisk.cfc:43) — IsBinary(...) ? content : CharsetDecode(content, "utf-8") writes bytes and dodges Adobe 2025's trailing-LF-on-simple-value behaviour without touching the binary path.
    • transactions.cfc:59-83 — the new outer try/catch resets request.wheels.transactions[local.connectionArgs] when transaction action="begin" throws before the inner block; double-reset is harmless as the comment notes.
    • channel/DatabaseAdapter.cfc:186-190 — the driver-level maxrows is now attached only when \$applyRowBound returned the statement unchanged (local.candidateSql == local.candidateSelect), which is exactly the no-dialect-rewrite case; correct, and it avoids pgjdbc's unimplemented setLargeMaxRows().
    • OracleModel.cfc:330-356INSERT ... SELECT ... UNION ALL FROM dual gives one driving row per record (fixing the shared-identity duplicate-key), aliases only the first branch, and mirrors \$upsertSQL's MERGE source.

Conventions

  • The typed-default -> untyped default change across TableDefinition.cfc, Base.cfc/Abstract.cfc, and references() correctly works around Adobe binding the string type token as the parameter name and swallowing default. Worth a note only: the params are now effectively any on every engine (previously string on the engines where the typed form worked). That is the intended fix and the values are string-rendered into DDL regardless, so no behavioural regression — just flagging the widened type for the record.
  • Changelog fragments (changelog.d/3302-*.fixed.md) follow the <slug>.<type>.md convention; no direct CHANGELOG.md [Unreleased] edit. Good.

Cross-engine

  • The BoxLang branches use runtime dispatch so the engine-specific names never compile on Lucee/Adobe: StructKeyExists(server, "boxlang") gating getBoxRuntime().executeStatement() in Test.cfc:770-772, and the early return false for two simple unequal values in Assertion.cfc:1296-1298 that keeps them off the .equals() -> DateTime-cast path. Both are correct.
  • \$detectDatabaseType() moved private -> public with the \$ prefix (channel/DatabaseAdapter.cfc:335), matching the mixin-visibility invariant so a spec can reproduce the chosen dialect.
  • _groupRow.cfm -> _grouprow.cfm rename plus the lowercase includePartial(partial="grouprow", …) call in contentSpec.cfc:107 resolves the \$generateIncludeTemplatePath LCase() path on case-sensitive filesystems; ViewFileNamingGuardSpec pins it. Confirmed no remaining code reference to the camelCase name — only doc/comment mentions survive.

Tests

  • New/updated specs accompany each fix (TransactionMarkerResetSpec, TimestampRoundTripSpec, ViewFileNamingGuardSpec, DatabaseAdapterSpec, oracleBulkInsertSqlSpec, CockroachDBTransactionSpec, AdapterIdentityTemplateSpec). Coverage matches the root causes.

Docs

  • CLAUDE.md gains the two new cross-engine invariants (typed default params; Adobe 2025 FileWrite trailing LF). Consistent with the change.

One standing caveat, unchanged from the PR body: verification is on sqlite, and the lucee6 / boxlang legs were noted as not yet matrix-confirmed. Nothing in the diff contradicts the expectation that they clear, but that is why this stays at comment rather than approve. Recommend holding merge until a matrix run on this branch confirms them.

@bpamiri
bpamiri merged commit 6bff054 into develop Aug 5, 2026
18 of 19 checks passed
@bpamiri
bpamiri deleted the fix/3302-matrix-burndown branch August 5, 2026 05:04
bpamiri added a commit that referenced this pull request Aug 5, 2026
Removes continue-on-error from the tests job now that the debt gate is
cleared: PR #3365 merged and dispatch run 30976993285 on develop came back
fully green (all 5 engine jobs, aggregate 6 oracle soft-fails only, which
stay non-blocking via SOFT_FAIL_DBS under #2663). The rustcfml job keeps
its continue-on-error — it is an informational lane by design.

From now on a red engine leg turns the weekly run banner red instead of
being silently swallowed.

Signed-off-by: Peter Amiri <petera@pai.com>
Co-authored-by: Peter Amiri <petera@pai.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.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