Fix configuration cache compatibility in all tasks#3
Merged
Conversation
Replace all project.rootDir, project.findProperty(), and project.gradle.includedBuilds access in @TaskAction methods with abstract Property<> fields wired at configuration time in Opsx.kt. - Add OpsxConfig serializable data class for settings - Convert all 14 tasks from lateinit extension to Property<> fields - Refactor SkillGenerator to accept data instead of Project - Remove dead inner task classes from Opsx.kt - Split registerWorkflowTasks into smaller methods for detekt - Update all tests for new Property-based API
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request refactors the Opsx Gradle plugin to decouple task execution from the plugin extension by introducing a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.kt (1)
25-26: Add one non-emptyincludedBuildDirscase.All of the updated setups initialize the new task input to
emptyList(), so the included-build cleanup path never gets exercised. One fixture with a nested included build would give this refactor much better protection for very little extra test code.Also applies to: 50-51, 87-88, 122-123, 153-154, 183-184, 204-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.kt` around lines 25 - 26, The tests set task.includedBuildDirs to emptyList() everywhere so the included-build cleanup path is never exercised; update the CleanTaskTest cases that call task.includedBuildDirs.set(emptyList()) to include at least one non-empty case by creating a nested included-build fixture (e.g., create a directory under the test projectDir like "included" or "nested-build") and call task.includedBuildDirs.set(listOf(includedDir)) for one test variant; ensure the test asserts that the included build directory is processed/cleaned (same assertions used for rootDir cleanup) and apply this change to each occurrence of task.includedBuildDirs.set(emptyList()) in CleanTaskTest.kt so the included-build branch is covered.src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt (1)
26-27:includedBuildDirsis unused inSyncTask.The property is declared but never referenced within the
SyncTaskimplementation—onlyincludedBuildNamesis passed toSkillGenerator. This appears to be a copy-paste artifact fromCleanTask, whereincludedBuildDirsis actually used. Remove this property to improve code clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt` around lines 26 - 27, Remove the unused includedBuildDirs property from SyncTask: delete the `@get`:Internal abstract val includedBuildDirs: ListProperty<File> declaration and any associated imports/annotations referencing it, leaving SyncTask to use includedBuildNames (which is actually passed to SkillGenerator); ensure no remaining references to includedBuildDirs exist (it was a copy-paste artifact from CleanTask) so the class compiles cleanly.src/main/kotlin/zone/clanker/opsx/task/OnboardTask.kt (1)
39-42: Bake the repo’s workflow guardrails into the onboarding prompt.Now that
buildOnboardPrompt(...)is the single place defining the guided tour, it should also teach the two repo-specific rules enforced elsewhere: start work viaopsx-propose, and don’t manually edit files across included builds. Otherwise the onboarding path can teach a workflow the repo rejects.Suggested prompt tweak
appendLine("- How to contribute changes using the opsx workflow") + appendLine("- That changes should start via `/opsx-propose` or `./gradlew -q opsx-propose`") + appendLine("- That included builds should not be edited manually; use OPSX Gradle tasks instead") if (userPrompt.isNotBlank()) {Based on learnings: Changes should be proposed via
/opsx-proposeor./gradlew -q opsx-proposecommand, and do NOT manually edit files across included builds. Use the Gradle tasks provided by OPSX.Also applies to: 51-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/task/OnboardTask.kt` around lines 39 - 42, The onboarding prompt is missing repo-specific workflow guardrails; update the buildOnboardPrompt(...) implementation (the single source for the guided tour) to append explicit instructions that all changes must be proposed via the opsx-propose entrypoints ("/opsx-propose" or "./gradlew -q opsx-propose") and that contributors must NOT manually edit files across included builds (use the provided Gradle OPSX tasks instead). Locate where PromptBuilder.srcxContext() and PromptBuilder.projectDescription(cfg) are used to compose the prompt (calls to buildOnboardPrompt in OnboardTask) and modify buildOnboardPrompt to include these two rules so the prompt text presented by OnboardTask (and the other call sites) enforces the repo workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/zone/clanker/opsx/Opsx.kt`:
- Around line 230-245: The provider lambdas for taskInfos, includedBuildNames,
and includedBuildDirs close over live Gradle model objects (rootProject.tasks
and rootProject.gradle.includedBuilds) and therefore must be evaluated during
configuration, not at task execution; change the Opsx.kt configuration so you
compute immutable snapshots (e.g., List<TaskInfo>, List<String>, List<File>) by
calling rootProject.tasks...map(...) and
rootProject.gradle.includedBuilds.map(...) immediately inside the configuration
block (instead of returning providers that reference the live model), then set
those snapshot values on the task properties so SyncTask.run() and
CleanTask.run() read only frozen data via .get() at execution time without
accessing the Gradle model. Ensure you update the code paths that set taskInfos,
includedBuildNames, and includedBuildDirs and keep
SyncTask.run()/CleanTask.run() signatures intact.
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.kt`:
- Around line 30-38: The test "generates no files" for SkillGenerator
(constructed via SkillGenerator(project.projectDir, emptyList(), emptyList())
using ProjectBuilder) has a mismatched assertion: it asserts result.isNotEmpty()
shouldBe true while the name implies no files; verify the intended behavior of
SkillGenerator.generate(), then either change the assertion to result.isEmpty()
shouldBe true if no files are expected, or rename the test description (e.g.,
"still generates instruction files") to match the existing result.isNotEmpty()
shouldBe true assertion so the test name and assertion align.
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/opsx/task/OnboardTask.kt`:
- Around line 39-42: The onboarding prompt is missing repo-specific workflow
guardrails; update the buildOnboardPrompt(...) implementation (the single source
for the guided tour) to append explicit instructions that all changes must be
proposed via the opsx-propose entrypoints ("/opsx-propose" or "./gradlew -q
opsx-propose") and that contributors must NOT manually edit files across
included builds (use the provided Gradle OPSX tasks instead). Locate where
PromptBuilder.srcxContext() and PromptBuilder.projectDescription(cfg) are used
to compose the prompt (calls to buildOnboardPrompt in OnboardTask) and modify
buildOnboardPrompt to include these two rules so the prompt text presented by
OnboardTask (and the other call sites) enforces the repo workflow.
In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt`:
- Around line 26-27: Remove the unused includedBuildDirs property from SyncTask:
delete the `@get`:Internal abstract val includedBuildDirs: ListProperty<File>
declaration and any associated imports/annotations referencing it, leaving
SyncTask to use includedBuildNames (which is actually passed to SkillGenerator);
ensure no remaining references to includedBuildDirs exist (it was a copy-paste
artifact from CleanTask) so the class compiles cleanly.
In `@src/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.kt`:
- Around line 25-26: The tests set task.includedBuildDirs to emptyList()
everywhere so the included-build cleanup path is never exercised; update the
CleanTaskTest cases that call task.includedBuildDirs.set(emptyList()) to include
at least one non-empty case by creating a nested included-build fixture (e.g.,
create a directory under the test projectDir like "included" or "nested-build")
and call task.includedBuildDirs.set(listOf(includedDir)) for one test variant;
ensure the test asserts that the included build directory is processed/cleaned
(same assertions used for rootDir cleanup) and apply this change to each
occurrence of task.includedBuildDirs.set(emptyList()) in CleanTaskTest.kt so the
included-build branch is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89a523fd-793a-4b00-bb22-c5aa04de2a2e
📒 Files selected for processing (39)
src/main/kotlin/zone/clanker/opsx/Opsx.ktsrc/main/kotlin/zone/clanker/opsx/model/OpsxConfig.ktsrc/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.ktsrc/main/kotlin/zone/clanker/opsx/task/ApplyTask.ktsrc/main/kotlin/zone/clanker/opsx/task/ArchiveTask.ktsrc/main/kotlin/zone/clanker/opsx/task/BulkArchiveTask.ktsrc/main/kotlin/zone/clanker/opsx/task/CleanTask.ktsrc/main/kotlin/zone/clanker/opsx/task/ContinueTask.ktsrc/main/kotlin/zone/clanker/opsx/task/ExploreTask.ktsrc/main/kotlin/zone/clanker/opsx/task/FeedbackTask.ktsrc/main/kotlin/zone/clanker/opsx/task/FfTask.ktsrc/main/kotlin/zone/clanker/opsx/task/ListTask.ktsrc/main/kotlin/zone/clanker/opsx/task/OnboardTask.ktsrc/main/kotlin/zone/clanker/opsx/task/ProposeTask.ktsrc/main/kotlin/zone/clanker/opsx/task/StatusTask.ktsrc/main/kotlin/zone/clanker/opsx/task/SyncTask.ktsrc/main/kotlin/zone/clanker/opsx/task/VerifyTask.ktsrc/main/kotlin/zone/clanker/opsx/workflow/ChangeReader.ktsrc/main/kotlin/zone/clanker/opsx/workflow/ChangeWriter.ktsrc/main/kotlin/zone/clanker/opsx/workflow/PromptBuilder.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.ktsrc/test/kotlin/zone/clanker/opsx/task/ApplyTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/ArchiveTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/BulkArchiveTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/ContinueTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/ExploreTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/FeedbackTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/FfTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/ListTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/OnboardTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/ProposeTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/StatusTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/SyncTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/VerifyTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/workflow/ChangeReaderTest.ktsrc/test/kotlin/zone/clanker/opsx/workflow/ChangeWriterTest.ktsrc/test/kotlin/zone/clanker/opsx/workflow/PromptBuilderTest.kt
Snapshot taskInfos, includedBuildNames, includedBuildDirs during configuration instead of using providers that close over live Gradle model. Fix misleading test name "generates no files" to "still generates instruction files".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
project.rootDir,project.findProperty(), andproject.gradle.includedBuildsaccess in@TaskActionmethods with abstractProperty<>fields wired at configuration timeOpsxConfigserializable data class for settingsSkillGeneratorto accept data instead ofProjectOpsx.ktTest plan
./gradlew buildpasses (detekt, ktlint, 400 tests, kover)./gradlew opsx-statusworks with configuration cache./gradlew opsx-listworks with configuration cacheChange proposed via opsx
This change was proposed with
opsx-proposeand applied withopsx-apply.Summary by CodeRabbit