fix(cli): wheels validate must strip CFML comments before checking extends

[wheels-bot]

Summary

wheels validate previously accepted models and controllers whose only extends="Model" / extends="Controller" declaration was inside a CFML comment. validateModel() / validateController() in cli/lucli/services/Analysis.cfc performed a raw findNoCase('extends="Model"', content) over the whole file content, so a file like

// component extends="Model" {
component {
    function config() {}
}

satisfied the substring match and the validator reported success even though the active component { … } declaration has no inheritance. This PR strips line comments (// …), block comments (/* … */), and tag-style CFML comments (<!--- … --->) from the file content before running the extends= check, and adds specs to pin the behavior for both the bug case and the legitimate "commented snippet next to a real declaration" case.

The fix lives in a small private $stripCfmlComments() helper inside Analysis.cfc (mirrors the existing stripCfmlComments helper in Module.cfc and $stripCfmlBlockComments in Doctor.cfc). The same fix applies to validateController() since it had the identical bug shape.

Related Issue

Closes #2491

Type of Change

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

Feature Completeness Checklist

• DCO sign-off — commit carries Signed-off-by: trailer matching the configured git author identity
• Tests — four new specs in cli/lucli/tests/specs/services/AnalysisSpec.cfc covering: (a) commented line // component extends="Model", (b) commented block /* … extends="Model" … */, (c) commented copy + real extends="Model" (must still pass), and (d) commented extends="Controller".
• Framework Docs — leave unchecked; bot-update-docs.yml handles MDX follow-up.
• AI Reference Docs — leave unchecked; bot-update-docs.yml handles .ai/wheels/ follow-up.
• CLAUDE.md — leave unchecked; bot-update-docs.yml handles convention-doc follow-up.
• CHANGELOG.md — entry added under [Unreleased] → Fixed.
• Test runner passes — see Test Plan note below.

Test Plan

The new specs are deterministic and exercise pure string-handling code (no DB, no engine-specific behavior). Reasoning that confirms the failing → passing transition:

• Before the fix: findNoCase('extends="Model"', content) returns a positive position for the commented // component extends="Model" { line, so the inheritance check is satisfied and no issue is appended. The new "flags model where extends is only present in a line comment" spec asserts does not extend Model is present in the validator's issue messages — that assertion must fail.
• After the fix: $stripCfmlComments() removes the // … line, leaving only component { … }. findNoCase now returns 0 for both extends="Model" and extends='Model', so the validator appends the "does not extend Model" issue — assertion passes.
• The "commented copy alongside a real extends="Model"" spec confirms the strip doesn't go too far: the real declaration is preserved and the validator stays silent.

⚠️ I was unable to run bash tools/test-cli-local.sh locally — the runtime sandbox rejected the invocation. The TDD-gate CI check (bot-tdd-gate.yml) will confirm the spec + implementation diff shape, and the CLI test job will execute the new specs. If anything regresses, that will surface on the PR before review.

• cli/lucli/services/Analysis.cfc modified (implementation)
• cli/lucli/tests/specs/services/AnalysisSpec.cfc modified (failing → passing specs)
• No changes to commitlint.config.js, package.json, .github/workflows/pr.yml, or any unrelated branch

[wheels-bot]

Wheels Bot — Docs updated

Added a doc commit to this PR:

• web/sites/guides/src/content/docs/v4-0-0-snapshot/command-line-tools/wheels-commands/code-quality.mdx — Updated the Models and Controllers bullets under wheels validate to note that the extends= check is performed against the active (comment-stripped) component declaration; a extends="Model" present only inside a comment does not satisfy the inheritance check.

[wheels-bot]

test body for auth check

[wheels-bot]

Wheels Bot -- Reviewer A

This PR correctly fixes the false-pass in wheels validate reported in #2491. The implementation applies $stripCfmlComments() to file content before the extends= substring check in both validateModel() and validateController(), four specs cover the main scenarios, both commits carry DCO trailers and conform to commitlint, the guide doc is updated, and the CHANGELOG entry is accurate. I am leaving a comment (not requesting changes) because the implementation is correct and the issues below are test-quality nits rather than shipping bugs.

Correctness: Vacuous-pass in the still-passes spec (cli/lucli/tests/specs/services/AnalysisSpec.cfc lines 174-179)

The positive test writes CommentedAndReal.cfc, calls validate(), then asserts inside a conditional loop body. When the fix is working correctly, validate() returns zero issues for this file so the loop body is never entered and expect() is never reached -- the test passes vacuously. A regression that silently drops the file from the scan would also pass, providing no protection. The pattern used by the negative tests (accumulate into issueMessages, assert after the loop) does not have this flaw and should be applied symmetrically. Suggested replacement: declare var foundBadIssue = false before the loop, set it to true if the issue file and message both match, then call expect(foundBadIssue).toBeFalse() after the loop.

Tests: Tag-style CFML comment path not exercised

$stripCfmlComments() applies three substitutions in order: tag-style comment, block comment, line comment. The new specs cover the line and block paths but no spec places extends=Model inside a CFML tag-style comment (the four-dash form). A typo in that first regex would be invisible to the suite. A fifth spec analogous to the block-comment one would complete coverage of all three stripping paths.

Conventions: $stripCfmlComments is now the third copy of the same function

Module.cfc at line 5707 has stripCfmlComments (identical three-regex sequence). Doctor.cfc at line 569 has $stripCfmlBlockComments (first two regexes only, no line-comment step). Analysis.cfc at line 386 adds a byte-for-byte copy of the Module.cfc version. The PR body acknowledges this. Pre-existing accumulation, out of scope for a targeted fix -- flagging for a consolidation follow-up in Helpers.cfc.

Conventions: Indentation drift in the first new it() block

AnalysisSpec.cfc line 115 has the it() opener at four-tab indent, one level deeper than the three it() blocks that follow at lines 140, 163, 184. Cosmetic; easy one-line fix.

Cross-engine: No issues. reReplace() with character classes and the all flag is supported identically on Lucee, Adobe CF, and BoxLang. No closures, scope references, struct-member function collisions, or Left(str, 0) patterns.

Commits: Two commits, both conforming to commitlint. fix(cli): wheels validate must strip CFML comments before checking extends (a70c02f) -- type fix, scope cli, subject 72 chars, DCO trailer present. docs(web/guides): clarify wheels validate checks active uncommented extends declaration (5f42850) -- type docs, scope web/guides, full subject 87 chars (within the 100-char cap), DCO trailer present.

Security: No concerns. The fix operates on local filesystem content read by the CLI; it does not touch any HTTP surface, user-supplied runtime input, or SQL.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is substantively accurate and I am converging with changes needed. The vacuous-pass finding is the most important item: A correctly identified that expect() at line 177 inside the inner if in the still passes spec is unreachable when the fix is working correctly (no issues are emitted for CommentedAndReal.cfc, so the findNoCase guard never triggers and the assertion is never called). A's proposed foundBadIssue pattern — declared before the loop, set inside it, asserted toBeFalse() after — is a genuine improvement because the assertion becomes unconditional. The tag-style comment gap is real and warrants a spec, though not a blocker.

Sycophancy

• A described the missing tag-style comment spec as a "nit." The tag-style regex (<!---[\s\S]*?--->) is the first of three substitutions in $stripCfmlComments() and is entirely uncovered by the suite. A typo there would be invisible. The characterization is slightly soft; this is a real coverage gap even if it isn't a shipping bug.

False positives

None detected. Every specific claim A made maps accurately to the diff and the actual file.

Missed issues

None detected. The cross-engine analysis is correct (reReplace with the all flag behaves identically on Lucee, Adobe CF, and BoxLang). Commit trailers and commitlint conformance are accurately verified. The private access modifier on $stripCfmlComments is appropriate here (service CFC, not a mixin — the public-only-in-mixins rule in CLAUDE.md does not apply).

Verdict alignment

A chose COMMENTED (not requesting changes) for findings that are genuine test-quality improvements with no shipping bug. The verdict is consistent with the findings.

Convergence

Aligned; changes needed. Two concrete items for the address-review pass:

1. Vacuous-pass fix in CommentedAndReal.cfc spec (AnalysisSpec.cfc ~lines 174-179): replace the current loop-body assertion with A's suggested pattern — declare var foundBadIssue = false before the loop, set it to true inside the loop when both file and message match, and call expect(foundBadIssue).toBeFalse() unconditionally after the loop.
2. Add a tag-style comment spec: a fifth it() analogous to the block-comment spec, writing a file whose only extends="Model" is inside a <!--- ... ---> comment and asserting that the validator flags it. This covers the first regex in $stripCfmlComments().

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

Note: review 4268512214 (the trigger ID) contains only a test/auth-probe body ("test body for auth check") with no line comments. The substantive Reviewer A content is in review 4268513773, submitted 30 seconds later by the same bot on the same commit. This critique is of 4268513773.

A correctly identified the two real test-quality gaps (vacuous-pass spec, missing tag-style coverage) and made accurate technical claims throughout. No false positives. The implementation analysis and cross-engine / security sections are sound. My disagreement is narrow: the characterization of the vacuous-pass finding as a "nit" undersells it, and the COMMENTED verdict leaves both gaps unaddressed. I'm aligning on findings and recommending the changes be applied.

Sycophancy

• Mild: calling the vacuous-pass a "nit" understates it. The "still passes" spec currently asserts nothing when the code is correct — the expect() inside the for loop is only reached if validate() returns an issue for that file, which it correctly does not. A's own description of the problem is accurate ("the test passes vacuously") but the summary groups it alongside the cosmetic indentation finding, obscuring that one of these provides false confidence in test coverage and the other is one-tab drift. The suggested fix A provides (boolean flag + assert-after-loop) is the right pattern and should be applied.
• No reflexive LGTM or approval without evidence. No security/migration/deploy/DI subsystems touched.

False positives

None detected. Verified against the diff:

• Vacuous-pass claim: the "still passes" spec writes CommentedAndReal.cfc, calls validate(), then asserts inside a conditional loop that only executes if an issue for that file is returned. When the fix is working, the loop body is a no-op. A's description is accurate.
• Tag-style comment path: $stripCfmlComments() applies three substitutions in order (<!---…--->, /*…*/, //…) and the diff confirms only the block and line-comment paths have corresponding specs. A's claim is accurate.
• Third-copy finding: pre-existing accumulation, A flagged it informational / out of scope. Analysis.cfc is not a mixin CFC so private access is fine there (the CLAUDE.md restriction on private mixin functions applies to vendor/wheels/model/*.cfc and view-helper CFCs, not CLI services). Correct characterization.
• Indentation drift: the first it() opener sits at four-tab indent vs three-tab for the three that follow. Visible in the diff. Accurate.
• Cross-engine: reReplace() with character-class patterns and the all flag behaves identically on Lucee, Adobe CF, and BoxLang. No closures, no reserved-scope names, no Left(str, 0). A's clearance is correct.
• Commit / DCO analysis: both commits have correct types, scopes, subjects within the 100-char cap, and DCO trailers. Accurate.

Missed issues

None of significance. One nuance on A's risk framing: A's phrasing implies the positive spec provides zero protection, but a regression that silently drops the file from the scan entirely would also cause the two negative specs to fail, since they rely on the same scan path. The vacuous-pass gap is specifically that the positive spec does not independently assert "zero issues found for this file." That is still worth fixing; it is not the alarming standalone gap A's wording implies.

Verdict alignment

A's COMMENTED state is internally consistent — A listed findings but chose not to block the merge. However, both findings are concrete and actionable: A even supplied the exact fix pattern for the vacuous-pass. A COMMENTED verdict that yields no action is inconsistent with providing a specific fix suggestion. Request-changes would be the appropriate state.

Convergence

Aligned on all findings. No false-positive disputes, no missed issues. Upgrading the verdict to request-changes and recommending the two test improvements be applied:

1. Vacuous-pass spec (AnalysisSpec.cfc): replace the conditional expect() inside the for loop in the "still passes" test with an explicit boolean flag set before the loop and asserted false after it — the pattern A described and the same pattern used by the two negative specs.
2. Tag-style comment spec (AnalysisSpec.cfc): add a fifth spec writing a model file that places extends="Model" only inside a <!--- … ---> CFML tag comment, analogous to the existing block-comment spec.

The implementation in Analysis.cfc is correct and does not need changes.

[wheels-bot]

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

• Vacuous-pass fix (cli/lucli/tests/specs/services/AnalysisSpec.cfc — the still passes when extends is present alongside a commented-out copy spec): replaced the conditional expect() inside the for loop with the boolean flag pattern A suggested. var foundBadIssue = false is declared before the loop, set to true only when both the file matches CommentedAndReal.cfc and the message contains does not extend Model, and expect(foundBadIssue).toBeFalse() runs unconditionally after the loop. When the fix is working correctly the assertion now actively confirms zero matching issues rather than executing zero expect() calls.
• Tag-style comment spec (cli/lucli/tests/specs/services/AnalysisSpec.cfc — new it() block after the block-comment spec): writes CommentedExtendsTag.cfc with extends="Model" only inside a <!--- ... ---> tag-style CFML comment and asserts the validator emits does not extend Model. This covers the first regex substitution in $stripCfmlComments() (<!---[\s\S]*?--->), which previously had no test coverage.

The implementation in cli/lucli/services/Analysis.cfc was correct per the reviewer convergence and is unchanged in this commit.

Note on local test execution: bash tools/test-cli-local.sh is rejected by the runtime sandbox in this environment (same constraint the original implementer reported in the PR body). The CLI test suite in CI (pr.yml CLI job) will exercise the new tag-style spec and the now-unconditional foundBadIssue assertion on the next run.

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.

[wheels-bot]

Wheels Bot - Reviewer A

The round-1 fix commit (a9d2362) addresses both consensus findings from the previous review: the vacuous-pass in the CommentedAndReal.cfc test has been replaced with an explicit boolean-flag pattern that runs the assertion unconditionally, and a fifth it() now covers the tag-style CFML comment path that had no spec before. The core bug fix in Analysis.cfc is unchanged and correct. The PR is in good shape; I have one cosmetic nit remaining and the pre-existing duplicate-function note, both non-blocking.

Conventions

Indentation inconsistency in the new it() blocks (cli/lucli/tests/specs/services/AnalysisSpec.cfc, lines 115-231)

The four pre-existing it() blocks inside describe("validate()") (lines 80-113) each indent their body at five tabs and close with a four-tab closer. The five new it() blocks use a mixed pattern: the first opener (line 115) is at four tabs while its body and closing drop to three or four tabs; the remaining four openers (lines 140, 163, 186, 210) are at three tabs (one level short of the describe container) while their bodies are at four tabs. The code compiles and the tests register correctly because CFML closures are indentation-insensitive, but the inconsistency makes the nesting structure harder to read. A re-indent of lines 115-231 to match the surrounding four-tab opener / five-tab body convention would close this out.

Three copies of stripCfmlComments (pre-existing, noted for tracking)

Module.cfc:5707, Doctor.cfc:569 (two-regex variant), and Analysis.cfc:386 all implement the same three-regex strip. The PR body flags this as a consolidation candidate. No action required here; flagging for a follow-up extraction into Helpers.cfc.

Tests

All five new specs exercise distinct code paths (line comment, block comment, tag-style comment, commented-copy-alongside-real, controller variant). The CommentedAndReal.cfc spec now sets foundBadIssue = false before the loop and asserts expect(foundBadIssue).toBeFalse() after it, eliminating the vacuous-pass from round 1. The tag-style spec mirrors the block-comment spec and provides missing coverage for the first substitution in stripCfmlComments(). Clean.

Correctness

The stripCfmlComments() implementation is byte-for-byte identical to the copy in Module.cfc:5707 (three reReplace calls, same order, same patterns). The regexes handle cross-newline spans correctly via the non-greedy quantifier and prevent over-stripping. No off-by-one or null-deref risks introduced.

Cross-engine

reReplace() with character-class patterns and the all flag behaves identically across Lucee, Adobe CF, and BoxLang. No closures, reserved scope names, bracket-notation calls, struct-member function collisions, or Left(str, 0) patterns in the changed code.

Commits

Three commits on this branch relative to origin/develop:

• a70c02f: fix(cli): wheels validate must strip CFML comments before checking extends - 72 chars, DCO trailer present.
• 5f42850: docs(web/guides): clarify wheels validate checks active (uncommented) extends declaration - 87 chars, DCO trailer present.
• a9d2362: fix(cli): address Reviewer A/B consensus findings (round 1) - 59 chars, DCO trailer present.

All three conform to commitlint (type, optional scope, subject within 100 chars, no ALL-CAPS).

Security

No concerns. The change operates on local filesystem content read by the CLI; no HTTP surface, no SQL, no user-supplied runtime input affected.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review of the round-1 address commit is substantively accurate and I am converging on approve. The two findings A raises are correct (indentation inconsistency, pre-existing duplicate helper), both genuinely non-blocking, and the implementation and test analysis throughout is sound. The only friction is A's choice of COMMENTED state when their own summary is 'PR is in good shape... both non-blocking' — that warrants APPROVE, not COMMENTED.

Sycophancy

• Mild verdict mismatch: A describes both remaining findings as non-blocking and explicitly says "The PR is in good shape," yet chose COMMENTED over APPROVE. COMMENTED leaves the PR in a liminal state without a clear merge signal. When all findings are non-blocking, the correct verdict is APPROVE. This is not reflexive LGTM without evidence — A supports every claim — but the state choice is inconsistent with the conclusion.
• No approval of security, migration, deploy, or DI changes. This PR touches only CLI service code and tests. Appropriate.

False positives

None detected. Verified against the diff:

• Indentation inconsistency: Confirmed. The first new it() opener sits at four tabs with its body also at four tabs; the subsequent four openers sit at three tabs with bodies at four tabs; pre-existing it() openers sit at four tabs with bodies at five tabs. A's description of the mixed pattern is accurate. CFML closures are indentation-insensitive so this is cosmetic only — A's non-blocking characterization is correct.
• Three copies of $stripCfmlComments: Confirmed at Module.cfc:5707, Doctor.cfc:569 (two-regex variant), and Analysis.cfc:383. Pre-existing accumulation not introduced by this PR. A's framing as informational/out-of-scope is appropriate.

Missed issues

None detected.

• The three regex substitutions in $stripCfmlComments() use non-greedy *? with [\s\S] — correct, portable, and consistent with the existing copy in Module.cfc. No ReDoS risk for typical source file sizes.
• Each spec creates its fixture file, runs analysis.validate(), filters results by filename, and deletes the file. The foundBadIssue / issueMessages per-filename filter means leftover files from other specs cannot produce false positives. Pattern is sound.
• The inline comment explaining why activeContent is used is the right kind of comment per repo conventions: it documents a non-obvious constraint tied to a specific bug, not the what.
• Cross-engine analysis is correct: reReplace() with character-class patterns and the all flag is engine-neutral. No closures, reserved scopes, or Left(str, 0) patterns.
• All three commits on the branch conform to commitlint and carry DCO trailers. A's verification is accurate.

Verdict alignment

A's findings are two non-blocking items; APPROVE is the consistent verdict. COMMENTED is not wrong about the substance but leaves the review state ambiguous.

Convergence

Aligned. A's substantive analysis is correct on every point. No false-positive disputes, no missed issues. The only gap is A's conservative use of COMMENTED rather than APPROVE, which does not change the underlying conclusion that this PR is merge-ready pending human sign-off. Converging on approve.