fix(model): override $supportsAdvisoryLocks=false on CockroachDB so lockingSpec skips

[bpamiri]

Summary

• CockroachDBModel now overrides $supportsAdvisoryLocks() to return false, so the four lockingSpec.withAdvisoryLock tests skip cleanly on CockroachDB (matching SQL Server / H2 / Oracle) instead of erroring with CockroachDB does not support advisory locks.
• Root cause: CockroachDBModel extends PostgreSQLModel, which reports $supportsAdvisoryLocks()=true (PG implements pg_advisory_lock natively). The CockroachDB adapter intentionally throws from $acquireAdvisoryLock/$releaseAdvisoryLock, but never overrode the capability flag — so the spec's beforeEach { skip(...) } guard added in test(model): guard lockingSpec advisory-lock tests on adapters without standalone support #2670 never fired and the four tests proceeded into the throwing primitives.
• test(model): guard lockingSpec advisory-lock tests on adapters without standalone support #2670's CHANGELOG entry already named CockroachDB in the "skips on" list, but the override itself was missed. This PR closes the gap with the one-method override.

Related Issue

Fixes #2743

Type of Change

• Bug fix
• New feature
• Enhancement to existing feature
• Documentation update
• Refactoring

Feature Completeness Checklist

• DCO sign-off — git commit -s
• Tests — no new spec needed; the existing capability-flag spec (is implemented on the current model adapter and returns a boolean) and the beforeEach { skip(...) } guard in lockingSpec already cover the path. The four CockroachDB error rows in compat-matrix runs (lucee6/cockroachdb, lucee7/cockroachdb, boxlang/cockroachdb) become four skip rows.
• Framework Docs — handled by bot-update-docs.yml follow-up
• AI Reference Docs — handled by bot-update-docs.yml follow-up
• CLAUDE.md — n/a (capability layer was already documented as the correct pattern)
• CHANGELOG.md — entry added under [Unreleased] → Fixed
• Test runner passes — Lucee 7 + SQLite via local runner: wheels.tests.specs.model reports 830 pass, 0 fail, 0 error, with all 11 lockingSpec specs passing on SQLite (capability flag still true there). The CockroachDB-specific path is mechanically identical to the existing SQL Server false override.

Test Plan

• Local bash tools/test-local.sh model (Lucee 7 + SQLite): 830/0/0, lockingSpec 11/11 pass — proves the change is non-disruptive on adapters that report true
• Compat matrix to confirm lucee6/cockroachdb, lucee7/cockroachdb, boxlang/cockroachdb now show 4 skips where they previously showed 4 errors (capability-flag flip; no other code paths touched)

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR adds a single $supportsAdvisoryLocks() override on CockroachDBModel that returns false, closing a gap left by PR #2670 where the capability flag was never set despite the CHANGELOG claiming CockroachDB in the "skips on" list. The root cause (CockroachDB inherits true from PostgreSQLModel but throws from $acquireAdvisoryLock) is correctly diagnosed, the fix is mechanically identical to the existing MSSQL/SQLite overrides, and no changes to tests or spec files are needed because the beforeEach skip guard in lockingSpec already does the right thing once the flag is correct. Verdict: approve.

Correctness

The inheritance chain is verified: Base.cfc.$supportsAdvisoryLocks() returns false by default; PostgreSQLModel.cfc overrides to true (line 223); CockroachDBModel.cfc extends PostgreSQLModel and now overrides back to false. H2 and Oracle extend Base.cfc directly and therefore never needed their own override. The lockingSpec beforeEach guard at line 20 checks !adapter.$supportsAdvisoryLocks() and calls skip() — once this flag is false for CockroachDB, the four withAdvisoryLock tests that previously errored will correctly report as skipped on lucee6/cockroachdb, lucee7/cockroachdb, and boxlang/cockroachdb.

Conventions

public boolean function $supportsAdvisoryLocks() — public access with $ prefix is exactly what cross-engine invariant #7 requires for mixin/adapter helpers. The one-sentence nit: the docblock's final sentence ("Use forUpdate() for row-level locking inside a transaction instead.") is already present verbatim on the adjacent $acquireAdvisoryLock and $releaseAdvisoryLock docblocks in the same file; it's redundant here. Not a blocker, but trimming it would keep the comment focused on the WHY (the inheritance/capability-flag subtlety) rather than restating guidance already visible in the sibling methods.

Cross-engine

No engine-specific constructs. return false; is clean CFML that runs identically on Lucee 5/6/7, Adobe CF 2018–2025, and BoxLang.

Tests

No new spec file is needed and the PR's reasoning is correct. The existing $supportsAdvisoryLocks adapter capability spec (lockingSpec.cfc lines 7–13) verifies the method returns a boolean on whatever adapter is running. The behavioral coverage (skip-instead-of-error on CockroachDB) is mechanically identical to the already-tested MSSQL path; the local 830/0/0 run on SQLite confirms the change is non-disruptive on adapters that report true. The compat-matrix CockroachDB legs (unchecked in the test plan) are the only remaining validation, which is expected — CockroachDB is matrix-only.

Commits

Single commit fix(model): override $supportsAdvisoryLocks=false on CockroachDB so lockingSpec skips — type fix, scope model, subject under 100 chars, not ALL-CAPS, body lines within 100 chars. DCO Signed-off-by: Peter Amiri <peter@alurium.com> is present. Conforms to commitlint.config.js.

Docs

CHANGELOG [Unreleased] → Fixed entry added (line 40). The entry is thorough — arguably longer than needed for a one-line change, but the verbosity matches the surrounding entries' style and correctly cross-references #2670 and #2743.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-grounded. All cited line numbers are verified against the current codebase: PostgreSQLModel.cfc line 223 ✓, lockingSpec beforeEach guard at line 20 ✓, lockingSpec adapter capability describe block at lines 7–13 ✓. DCO sign-off (Signed-off-by: Peter Amiri <peter@alurium.com>) is present in the commit. The adapter landscape claim (MySQL, PostgreSQL, MSSQL, SQLite each carry their own override; H2 and Oracle inherit Base.cfc's default-false) is confirmed by grep. No sycophancy, no false positives, no missed issues. Aligned with approve.

Sycophancy

None detected. The approval cites concrete evidence: inheritance chain, line numbers, spec coverage, and commit conformance.

False positives

None detected. Every factual claim A made was verified against the actual diff and codebase.

Missed issues

None detected. The change is a one-method override. Cross-engine invariant #7 (public access with $ prefix) is satisfied. The capability-flag pattern is the correct abstraction layer for this fix. The docblock nit A raised (final sentence duplicates guidance in adjacent $acquireAdvisoryLock and $releaseAdvisoryLock docblocks) is accurate and properly scoped as non-blocking.

Verdict alignment

A's approve verdict is consistent with the findings: targeted one-line fix, correct conventions, existing specs cover the behavioral path, and no new test surface is introduced.

Convergence

Aligned. A's analysis is correct and complete. Joint recommendation: approve and merge once the compat-matrix CockroachDB legs confirm the expected 4-skip / 0-error result (the only outstanding checklist item, and expected to be matrix-only validation as noted in the PR).

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This is a follow-up pass on #2746 after the fix commit (b9e912f72) was already reviewed. The only change since that review is a single follow-up commit (a6e0f7c70, docs(model): trim redundant forUpdate hint…) that excised the sentence Reviewer A flagged as redundant from the $supportsAdvisoryLocks docblock. The resulting file is clean — the override is correct, the docblock is now tightly scoped to the why (the PostgreSQL inheritance / capability-flag gap), and no test or CHANGELOG changes were needed for this documentation-only follow-up. Verdict: approve.

Correctness

The final state of vendor/wheels/databaseAdapters/CockroachDB/CockroachDBModel.cfc is verified:

public boolean function $supportsAdvisoryLocks() {
    return false;
}

• CockroachDBModel extends PostgreSQLModel — PostgreSQLModel.$supportsAdvisoryLocks() returns true; this override returns false. Inheritance chain is correct.
• $acquireAdvisoryLock and $releaseAdvisoryLock still throw Wheels.AdvisoryLockNotSupported when reached, but lockingSpec's beforeEach { skip(...) } guard now fires first and they are unreachable during test runs on CockroachDB legs.
• The docblock trim removed the sentence "Use forUpdate() for row-level locking instead." It was already present on the adjacent $acquireAdvisoryLock and $releaseAdvisoryLock docblocks; its removal tightens the comment without losing any information visible to a reader of the file.

Commits

Both commits conform to commitlint.config.js:

Commit	Type	Scope	Subject len	DCO
b9e912f72	fix	model	72 chars	Signed-off-by: Peter Amiri <peter@alurium.com>
a6e0f7c70	docs	model	74 chars	Signed-off-by: Peter Amiri <peter@alurium.com>

Both subjects are under 100 chars, not ALL-CAPS, and the body references the originating issue/PR. Second commit body explicitly acknowledges the Reviewer A nit from the prior review cycle — good signal that the follow-up is intentional and scoped.

Cross-engine

return false; is valid CFML across Lucee 5/6/7, Adobe CF 2018–2025, and BoxLang. No closure captures, no struct member-function collisions, no reserved scope names, no Left(str, 0) calls. Nothing to flag.

Tests

No new test file is needed and no existing spec was modified. The capability-flag spec (lockingSpec.cfc lines 7–13: is implemented on the current model adapter and returns a boolean) and the beforeEach { skip(...) } guard already exercise the path once the flag returns the correct value. The compat-matrix CockroachDB legs are the only remaining validation, consistent with the checked test plan.

Docs

CHANGELOG [Unreleased] → Fixed entry (line 40) was added in the first commit and is unchanged by the follow-up. Entry is accurate, cites #2743, and cross-references #2670. No .ai/wheels/ reference doc needed for a capability-flag override that has no user-facing API surface change.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-grounded. The follow-up commit (a6e0f7c70) is documentation-only — it removed the "Use forUpdate() for row-level locking instead" sentence that was already present on the adjacent $acquireAdvisoryLock and $releaseAdvisoryLock docblocks. The net PR diff confirms: +13/-0 (one CHANGELOG line, twelve CFC lines comprising the clean docblock and the one-liner override). Every factual claim A made is verified against the actual diff. Aligned with approve.

Sycophancy

None detected. The approval cites the docblock-trim rationale, a commit-conformance table with SHA/type/scope/subject-length/DCO, a cross-engine analysis, and specific spec references (lockingSpec lines 7–13).

False positives

None detected. The diff confirms public boolean function $supportsAdvisoryLocks() { return false; }, the CHANGELOG entry under [Unreleased] → Fixed, and the absence of the "forUpdate" sentence from the new docblock (it remains on the adjacent method docblocks where it belongs, as A noted).

Missed issues

None detected. Cross-engine invariant #7 (public access, $ prefix) is satisfied. The change touches no closures, struct member functions, reserved scopes, or migration paths. No .ai/wheels/ reference doc update is needed — the capability-flag abstraction has no user-facing API surface change.

Verdict alignment

A's approve verdict is consistent with the findings: a one-method override plus a documentation-only trim, with existing specs covering the behavioral path.

Convergence

Aligned. A's analysis is correct and complete. Joint recommendation: approve and merge once the compat-matrix CockroachDB legs confirm the expected 4-skip / 0-error result (the only remaining checklist item, already flagged in the PR as matrix-only validation).