fix(konsist): anchor path rules to the scanned checkout root so the commonMain guard isn't vacuously green in agent worktrees - #6665
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughKonsist tests now use shared project-relative scan paths, nested agent worktree exclusion, and standardized empty-scan messages. BLE and commonMain scans use these utilities for filtering and offender reporting. ChangesKonsist path handling
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This localized test-only change anchors path filtering to the scanned checkout so framework-boundary checks inspect the intended sources while preserving nested worktree exclusions. No actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
❌ 11 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 8 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
The konsist guards excluded agent scratch sources with `"/.claude/" in it.path`, matched against the ABSOLUTE path. Agent sessions run from worktrees under `<main checkout>/.claude/worktrees/`, so that substring is present in every file of such a checkout and the scan set became empty — the "No Framework Bleed" guard verified nothing while reporting a result. Konsist resolves its root by walking up for `gradlew`, so `KoFileDeclaration.projectPath` is relative to whichever checkout is scanned. Keying every path rule off that (`scanPath`) makes `.claude/` an exclusion only when it is nested UNDER the scanned root, which is the actual intent: `scopeFromProject` does sweep nested worktree checkouts, whose stale copies resurface long-fixed lines as phantom offenders. The empty-scan self-check stays a hard failure and now names the cause instead of the symptom. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4a8d45d to
3883667
Compare
CI triage: the 11 failures were inherited from the base commit, not this changeCodecov flagged 11 failing tests. None are konsist — the full set is:
That is the CMP 1.12.0-rc01 regression set. This branch was cut from Fix: rebased onto Re-verified on the rebased tree: No code changes were made in response to review feedback — CodeRabbit reported no actionable comments, so there are no threads to reply to. 🤖 Addressed by Claude Code |
Why
CommonMainFrameworkBoundaryTestenforces the No Framework Bleed rule fromAGENTS.md— nojava.*orandroid.*imports in anycommonMainsource set. It was silently verifying nothing for any agent session working from a worktree.Both guards excluded agent scratch sources with
"/.claude/" in it.path— matched against the absolute path. Agent worktrees are rooted at<main checkout>/.claude/worktrees/<name>, so that substring is present in every file of the entire checkout. The scan set became empty.Today that surfaces as a confusing failure, because the "scan actually reaches sources" self-check trips. But the guard was one relaxed assertion away from a vacuous green — and a rule that passes without scanning anything is worse than no rule, because it reads as verification. CI checks out at a normal path, so CI has been the only real gate for these two rules.
🛠️ What changed
Konsist resolves its project root by walking up for
gradlew(GradleProjectRootDirResolver), soKoFileDeclaration.projectPathis relative to whichever checkout is being scanned. That is the correct anchor, and every path rule now keys off it:scanPath(ProjectPath.kt) — normalizedprojectPath; replaces all uses of the absolutepath, in filters and in offender messages alike.isNestedAgentWorktree()— matches.claude/only as the first segment relative to the scanned root. Excludes nested worktrees when scanning the main checkout; excludes nothing when the scan runs from a worktree.emptyScanMessage()— the empty-scan self-check stays a hard failure (it did its job here), but now names the cause rather than the symptom and points atProjectPath.kt.The exclusion is not vestigial and was deliberately kept:
scopeFromProjectgenuinely does sweep nested.claude/worktrees/checkouts, whose stale copies resurface long-fixed lines as phantom offenders. Correct scoping — not deletion — is the fix. See the control run below.Testing Performed
:core:konsist:allTests, both path shapes, plus a negative test and a control for each direction:.claude/worktrees/<name>(this branch's worktree)import java.util.UUIDinjected intocore/modelcommonMaincommonMain declares no java importsnamesMessage.kt; injection reverted/private/tmp).claude/worktrees/fake-agent-session/core/ble/.../StaleOffender.ktisNestedAgentWorktree()forced tofalseStaleOffender.kt— proving the nested sweep is real and the exclusion is load-bearingThat last row is the one worth a reviewer's attention: it rules out the tempting simpler "fix" of just deleting the filter.
spotlessApply spotlessCheck detekt— clean, no findings.Reviewer notes
core/konsist, no production code touched.scanPath, never the absolutepath.--continueso they don't abort a baseline run) no longer applies.🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores