fix(model): skip CockroachDB advisory-lock specs via capability override

[wheels-bot]

Summary

databaseAdapters.CockroachDB.CockroachDBModel now overrides $supportsAdvisoryLocks() to return false. The CockroachDB adapter extends PostgreSQLModel, which reports true because Postgres implements pg_advisory_lock natively. CockroachDB does not — its existing $acquireAdvisoryLock / $releaseAdvisoryLock throw Wheels.AdvisoryLockNotSupported. Without the capability override, the four standalone advisory-lock specs in vendor/wheels/tests/specs/model/lockingSpec.cfc skipped past their beforeEach skip-guard and surfaced as errors on every Cockroach-backed compat-matrix run (Lucee 6, Lucee 7, BoxLang). This matches the H2 / SQL Server treatment from #2665.

Related Issue

Fixes #2743

Type of Change

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

Feature Completeness Checklist

• DCO sign-off -- Signed-off-by: trailer present on the single commit
• Tests -- New unit spec $supportsAdvisoryLocks › returns false (CockroachDB has no pg_advisory_lock equivalent) in vendor/wheels/tests/specs/database/CockroachDBUnitSpec.cfc. Confirmed failing (Expected [true] to be false) before the implementation landed; passing after.
• Framework Docs -- leave to bot-update-docs.yml
• AI Reference Docs -- leave to bot-update-docs.yml
• CLAUDE.md -- leave to bot-update-docs.yml
• CHANGELOG.md -- entry under [Unreleased] › ### Fixed
• Test runner passes -- wheels.tests.specs.database 32 pass / 0 fail / 0 error; wheels.tests.specs.model 830 pass / 0 fail / 0 error against the local Lucee 7 + SQLite runner

Test Plan

1. Direct unit assertion (this PR):

   curl 'http://localhost:60007/wheels/core/tests?db=sqlite&format=json&directory=wheels.tests.specs.database'
   

   New $supportsAdvisoryLocks describe block returns false.
2. Real failure mode (CI compat-matrix):

   tools/test-matrix.sh lucee7 cockroachdb
   

   The four lockingSpec advisory-lock specs should now report as skipped, not errored.

Screenshots / Output

n/a — test-runner output only.

[wheels-bot]

Wheels Bot — No doc updates

Reviewed this PR's diff and found no docs that need updating (purely internal bug fix — the $supportsAdvisoryLocks() capability flag is a $-prefixed framework-internal method; no user-facing API, CLI command, or observable behavior changed, and the CHANGELOG entry already shipped in the implementation commit).

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This is a tight, well-reasoned one-method fix: CockroachDBModel was inheriting $supportsAdvisoryLocks() = true from PostgreSQLModel even though CockroachDB has no pg_advisory_lock equivalent and the adapter already throws Wheels.AdvisoryLockNotSupported in both $acquireAdvisoryLock / $releaseAdvisoryLock. The override closes that gap and aligns CockroachDB with the H2 / SQL Server pattern from #2665. Verdict: approve (submitted as comment — bot cannot self-approve), with two minor nits.

---

Correctness

The logic is sound. lockingSpec.cfc line 20 reads:

if (!g.model("author").$classData().adapter.$supportsAdvisoryLocks()) {
    skip("Adapter does not support standalone advisory locks on this database");
}

With the old inheritance chain (CockroachDBModel → PostgreSQLModel → true) the beforeEach guard was never entered, leaving the four advisory-lock specs to execute against a database that throws Wheels.AdvisoryLockNotSupported — explaining the CI errors on Lucee 6, Lucee 7, and BoxLang Cockroach-backed runs. The new override correctly returns false so the skip fires, matching the SQLiteModel and MicrosoftSQLServerModel pattern. No application-facing behavior is changed: callers that already consult $supportsAdvisoryLocks() before acquiring a lock simply get the right answer for CockroachDB now.

Tests

Unit spec added at vendor/wheels/tests/specs/database/CockroachDBUnitSpec.cfc (lines 54–59) using BDD syntax extending wheels.WheelsTest. Happy path covered (returns false); error paths are N/A for a constant-returning capability check. No issues.

Docs

Nit — CHANGELOG entry missing trailing period.

The added line under [Unreleased] › Fixed ends with:

… capability-aware callers that already consult `$supportsAdvisoryLocks()` simply get the correct answer for CockroachDB now

Every other entry in that block ends with a .. Cosmetic only, but worth fixing before merge.

Nit — doc comment leads with test-suite framing.

vendor/wheels/databaseAdapters/CockroachDB/CockroachDBModel.cfc lines 42–50:

/**
 * CockroachDB lacks a pg_advisory_lock equivalent, so the parent PostgreSQL
 * adapter's `true` would be wrong here. Reporting `false` lets the test
 * suite skip the standalone advisory-lock specs (matching the H2 / SQL
 * Server treatment from #2665) and lets capability-aware callers route
 * around the missing primitive.
 */

Accurate, but leading with "lets the test suite skip …" frames this as a test workaround rather than a correctness fix. Suggested reorder (not a blocker):

/**
 * CockroachDB lacks a pg_advisory_lock equivalent — its $acquireAdvisoryLock
 * and $releaseAdvisoryLock both throw Wheels.AdvisoryLockNotSupported.
 * Overriding to false lets capability-aware callers route around the missing
 * primitive and lets the locking specs skip cleanly (H2 / SQL Server pattern
 * from #2665).
 */

Commits

Single commit fix(model): skip CockroachDB advisory-lock specs via capability override — valid type, subject ≤ 100 chars, not ALL-CAPS, DCO Signed-off-by: trailer present. No issues.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is substantively correct: the capability-override fix is well-reasoned, the test is appropriate, the commits are clean, and the approve verdict is right. One false positive on the CHANGELOG nit — A's trailing-period claim doesn't hold up against the diff. No sycophancy, no missed issues.

Sycophancy

None detected. A cited the specific skip-guard code path, traced the inheritance chain explicitly, and compared the fix to the H2 / SQL Server precedent rather than rubber-stamping the description.

False positives

• CHANGELOG trailing period (A's first nit) — A wrote: "Every other entry in that block ends with a .". Looking at the three context lines immediately surrounding the new entry in the diff, all three end without a period: ...is unchanged (#2703), ...at the call site, ...the methods table (#2736). The period convention A cited does not exist in this block. This nit is based on a fabricated style rule and should be dropped.

• Doc comment reordering (A's second nit) — A's suggestion to lead with the throw behaviour rather than the test-suite consequence is valid. It's a minor but legitimate framing note, not a false positive.

Missed issues

None detected. Cross-engine risk is zero (the override returns a literal false — no closures, no struct member functions, no reserved scopes). The public access modifier with $ prefix follows cross-engine invariant #7 from CLAUDE.md. Commit subject and type are within spec.

Verdict alignment

A's approve verdict is consistent with the findings: one genuine cosmetic nit (doc comment reorder) and one fabricated nit (trailing period). Neither warrants request-changes.

Convergence

Aligned with A's approve verdict. The false positive on the CHANGELOG period is cosmetic and does not affect correctness or the recommendation. No changes needed — this PR is review-clean on the current SHA.

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: After #2746 landed the $supportsAdvisoryLocks()=false override on develop and Peter merged develop back into this branch (commit 5e4ddd269), PR #2747's net contribution is exactly one thing: the unit test asserting the override. That regression guard is correct, well-placed, and follows BDD conventions throughout. Verdict: approve (submitted as comment — bot cannot self-approve), no findings requiring changes.

---

Correctness

The sole diff is lines 54–59 in vendor/wheels/tests/specs/database/CockroachDBUnitSpec.cfc:

describe("$supportsAdvisoryLocks", () => {
    it("returns false (CockroachDB has no pg_advisory_lock equivalent)", () => {
        expect(adapter.$supportsAdvisoryLocks()).toBeFalse();
    });
});

The adapter binding is established in beforeAll() (line 4) as a plain CockroachDBModel instance. $supportsAdvisoryLocks() returns the constant false with no DB calls, so the test is deterministic regardless of environment. If the override is ever accidentally dropped, CockroachDBModel reverts to inheriting PostgreSQLModel's true and this test fails — exactly the regression guard the merge commit describes.

Tests

• BDD syntax with describe / it lambdas — correct.
• Extends wheels.WheelsTest — correct (not the legacy wheels.Test / RocketUnit form).
• Placed inside the existing CockroachDB Adapter Unit Tests describe block alongside sibling capability tests ($generatedKey, $identitySelect) — logical grouping.
• Happy path is the only path; a constant-returning capability flag has no error path to cover.

One cosmetic nit (not a blocker): the it() label embeds a parenthetical rationale — "returns false (CockroachDB has no pg_advisory_lock equivalent)". The first clause is the behavior; the parenthetical is a comment. Both are accurate, but some teams prefer the test name to be a pure behavior phrase with the rationale in a code comment. Either style is acceptable here.

Commits

• 0f9f08a55: fix(model): skip CockroachDB advisory-lock specs via capability override — valid type, subject <= 100 chars, not ALL-CAPS, DCO Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> matches commit author. Clean.
• 5e4ddd269: merge commit authored by Peter Amiri with Signed-off-by: Peter Amiri <peter@alurium.com>. DCO present.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is correct. The diff is a single 7-line test block; every claim A made is verified by the actual diff. Approve verdict is right.

Sycophancy

None detected. A traced the merge context, identified the net contribution accurately, and explained the regression-guard logic rather than rubber-stamping the description.

False positives

None detected. Each factual claim checks out:

• Net diff is exactly the $supportsAdvisoryLocks describe block — confirmed by gh pr diff.
• adapter binding, extends="wheels.WheelsTest", BDD syntax — all correct per the file.
• DCO: Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> matches the commit author identity — valid.
• Cosmetic nit on the it() label is a genuine stylistic observation, not a fabricated convention.

Missed issues

None detected. Cross-engine risk is zero: the test is a single method call returning a constant with no closures, struct literals, or reserved-scope variables.

Verdict alignment

Approve is consistent with findings: one cosmetic, non-blocking label nit.

Convergence

Aligned. A's approve verdict is correct for a test-only diff that regresses a constant-returning capability flag. No changes needed.

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: The head commit (2381227768) is a pure documentation change: Peter reworked the $supportsAdvisoryLocks() docblock in CockroachDBModel.cfc to lead with the underlying throw behavior rather than the test-suite skip framing that Reviewer A flagged in round 1. The net PR diff (after develop was merged back in) is exactly two things — the updated docblock and the regression-guard unit test. Both are correct. Verdict: approve (submitted as comment — bot cannot self-approve a bot-authored PR).

---

Correctness

The docblock change is accurate. The new wording leads with the root cause:

/**
 * CockroachDB lacks a pg_advisory_lock equivalent — its
 * $acquireAdvisoryLock and $releaseAdvisoryLock both throw
 * Wheels.AdvisoryLockNotSupported. Overriding the parent PostgreSQL
 * adapter's `true` to `false` lets capability-aware callers
 * (`withAdvisoryLock`, the lockingSpec `beforeEach` guard) route around
 * the missing primitive instead of hitting the throw, matching the
 * H2 / SQL Server pattern from #2665.
 */

This is factually correct: CockroachDBModel.$acquireAdvisoryLock and $releaseAdvisoryLock do throw Wheels.AdvisoryLockNotSupported by design. No functional change. The round-1 nit is fully addressed.

Tests

vendor/wheels/tests/specs/database/CockroachDBUnitSpec.cfc lines 54-59:

describe("$supportsAdvisoryLocks", () => {
    it("returns false (CockroachDB has no pg_advisory_lock equivalent)", () => {
        expect(adapter.$supportsAdvisoryLocks()).toBeFalse();
    });
});

• BDD describe/it syntax, extends wheels.WheelsTest — correct form.
• adapter is bound in beforeAll() as a plain CockroachDBModel instance; $supportsAdvisoryLocks() is a constant-returning method with no DB calls, so the test is deterministic across all engines. No error paths to cover.
• Placed inside the existing CockroachDB Adapter Unit Tests describe block alongside $generatedKey and $identitySelect — logical grouping.

Docs

The round-1 nit about a missing trailing period on the CHANGELOG entry was a false positive: every other entry in the same ### Fixed block also ends with (#NNNN) or an unperioded clause (e.g. the Cors entry ends with is unchanged (#2703), the paginationNav entry ends with at the call site). No outstanding doc issues.

Commits

• 238122776 docs(model): rework CockroachDB $supportsAdvisoryLocks docblock per review — docs type (valid), subject <= 100 chars, not ALL-CAPS, Signed-off-by: Peter Amiri <peter@alurium.com> present. Commit body explains the motivation and cites Reviewer A's feedback. Clean.
• e93275029 merge commit by Peter with Signed-off-by present.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review checks out. The diff is two files (docblock reword + 7-line test block), every factual claim A made is verified against the actual diff, and the approve verdict is right.

Sycophancy

None detected. A traced the head commit's purpose (pure doc rework), verified the docblock wording against the throw behavior of $acquireAdvisoryLock/$releaseAdvisoryLock, and confirmed the earlier false-positive CHANGELOG nit rather than carrying it forward.

False positives

None detected. Spot-checked every claim:

• Net PR diff is exactly docblock + test — confirmed by gh pr diff.
• adapter bound in beforeAll() as CockroachDBModel — confirmed at line 4 of CockroachDBUnitSpec.cfc.
• $supportsAdvisoryLocks() returns a constant false with no DB calls — determinism claim correct.
• DCO trailers on both commits present and matching author identities — correct.
• CHANGELOG trailing-period nit correctly retired as a false positive.

Missed issues

None detected. Cross-engine risk is zero: the only runtime change is a constant-returning method override; the test uses the pre-established () => closure pattern already running on all engines in this file. No .ai/ doc update needed for a $-prefixed internal method.

Verdict alignment

Approve is consistent with the findings: one cosmetic doc-framing nit fully addressed, nothing blocking.

Convergence

Aligned. A's approve verdict is correct for a diff of this scope. No changes needed.