Scaffold android-cli Gradle plugin#1
Conversation
Settings plugin wrapping Google's agent-first 'android' CLI. - Plugin ID: zone.clanker.gradle.android-cli - Artifact: zone.clanker:android-cli - clkx convention plugins (conventions/library/module/publish/ktlint/detekt/konsist/cover/toolchain) - Exec-based tasks grouped by CLI subcommand: - task/: AndroidExecTask (base), version, describe, run - task/screen/: screen capture - task/skills/: list, find, add, remove - Gradle property forwarding for skill name, query, args, JSON flag - 24 tests, 93% kover coverage (no exclusions) - Konsist forbidden-pattern checks on main source - CI: build on push/PR to main, release to Maven Central on published release
|
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 35 minutes and 52 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 (2)
📝 WalkthroughWalkthroughThis pull request establishes a complete Gradle plugin project infrastructure for wrapping Google's Android CLI tool. It introduces CI/CD workflows, build logic conventions, Maven publishing configuration, a Gradle settings plugin that registers CLI wrapper tasks, and comprehensive test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 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: 8
🧹 Nitpick comments (13)
build-logic/src/main/kotlin/clkx-testing.gradle.kts (1)
5-10: Consider extracting Kotest version into a version catalog.Three Kotest coordinates are duplicated with the same literal
5.9.1. If/when alibs.versions.tomlis added, consolidate these to a singlekotestversion key so upgrades touch one place. Not blocking for a scaffold PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-logic/src/main/kotlin/clkx-testing.gradle.kts` around lines 5 - 10, Extract the repeated Kotest version "5.9.1" into the version catalog and reference that single key from the dependencies block: add a kotest = "5.9.1" entry in libs.versions.toml (or the project version catalog) and then replace the three literals in the dependencies block referencing "io.kotest:kotest-runner-junit5", "io.kotest:kotest-assertions-core", and "io.kotest:kotest-framework-datatest" with the catalog version reference (e.g., use the libs.versions.kotest entry) so upgrades only change the single kotest version key.build-logic/src/main/kotlin/clkx-cover.gradle.kts (1)
5-21: Exclusion filters are report-wide; consider scoping toverifyonly.Excluding
@TaskActionmethods and*SettingsPluginclasses underreports.filtersremoves them from the HTML/XML reports as well as from verification. If you want the HTML report to still show these classes (useful for spotting that they exist and are uncovered) while keeping the 90% gate focused on testable code, move the excludes underverify { rule { ... } }instead of the top-levelfilters. Otherwise, this looks consistent with the 93% coverage claim in the PR description.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-logic/src/main/kotlin/clkx-cover.gradle.kts` around lines 5 - 21, The current kover configuration places excludes under reports.filters which removes them from both HTML/XML reports and verification; move the annotatedBy("org.gradle.api.tasks.TaskAction") and classes("*$SettingsPlugin", "*$SettingsPlugin$*") exclusion entries out of reports { filters { ... } } and instead add them under verify { rule { ... } } so the minBound(90) verification uses those excludes while the reports still show the unexcluded classes; update the kover block (kover -> reports -> filters -> remove those excludes; kover -> reports -> verify -> rule -> include the excludes alongside minBound(90)) to implement this scoping change..github/workflows/build.yml (1)
12-27: Consider adding concurrency cancellation for PR pushes.Rapid pushes to a PR will queue redundant builds. A small
concurrencyblock saves CI time and Gradle cache contention:♻️ Suggested addition
permissions: contents: read +concurrency: + group: build-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: build: runs-on: ubuntu-latestAlso optional: pin third-party actions (
gradle/actions/setup-gradle@v4) by commit SHA for supply-chain hardening — not required for a scaffold.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 12 - 27, Add a GitHub Actions concurrency block to the workflow's jobs.build section to cancel in-progress runs for the same PR on new pushes (use the concurrency key with a name like "build-${{ github.ref }}" and cancel-in-progress: true) so redundant Gradle builds started by rapid PR pushes are canceled; optionally pin external actions like actions/checkout@v4 and gradle/actions/setup-gradle@v4 to specific commit SHAs for supply-chain hardening.build-logic/src/main/kotlin/clkx-library.gradle.kts (1)
4-17: Verifyclkx-libraryandclkx-conventionsare never co-applied.
clkx-conventionsappliesclkx-publish, which callsmavenPublishing { configure(GradlePlugin(...)) }, whileclkx-librarycallsconfigure(JavaLibrary(...)). If any module ever applies both, the laterconfigure(...)call silently overrides the earlier one, changing the publication type. Currently, only the:pluginmodule appliesclkx-conventionsand no modules applyclkx-library, so this is safe today.Consider either:
- Dropping
com.vanniktech.maven.publishfromclkx-publishand lettingclkx-libraryandclkx-conventionseach own theirconfigure(...)calls, or- Adding a check (or README note) that
clkx-libraryandclkx-conventionsare mutually exclusive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-logic/src/main/kotlin/clkx-library.gradle.kts` around lines 4 - 17, The mavenPublishing configure call in clkx-library can silently override the configure call from clkx-conventions (which applies clkx-publish/GradlePlugin), so add a guard to prevent both from being applied: in the clkx-library plugin/application code (where you apply id("clkx-library")/mavenPublishing/JavaLibrary) check for the presence of clkx-conventions/clkx-publish (e.g., project.plugins.hasPlugin("clkx-conventions") or pluginManager.withPlugin("clkx-conventions") ) and throw a clear GradleException (or fail the configuration) if both are present; alternatively, if you prefer ownership changes, remove the com.vanniktech.maven.publish application from clkx-publish and let each module call mavenPublishing.configure(...) for its own publication type (JavaLibrary vs GradlePlugin) and update README to state they are mutually exclusive.build-logic/src/main/kotlin/clkx-konsist.gradle.kts (1)
7-13:slopTestclasspath doesn't include thetestsource set output.
compileClasspath/runtimeClasspathonly inherit frommain, and the configurations extendtestImplementation/testRuntimeOnly(dependencies), notsourceSets["test"].output(compiled test classes). That's likely intentional here since the Konsist check only needs production scope — just flagging so future slop tests that want to reuse test helpers/fixtures don't silently compile against a narrower classpath than expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-logic/src/main/kotlin/clkx-konsist.gradle.kts` around lines 7 - 13, The slopTest source set currently only adds sourceSets["main"].output to its compileClasspath and runtimeClasspath, so compiled test classes (sourceSets["test"].output) are not available; update the slopTest configuration (the val slopTest block) to also include sourceSets["test"].output in compileClasspath and runtimeClasspath so slop tests can compile/run against test helpers/fixtures, and keep the existing configuration extensions from testImplementation/testRuntimeOnly as-is.plugin/src/slopTest/kotlin/zone/clanker/gradle/androidcli/ForbiddenPatternTest.kt (1)
12-28: Substring checks are brittle and will false-positive on strings/comments.
it.text.contains("println(")/"System.err"/"System.out"match anywhere in the file, including doc comments, string literals, and unrelated identifiers (e.g., a futureSystem.out_of_memory_handlerwouldn't, but"// prefer logging over println(...)"in a KDoc would). Konsist provides stricter AST-based checks —scope.functions()/files.functionsfiltered by name, andscope.imports/file property access — that avoid this. Consider something like:scope.imports.assertFalse { it.name == "java.lang.System.out" || it.name == "java.lang.System.err" } scope.functions(includeNested = true).assertFalse { it.name == "println" } // for calls, use Konsist's call APIsNot blocking, but today a stray doc-comment mentioning
println(would break the build.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/slopTest/kotlin/zone/clanker/gradle/androidcli/ForbiddenPatternTest.kt` around lines 12 - 28, Replace brittle substring checks that inspect raw file text (scope.files.assertFalse { it.text.contains("...") }) with AST-aware checks: use scope.functions(includeNested = true).assertFalse { it.name == "println" } to detect actual println function declarations/calls and use scope.imports.assertFalse { it.name == "java.lang.System.out" || it.name == "java.lang.System.err" } (or equivalent file-level import/property accessors) to detect direct System.out/System.err usage; update the three then-blocks that reference println(, System.err and System.out to use these Konsist APIs (scope.functions and scope.imports) so comments and string literals no longer trigger false positives.build-logic/build.gradle.kts (1)
10-17: Consider centralizing plugin versions in the version catalog.Hard-coding versions here duplicates what a
libs.versions.tomltypically owns and makes bumps (e.g., Detekt, Kover) noisier. Given the scaffold is still small this is purely optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-logic/build.gradle.kts` around lines 10 - 17, The dependencies block currently hardcodes plugin versions (e.g., "org.jetbrains.kotlinx:kover-gradle-plugin:0.9.1", "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.23.8", "org.jlleitschuh.gradle:ktlint-gradle:12.2.0", "com.vanniktech:gradle-maven-publish-plugin:0.32.0", "org.gradle.toolchains:foojay-resolver:1.0.0"); centralize these by adding entries in the version catalog (libs.versions.toml) for each plugin coordinate and version, then update the dependencies block in build.gradle.kts to use the catalog references (e.g., libs.plugins.detekt, libs.plugins.kover, etc.) or the typed accessors (libs.xxx) instead of hardcoded :version strings; ensure the version keys are named clearly (kover, detekt, ktlint, mavenPublish, foojay) and that any build logic that expects the previous strings is adjusted to use the catalog symbols..github/workflows/release.yml (1)
31-34: Redundant./gradlew buildbefore publish.
publishAndReleaseToMavenCentralalready depends onassemble/checkthrough the publication and sign chains, so the precedingbuildstep roughly doubles wall-clock time for releases. If you want a pre-publish safety net, consider./gradlew check publishAndReleaseToMavenCentralin a single invocation so Gradle can reuse the task graph and up-to-date checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 31 - 34, Replace the separate "Build and test" and "Publish to Maven Central" steps with a single Gradle invocation so Gradle can reuse the task graph; specifically, update the step(s) referencing the run commands in the steps named "Build and test" and "Publish to Maven Central" to invoke ./gradlew check publishAndReleaseToMavenCentral -PVERSION_NAME=${{ steps.version.outputs.version }} (instead of running ./gradlew build and then publishAndReleaseToMavenCentral separately).plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/screen/AndroidScreenCaptureTask.kt (1)
1-13: Consider exposing screenshot output path as a task input.Users invoking
android-screen-capturetypically want to control where the screenshot is written. Relying solely onextension.extraArgs(which is global across all tasks) makes per-invocation output path awkward. Consider a task-level@Input/@Optionor a dedicated-Pproperty akin toPROP_SKILL/PROP_ARGS, forwarded as the CLI's output flag. Non-blocking for the scaffold.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/screen/AndroidScreenCaptureTask.kt` around lines 1 - 13, The task currently hardcodes CLI args via extension.extraArgs and doesn't expose a per-task output path; add a task-level property (e.g., an `@Input` and `@Option` annotated var named outputPath or screenshotPath) on AndroidScreenCaptureTask and include its flag in buildArgs() so doFirst uses it when present; update buildArgs() to merge listOf("screen","capture") + optional output flag (from the new outputPath property) + extension.extraArgs, and document that outputPath overrides/augments the global extraArgs.plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidDescribeTask.kt (1)
12-19: Minor:toBoolean()silently accepts any non-"true" value as false.
"yes","1", or a typo like"ture"all evaluate tofalse, which may surprise users who expect-Pzone.clanker.android-cli.json=1to work. Acceptable for a scaffold; consider documenting the accepted values or usingtoBooleanStrictOrNull()to surface misuse. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidDescribeTask.kt` around lines 12 - 19, The JSON flag parsing in buildArgs (function buildArgs, property AndroidCli.PROP_JSON) uses String.toBoolean() which silently treats any non-"true" value as false; change to String.toBooleanStrictOrNull() and handle the null case so malformed values are surfaced: if the property is absent keep the existing default (false), but if the property is present and toBooleanStrictOrNull() returns null, throw a clear GradleException or log an error indicating acceptable values ("true"/"false") so callers using "1"/"yes"/typos get immediate feedback; leave the rest of buildArgs (adding "describe", "--json", and extension.extraArgs) unchanged.plugin/src/main/kotlin/zone/clanker/gradle/androidcli/AndroidCli.kt (2)
43-47: Nit: explicitAction { }wrapper is unnecessary.
Gradle.rootProjectaccepts a SAMAction<? super Project>, so a lambda works directly:- settings.gradle.rootProject( - Action { rootProject -> - registerTasks(rootProject, extension) - }, - ) + settings.gradle.rootProject { rootProject -> + registerTasks(rootProject, extension) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/AndroidCli.kt` around lines 43 - 47, The call uses an unnecessary explicit Action wrapper; replace the Action { rootProject -> registerTasks(rootProject, extension) } with a direct lambda passed to settings.gradle.rootProject so that settings.gradle.rootProject { rootProject -> registerTasks(rootProject, extension) } (i.e., remove the explicit Action wrapper around the lambda used to invoke registerTasks).
51-95: Optional: extract task-registration boilerplate.The 9
registerblocks all repeatit.group = GROUP; it.description = ...; it.extension = extension. A small helper would shrink the surface and make adding new tasks a one-liner.♻️ Sketch
private inline fun <reified T : AndroidExecTask> Project.registerAndroidTask( name: String, description: String, extension: SettingsExtension, ) = tasks.register(name, T::class.java) { it.group = GROUP it.description = description it.extension = extension }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/AndroidCli.kt` around lines 51 - 95, Extract the repeated task-registration boilerplate inside registerTasks by adding a small helper (e.g. a private inline fun registerAndroidTask<T : AndroidExecTask>(name: String, description: String, extension: SettingsExtension)) and then replace the nine tasks registered in registerTasks (TASK_VERSION, TASK_DESCRIBE, TASK_SCREEN_CAPTURE, TASK_SKILLS_LIST, TASK_SKILLS_FIND, TASK_SKILLS_ADD, TASK_SKILLS_REMOVE, TASK_RUN) with single-line calls to that helper that pass the corresponding AndroidVersionTask/AndroidDescribeTask/AndroidScreenCaptureTask/AndroidSkillsListTask/AndroidSkillsFindTask/AndroidSkillsAddTask/AndroidSkillsRemoveTask/AndroidRunTask classes, the description strings, and the extension; the helper should set it.group = GROUP, it.description = description and it.extension = extension so behavior is unchanged.plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt (1)
30-77: Test scoping caveat withBehaviorSpec.Project/extension creation lives in the
given/whenbody rather than insidethen. With Kotest's default isolation, that body is executed when the spec is constructed and shared across the containedthenblocks. Each given/when currently has only a singlethen, so behavior is correct today, but adding a secondthento any block would silently leak mutations (e.g.,ext.extraArgsat Line 70,extraPropertiesset at Line 52/61) into sibling tests.Consider moving setup into
thenblocks (or usingbeforeTest) to keep tests independent as coverage grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt` around lines 30 - 77, The tests use Kotest BehaviorSpec with project/extension setup (newProject(), project.extensions.extraProperties.set(AndroidCli.PROP_JSON, ...), and ext.extraArgs) inside given/when blocks which run at spec construction and thus share state across their then blocks; move the creation of the Project and extension and any mutations to inside each then block (or use a per-test lifecycle like beforeTest) so AndroidDescribeTask/AndroidVersionTask tests (referencing AndroidCli.TASK_DESCRIBE, AndroidCli.TASK_VERSION, AndroidDescribeTask, AndroidCli.PROP_JSON and ext.extraArgs) get a fresh newProject() per assertion and cannot leak state between tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 20-25: The step "Extract version from tag" currently interpolates
github.event.release.tag_name directly in the shell; instead set an environment
variable (e.g., RELEASE_TAG) to ${{ github.event.release.tag_name }} for that
step and reference "$RELEASE_TAG" inside the run block, then perform the strip
operation (VERSION="${RELEASE_TAG#v}") and echo to GITHUB_OUTPUT; ensure the
variable is double-quoted when used to safely handle special characters.
In `@build-logic/src/main/kotlin/clkx-plugin.gradle.kts`:
- Around line 1-5: Add a mavenPublishing configuration that registers this
Gradle plugin artifact the same way the library convention does: call
mavenPublishing.configure(GradlePlugin(...)) for the plugin artifact and set its
JavaLibrary publishing options to JavaLibrary(javadocJar = JavadocJar.Empty())
so the published plugin gets the empty Javadoc jar and consistent POM metadata;
locate the plugins block in clkx-plugin.gradle.kts and add a
mavenPublishing.configure block referencing GradlePlugin and
JavaLibrary/JavadocJar.Empty to mirror the sibling
clkx-library.gradle.kts/clkx-publish.gradle.kts setup.
In `@config/detekt.yml`:
- Line 106: The allowedPatterns value is over-escaped so Detekt sees literal
backslashes and the exemption never matches TODO(owner): comments; update the
allowedPatterns entry (allowedPatterns) to use single backslashes in the regex
(e.g., change the current 'TODO\\(.*\\):' to use single backslashes like
'TODO\(.*\):' so Detekt receives the intended pattern and the ForbiddenComment
exemption for TODO(name): works correctly.
In `@gradle/wrapper/gradle-wrapper.properties`:
- Line 3: Upgrade the incompatible Gradle plugins to versions that support
Gradle 9: update the ktlint-gradle plugin (org.jlleitschuh.gradle.ktlint) from
12.2.0 to at least 13.1.0 (recommend 14.2.0) and update the detekt Gradle plugin
(io.gitlab.arturbosch.detekt / detekt-gradle-plugin) from 1.23.8 to a Detekt 2.x
release (2.0.0-alpha or preferably a stable 2.x) in your plugin declarations
(build.gradle / build.gradle.kts or pluginManagement block) and refresh the
Gradle wrapper to use the existing distributionUrl (Gradle 9.4.1) so those
plugin versions are compatible.
In
`@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidExecTask.kt`:
- Around line 17-23: checkBinaryInstalled() currently invokes "which" which is
not portable and fails on Windows and when extension.binary is an
absolute/path-containing value; change the logic to first treat extension.binary
as a Path and if it contains a file separator or is absolute use
java.nio.file.Path/Files.exists + Files.isExecutable (or File.canExecute()) to
validate it, otherwise perform a PATH search by splitting System.getenv("PATH")
and checking each entry for an executable named extension.binary (or, on
Windows, prefer calling "where" only as a fallback), and throw the same error
when no executable is found; update the method to reference extension.binary and
checkBinaryInstalled() accordingly.
In
`@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidRunTask.kt`:
- Around line 12-16: The buildArgs() function naively splits
AndroidCli.PROP_ARGS with raw.split(' '), which breaks quoted arguments and
doesn't collapse whitespace; replace this with a shell-like tokenizer so quoted
strings are preserved (e.g. use a small lexer such as Apache Commons Exec
CommandLine.parse or implement a minimal parser that splits on runs of
whitespace and respects single/double quotes), keep appending
extension.extraArgs as before, and ensure empty input still yields an empty list
rather than spurious tokens.
In
`@plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidCliPluginTest.kt`:
- Around line 47-69: The test shares a mutable AndroidCli.SettingsExtension
instance (ext) across multiple when/then leaves which makes the tests
order-dependent; instead create a fresh SettingsExtension inside each when or
each then so assertions don't rely on state mutated by other leaves—replace the
shared val ext = AndroidCli.SettingsExtension() at the given scope with
instantiations inside the individual `when` or `then` blocks that reference
AndroidCli.SettingsExtension and ext so each assertion (defaults and mutability
checks) uses its own new instance.
In `@plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt`:
- Around line 200-209: The test currently relies on an external "which" binary;
instead create a temporary executable in the test and prepend its directory to
PATH so the test does not depend on system image. In the
AndroidExecTask.checkBinaryInstalled test (where ext.binary is set and
AndroidCli.TASK_VERSION/AndroidVersionTask used), create a temp file executable
(a small shell script or platform-appropriate no-op), mark it executable, add
its parent directory to the PATH environment for the test process, set
ext.binary to the temp filename, then call task.checkBinaryInstalled(); finally
clean up the temp file and restore PATH.
---
Nitpick comments:
In @.github/workflows/build.yml:
- Around line 12-27: Add a GitHub Actions concurrency block to the workflow's
jobs.build section to cancel in-progress runs for the same PR on new pushes (use
the concurrency key with a name like "build-${{ github.ref }}" and
cancel-in-progress: true) so redundant Gradle builds started by rapid PR pushes
are canceled; optionally pin external actions like actions/checkout@v4 and
gradle/actions/setup-gradle@v4 to specific commit SHAs for supply-chain
hardening.
In @.github/workflows/release.yml:
- Around line 31-34: Replace the separate "Build and test" and "Publish to Maven
Central" steps with a single Gradle invocation so Gradle can reuse the task
graph; specifically, update the step(s) referencing the run commands in the
steps named "Build and test" and "Publish to Maven Central" to invoke ./gradlew
check publishAndReleaseToMavenCentral -PVERSION_NAME=${{
steps.version.outputs.version }} (instead of running ./gradlew build and then
publishAndReleaseToMavenCentral separately).
In `@build-logic/build.gradle.kts`:
- Around line 10-17: The dependencies block currently hardcodes plugin versions
(e.g., "org.jetbrains.kotlinx:kover-gradle-plugin:0.9.1",
"io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.23.8",
"org.jlleitschuh.gradle:ktlint-gradle:12.2.0",
"com.vanniktech:gradle-maven-publish-plugin:0.32.0",
"org.gradle.toolchains:foojay-resolver:1.0.0"); centralize these by adding
entries in the version catalog (libs.versions.toml) for each plugin coordinate
and version, then update the dependencies block in build.gradle.kts to use the
catalog references (e.g., libs.plugins.detekt, libs.plugins.kover, etc.) or the
typed accessors (libs.xxx) instead of hardcoded :version strings; ensure the
version keys are named clearly (kover, detekt, ktlint, mavenPublish, foojay) and
that any build logic that expects the previous strings is adjusted to use the
catalog symbols.
In `@build-logic/src/main/kotlin/clkx-cover.gradle.kts`:
- Around line 5-21: The current kover configuration places excludes under
reports.filters which removes them from both HTML/XML reports and verification;
move the annotatedBy("org.gradle.api.tasks.TaskAction") and
classes("*$SettingsPlugin", "*$SettingsPlugin$*") exclusion entries out of
reports { filters { ... } } and instead add them under verify { rule { ... } }
so the minBound(90) verification uses those excludes while the reports still
show the unexcluded classes; update the kover block (kover -> reports -> filters
-> remove those excludes; kover -> reports -> verify -> rule -> include the
excludes alongside minBound(90)) to implement this scoping change.
In `@build-logic/src/main/kotlin/clkx-konsist.gradle.kts`:
- Around line 7-13: The slopTest source set currently only adds
sourceSets["main"].output to its compileClasspath and runtimeClasspath, so
compiled test classes (sourceSets["test"].output) are not available; update the
slopTest configuration (the val slopTest block) to also include
sourceSets["test"].output in compileClasspath and runtimeClasspath so slop tests
can compile/run against test helpers/fixtures, and keep the existing
configuration extensions from testImplementation/testRuntimeOnly as-is.
In `@build-logic/src/main/kotlin/clkx-library.gradle.kts`:
- Around line 4-17: The mavenPublishing configure call in clkx-library can
silently override the configure call from clkx-conventions (which applies
clkx-publish/GradlePlugin), so add a guard to prevent both from being applied:
in the clkx-library plugin/application code (where you apply
id("clkx-library")/mavenPublishing/JavaLibrary) check for the presence of
clkx-conventions/clkx-publish (e.g.,
project.plugins.hasPlugin("clkx-conventions") or
pluginManager.withPlugin("clkx-conventions") ) and throw a clear GradleException
(or fail the configuration) if both are present; alternatively, if you prefer
ownership changes, remove the com.vanniktech.maven.publish application from
clkx-publish and let each module call mavenPublishing.configure(...) for its own
publication type (JavaLibrary vs GradlePlugin) and update README to state they
are mutually exclusive.
In `@build-logic/src/main/kotlin/clkx-testing.gradle.kts`:
- Around line 5-10: Extract the repeated Kotest version "5.9.1" into the version
catalog and reference that single key from the dependencies block: add a kotest
= "5.9.1" entry in libs.versions.toml (or the project version catalog) and then
replace the three literals in the dependencies block referencing
"io.kotest:kotest-runner-junit5", "io.kotest:kotest-assertions-core", and
"io.kotest:kotest-framework-datatest" with the catalog version reference (e.g.,
use the libs.versions.kotest entry) so upgrades only change the single kotest
version key.
In `@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/AndroidCli.kt`:
- Around line 43-47: The call uses an unnecessary explicit Action wrapper;
replace the Action { rootProject -> registerTasks(rootProject, extension) } with
a direct lambda passed to settings.gradle.rootProject so that
settings.gradle.rootProject { rootProject -> registerTasks(rootProject,
extension) } (i.e., remove the explicit Action wrapper around the lambda used to
invoke registerTasks).
- Around line 51-95: Extract the repeated task-registration boilerplate inside
registerTasks by adding a small helper (e.g. a private inline fun
registerAndroidTask<T : AndroidExecTask>(name: String, description: String,
extension: SettingsExtension)) and then replace the nine tasks registered in
registerTasks (TASK_VERSION, TASK_DESCRIBE, TASK_SCREEN_CAPTURE,
TASK_SKILLS_LIST, TASK_SKILLS_FIND, TASK_SKILLS_ADD, TASK_SKILLS_REMOVE,
TASK_RUN) with single-line calls to that helper that pass the corresponding
AndroidVersionTask/AndroidDescribeTask/AndroidScreenCaptureTask/AndroidSkillsListTask/AndroidSkillsFindTask/AndroidSkillsAddTask/AndroidSkillsRemoveTask/AndroidRunTask
classes, the description strings, and the extension; the helper should set
it.group = GROUP, it.description = description and it.extension = extension so
behavior is unchanged.
In
`@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidDescribeTask.kt`:
- Around line 12-19: The JSON flag parsing in buildArgs (function buildArgs,
property AndroidCli.PROP_JSON) uses String.toBoolean() which silently treats any
non-"true" value as false; change to String.toBooleanStrictOrNull() and handle
the null case so malformed values are surfaced: if the property is absent keep
the existing default (false), but if the property is present and
toBooleanStrictOrNull() returns null, throw a clear GradleException or log an
error indicating acceptable values ("true"/"false") so callers using
"1"/"yes"/typos get immediate feedback; leave the rest of buildArgs (adding
"describe", "--json", and extension.extraArgs) unchanged.
In
`@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/screen/AndroidScreenCaptureTask.kt`:
- Around line 1-13: The task currently hardcodes CLI args via
extension.extraArgs and doesn't expose a per-task output path; add a task-level
property (e.g., an `@Input` and `@Option` annotated var named outputPath or
screenshotPath) on AndroidScreenCaptureTask and include its flag in buildArgs()
so doFirst uses it when present; update buildArgs() to merge
listOf("screen","capture") + optional output flag (from the new outputPath
property) + extension.extraArgs, and document that outputPath overrides/augments
the global extraArgs.
In
`@plugin/src/slopTest/kotlin/zone/clanker/gradle/androidcli/ForbiddenPatternTest.kt`:
- Around line 12-28: Replace brittle substring checks that inspect raw file text
(scope.files.assertFalse { it.text.contains("...") }) with AST-aware checks: use
scope.functions(includeNested = true).assertFalse { it.name == "println" } to
detect actual println function declarations/calls and use
scope.imports.assertFalse { it.name == "java.lang.System.out" || it.name ==
"java.lang.System.err" } (or equivalent file-level import/property accessors) to
detect direct System.out/System.err usage; update the three then-blocks that
reference println(, System.err and System.out to use these Konsist APIs
(scope.functions and scope.imports) so comments and string literals no longer
trigger false positives.
In `@plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt`:
- Around line 30-77: The tests use Kotest BehaviorSpec with project/extension
setup (newProject(),
project.extensions.extraProperties.set(AndroidCli.PROP_JSON, ...), and
ext.extraArgs) inside given/when blocks which run at spec construction and thus
share state across their then blocks; move the creation of the Project and
extension and any mutations to inside each then block (or use a per-test
lifecycle like beforeTest) so AndroidDescribeTask/AndroidVersionTask tests
(referencing AndroidCli.TASK_DESCRIBE, AndroidCli.TASK_VERSION,
AndroidDescribeTask, AndroidCli.PROP_JSON and ext.extraArgs) get a fresh
newProject() per assertion and cannot leak state between tests.
🪄 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: 5fb2082a-3551-45b4-81f9-5f98356f9404
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (37)
.github/workflows/build.yml.github/workflows/release.ymlbuild-logic/build.gradle.ktsbuild-logic/settings.gradle.ktsbuild-logic/src/main/kotlin/clkx-conventions.gradle.ktsbuild-logic/src/main/kotlin/clkx-cover.gradle.ktsbuild-logic/src/main/kotlin/clkx-detekt.gradle.ktsbuild-logic/src/main/kotlin/clkx-konsist.gradle.ktsbuild-logic/src/main/kotlin/clkx-ktlint.gradle.ktsbuild-logic/src/main/kotlin/clkx-library.gradle.ktsbuild-logic/src/main/kotlin/clkx-module.gradle.ktsbuild-logic/src/main/kotlin/clkx-plugin.gradle.ktsbuild-logic/src/main/kotlin/clkx-publish.gradle.ktsbuild-logic/src/main/kotlin/clkx-testing.gradle.ktsbuild-logic/src/main/kotlin/clkx-toolchain.gradle.ktsbuild-logic/src/main/kotlin/zone/clanker/gradle/conventions/ClkxSettingsPlugin.ktconfig/detekt.ymlgradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batplugin/build.gradle.ktsplugin/gradle.propertiesplugin/src/main/kotlin/zone/clanker/gradle/androidcli/AndroidCli.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidDescribeTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidExecTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidRunTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidVersionTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/screen/AndroidScreenCaptureTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/skills/AndroidSkillsAddTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/skills/AndroidSkillsFindTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/skills/AndroidSkillsListTask.ktplugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/skills/AndroidSkillsRemoveTask.ktplugin/src/slopTest/kotlin/zone/clanker/gradle/androidcli/ForbiddenPatternTest.ktplugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidCliPluginTest.ktplugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.ktsettings.gradle.kts
| - name: Extract version from tag | ||
| id: version | ||
| run: | | ||
| VERSION="${{ github.event.release.tag_name }}" | ||
| VERSION="${VERSION#v}" | ||
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions script injection hardening github.event context run step
💡 Result:
Script injection in GitHub Actions occurs when untrusted inputs from the github.event context, such as github.event.pull_request.title or github.event.issue.body, are interpolated into 'run' steps, allowing attackers to execute arbitrary shell commands. This is particularly risky in workflows triggered by pull_request_target, workflow_run, or similar high-privilege events where attackers control parts of github.event. Vulnerable example: - name: Check PR title run: | title="${{ github.event.pull_request.title }}" if [[ $title =~ ^octocat ]]; then echo "Valid" fi An attacker sets PR title to 'a"; ls
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://docs.github.com/en/actions/reference/security/secure-use
- 3: https://docs.github.com/actions/learn-github-actions/security-hardening-for-github-actions
- 4: https://docs.github.com/en/enterprise-server@3.0/actions/security-guides/security-hardening-for-github-actions
- 5: https://www.wiz.io/blog/github-actions-security-guide
- 6: https://openssf.org/blog/2024/08/12/mitigating-attack-vectors-in-github-workflows/
🌐 Web query:
GitHub Actions github.event.release.tag_name untrusted field security
💡 Result:
github.event.release.tag_name in GitHub Actions is generally considered a trusted field and not listed among the explicitly untrusted inputs by official GitHub documentation or security advisories. Official GitHub docs on script injections and untrusted inputs (e.g., issue titles, PR bodies, comments) do not include github.event.release.tag_name or other release event fields in their lists of potentially attacker-controlled data. The tag_name corresponds to a Git tag ref created by users with write access to the repository, making it controlled by trusted actors rather than external contributors like in pull_request events. However, general best practices for GitHub Actions security recommend treating many github.event fields cautiously to prevent script injection when passed directly into shell commands or expressions. For tag_name specifically: - It can contain special characters allowed in Git refs (e.g., zzz";echo${IFS}"hello";#), which could lead to command injection if unsanitized in run: steps. - Release events are triggered by repository collaborators, so the risk is lower than for fork PRs or issues, but still present if workflows use elevated permissions. Mitigation: - Use environment variables: env: TAG_NAME: ${{ github.event.release.tag_name }} then run: echo $TAG_NAME - Validate/sanitize before use (e.g., regex check for expected semver format). - Prefer github.ref_name for tag-triggered workflows, as it's equivalent and sometimes more reliable. No known CVEs or breaches specifically exploiting github.event.release.tag_name as of 2026-04-17. Primary risks in release workflows stem from other factors like untrusted actions or improper GITHUB_TOKEN permissions.
Citations:
- 1: https://securitylab.github.com/resources/github-actions-untrusted-input/
- 2: https://docs.github.com/en/actions/concepts/security/script-injections
- 3: https://docs.github.com/en/actions/reference/contexts-reference
- 4: https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-githubs-security-features-to-secure-your-use-of-github-actions
🌐 Web query:
GitHub Actions release event context security sensitive fields
💡 Result:
In GitHub Actions workflows triggered by the 'release' event (e.g., on: release: types: [published]), the github.event.release context provides the full release object from the webhook payload, which matches the GitHub REST API Release schema. This includes fields like id, tag_name, target_commitish, name, body, draft, prerelease, author (user details), assets array (with download URLs, sizes), and timestamps (created_at, published_at). No fields contain secrets or tokens by default. The only known sensitive field in the broader github context is github.token, but it is not part of github.event.release. Security considerations: github.event data, including release fields like body or asset names, should be treated as potentially untrusted if the release creator has write access, as it could enable script injection if interpolated into commands without sanitization. Use safe handling practices like passing to JS actions or env vars with quoting. GitHub redacts secrets from logs but does not mask event data. Release workflows typically run with full GITHUB_TOKEN access, so minimize permissions and avoid untrusted code execution.
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 2: https://docs.github.com/en/actions/reference/contexts-reference
- 3: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows
- 4: https://docs.github.com/developers/webhooks-and-events/webhook-events-and-payloads
- 5: https://docs.github.com/rest/releases/releases
Best practice: Use environment variables for release tag names instead of direct interpolation.
While github.event.release.tag_name is controlled by repository maintainers and is not listed in GitHub's official untrusted inputs, passing it through an environment variable before use in shell commands aligns with GitHub's security hardening guidance and prevents potential issues if special characters in the tag name are mishandled.
Recommended fix
- name: Extract version from tag
id: version
+ env:
+ RAW_TAG: ${{ github.event.release.tag_name }}
run: |
- VERSION="${{ github.event.release.tag_name }}"
- VERSION="${VERSION#v}"
+ VERSION="${RAW_TAG#v}"
echo "version=$VERSION" >> "$GITHUB_OUTPUT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Extract version from tag | |
| id: version | |
| run: | | |
| VERSION="${{ github.event.release.tag_name }}" | |
| VERSION="${VERSION#v}" | |
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | |
| - name: Extract version from tag | |
| id: version | |
| env: | |
| RAW_TAG: ${{ github.event.release.tag_name }} | |
| run: | | |
| VERSION="${RAW_TAG#v}" | |
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 20 - 25, The step "Extract
version from tag" currently interpolates github.event.release.tag_name directly
in the shell; instead set an environment variable (e.g., RELEASE_TAG) to ${{
github.event.release.tag_name }} for that step and reference "$RELEASE_TAG"
inside the run block, then perform the strip operation
(VERSION="${RELEASE_TAG#v}") and echo to GITHUB_OUTPUT; ensure the variable is
double-quoted when used to safely handle special characters.
There was a problem hiding this comment.
__
(-.-)
/ \ matching siblings, not
( /\ ) changing just this repo
\_/\_/
Sensible in isolation, but this release.yml is a copy of our sibling Clanker repos (clkx-agents, wrkx, srcx, opsx) — they all use the same ${{ github.event.release.tag_name }} interpolation. tag_name comes from an event that already requires write access to trigger, so the blast radius is low. If we tighten this we'll do it in a sweep across all repos at once, not one-off here.
Parking as known-and-intentional for now. 🐰
There was a problem hiding this comment.
@ClankerGuru — Understood, that's a reasonable call. The event already requires write access, so the risk is bounded, and keeping the pattern consistent across the sibling repos makes a coordinated sweep much cleaner. I'll note this for future reviews. 🐰
✏️ Learnings added
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: .github/workflows/release.yml:20-25
Timestamp: 2026-04-17T20:11:42.419Z
Learning: In the ClankerGuru family of repos (android-cli, clkx-agents, wrkx, srcx, opsx), direct interpolation of `${{ github.event.release.tag_name }}` in `release.yml` run steps is intentional and consistent across all sibling repos. The pattern is considered acceptable because the `release: published` event requires repository write access to trigger (low blast radius). Any hardening (e.g., moving to an env var) will be done as a coordinated sweep across all repos, not one-off. Do not flag this pattern in individual PR reviews for these repos.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: build-logic/src/main/kotlin/clkx-plugin.gradle.kts:1-5
Timestamp: 2026-04-17T20:11:24.722Z
Learning: In the android-cli Gradle project (build-logic/src/main/kotlin/), the Vanniktech Maven publish configuration is intentionally split across two convention scripts:
- `clkx-plugin.gradle.kts` applies only the `java-gradle-plugin` and `com.vanniktech.maven.publish` plugin IDs.
- `clkx-publish.gradle.kts` owns the `mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) }` block.
Both are composed together by `clkx-conventions.gradle.kts` (which applies `clkx-plugin` and `clkx-publish`), so every plugin module gets the full publishing config. The split is intentional: `clkx-publish` is reused for non-plugin library artifacts as well (via `clkx-library.gradle.kts`, which uses `JavaLibrary` instead of `GradlePlugin`). Do not flag the absence of a `mavenPublishing` block in `clkx-plugin.gradle.kts` as a missing configuration.
| plugins { | ||
| id("clkx-module") | ||
| `java-gradle-plugin` | ||
| id("com.vanniktech.maven.publish") | ||
| } |
There was a problem hiding this comment.
Missing mavenPublishing configuration for the Gradle plugin artifact.
Unlike clkx-library.gradle.kts (which configures JavaLibrary(javadocJar = JavadocJar.Empty())), this script applies com.vanniktech.maven.publish without any mavenPublishing.configure(GradlePlugin(...)) block. The sibling clkx-publish.gradle.kts has that configuration but does not appear to be applied here. As-is, the published plugin artifact won't have the intended empty-Javadoc behavior and POM configuration may be inconsistent with the library convention.
Proposed fix
plugins {
id("clkx-module")
`java-gradle-plugin`
- id("com.vanniktech.maven.publish")
+ id("clkx-publish")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| plugins { | |
| id("clkx-module") | |
| `java-gradle-plugin` | |
| id("com.vanniktech.maven.publish") | |
| } | |
| plugins { | |
| id("clkx-module") | |
| `java-gradle-plugin` | |
| id("clkx-publish") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build-logic/src/main/kotlin/clkx-plugin.gradle.kts` around lines 1 - 5, Add a
mavenPublishing configuration that registers this Gradle plugin artifact the
same way the library convention does: call
mavenPublishing.configure(GradlePlugin(...)) for the plugin artifact and set its
JavaLibrary publishing options to JavaLibrary(javadocJar = JavadocJar.Empty())
so the published plugin gets the empty Javadoc jar and consistent POM metadata;
locate the plugins block in clkx-plugin.gradle.kts and add a
mavenPublishing.configure block referencing GradlePlugin and
JavaLibrary/JavadocJar.Empty to mirror the sibling
clkx-library.gradle.kts/clkx-publish.gradle.kts setup.
There was a problem hiding this comment.
__
/ \
( o.o ) Actually — we DO use Vanniktech.
> ^ < You caught one half of a split config.
clkx-plugin.gradle.kts only applies the java-gradle-plugin + com.vanniktech.maven.publish plugin IDs. The mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) } block lives in the sibling clkx-publish.gradle.kts:
// clkx-publish.gradle.kts
import com.vanniktech.maven.publish.GradlePlugin
import com.vanniktech.maven.publish.JavadocJar
plugins {
id("com.vanniktech.maven.publish")
}
mavenPublishing {
configure(GradlePlugin(javadocJar = JavadocJar.Empty()))
}Both are applied together by clkx-conventions, so every plugin module gets the full publishing config:
// clkx-conventions.gradle.kts
plugins {
id("clkx-module")
id("clkx-toolchain")
id("clkx-plugin") // java-gradle-plugin + vanniktech ID
id("clkx-publish") // vanniktech config block
...
}Same result as your diff — just split across two convention plugins on purpose, so we can reuse clkx-publish for non-plugin library artifacts too (see clkx-library.gradle.kts which uses JavaLibrary instead of GradlePlugin).
🐰
There was a problem hiding this comment.
@ClankerGuru, thanks for the clear walkthrough — you're absolutely right, and I apologize for the noise. The split is a clean design: clkx-plugin registers the plugin IDs, clkx-publish owns the mavenPublishing configuration block, and clkx-conventions composes both so every plugin module gets the full setup. The reuse of clkx-publish with JavaLibrary vs GradlePlugin depending on context makes the intent clear. I'll remember this pattern for future reviews.
✏️ Learnings added
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: build-logic/src/main/kotlin/clkx-plugin.gradle.kts:1-5
Timestamp: 2026-04-17T20:11:24.722Z
Learning: In the android-cli Gradle project (build-logic/src/main/kotlin/), the Vanniktech Maven publish configuration is intentionally split across two convention scripts:
- `clkx-plugin.gradle.kts` applies only the `java-gradle-plugin` and `com.vanniktech.maven.publish` plugin IDs.
- `clkx-publish.gradle.kts` owns the `mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) }` block.
Both are composed together by `clkx-conventions.gradle.kts` (which applies `clkx-plugin` and `clkx-publish`), so every plugin module gets the full publishing config. The split is intentional: `clkx-publish` is reused for non-plugin library artifacts as well (via `clkx-library.gradle.kts`, which uses `JavaLibrary` instead of `GradlePlugin`). Do not flag the absence of a `mavenPublishing` block in `clkx-plugin.gradle.kts` as a missing configuration.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| @@ -0,0 +1,7 @@ | |||
| distributionBase=GRADLE_USER_HOME | |||
| distributionPath=wrapper/dists | |||
| distributionUrl=https\://services.gradle.org/distributions/gradle-9.4.1-bin.zip | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
curl -sI https://services.gradle.org/distributions/gradle-9.4.1-bin.zip | head -n 1Repository: ClankerGuru/android-cli
Length of output: 98
🌐 Web query:
ktlint gradle plugin org.jlleitschuh.gradle.ktlint Gradle 9 compatibility
💡 Result:
Yes, the org.jlleitschuh.gradle.ktlint Gradle plugin is compatible with Gradle 9. Use version 14.2.0 (latest as of 2026-03-12) or newer. Key evidence: - Version 13.1.0 (2025-08-21): Added Gradle 9 support (PR #937). - Version 14.0.1 (2025-11-10): Updated build for Gradle 9.1 and Java 25 (PR #962). - Latest versions 14.1.0+ (2026) include further improvements like AGP 9 / built-in Kotlin support and isolated projects compatibility. - Minimum supported Gradle version is 7.4; no upper limit specified, and CI/build explicitly tested/built against Gradle 9.x. - Available on Gradle Plugin Portal: plugins { id("org.jlleitschuh.gradle.ktlint") version "14.2.0" } Note: Some users reported issues with AGP 9.0 + built-in Kotlin (issue #1035, Feb 2026), resolved in 14.1.0+ (#1016).
Citations:
- 1: update build to work with gradle 9.1 and Java 25 JLLeitschuh/ktlint-gradle#962
- 2: Ktlint plugin not detecting lint errors on Gradle 9.0 (with Kotlin-built in) JLLeitschuh/ktlint-gradle#1035
- 3: https://plugins.gradle.org/plugin/org.jlleitschuh.gradle.ktlint
- 4: https://github.com/JLLeitschuh/ktlint-gradle
- 5: https://github.com/JLLeitschuh/ktlint-gradle/releases
- 6: https://github.com/jlleitschuh/ktlint-gradle
- 7: https://github.com/JLLeitschuh/ktlint-gradle/blob/main/CHANGELOG.md
- 8: https://raw.githubusercontent.com/JLLeitschuh/ktlint-gradle/master/CHANGELOG.md
🏁 Script executed:
#!/bin/bash
# Find build.gradle or build.gradle.kts files
find . -maxdepth 3 -type f \( -name "build.gradle" -o -name "build.gradle.kts" \) | head -20Repository: ClankerGuru/android-cli
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Also check if there's a settings.gradle or settings.gradle.kts
find . -maxdepth 3 -type f \( -name "settings.gradle" -o -name "settings.gradle.kts" \)Repository: ClankerGuru/android-cli
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Read the main build.gradle.kts file
wc -l ./plugin/build.gradle.kts ./build-logic/build.gradle.ktsRepository: ClankerGuru/android-cli
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Read plugin/build.gradle.kts to see plugin declarations
cat -n ./plugin/build.gradle.ktsRepository: ClankerGuru/android-cli
Length of output: 544
🏁 Script executed:
#!/bin/bash
# Read build-logic/build.gradle.kts to see the actual plugin declarations
cat -n ./build-logic/build.gradle.ktsRepository: ClankerGuru/android-cli
Length of output: 1002
🏁 Script executed:
#!/bin/bash
# Also check settings.gradle.kts to see pluginManagement configurations
cat -n ./settings.gradle.ktsRepository: ClankerGuru/android-cli
Length of output: 324
🌐 Web query:
com.vanniktech gradle-maven-publish-plugin 0.32.0 Gradle 9 compatibility
💡 Result:
Yes, com.vanniktech gradle-maven-publish-plugin version 0.32.0 is compatible with Gradle 9. The 0.32.0 release (May 2025) explicitly includes "Fix compatibility with Gradle 9.0" and lists compatibility tested up to Gradle 9.0-milestone-6. Minimum supported Gradle version is 8.5. Later versions like 0.36.0 (Jan 2026) raise the minimum to Gradle 9.0.0.
Citations:
- 1: https://github.com/vanniktech/gradle-maven-publish-plugin/releases
- 2: https://vanniktech.github.io/gradle-maven-publish-plugin/changelog/
- 3: https://github.com/vanniktech/gradle-maven-publish-plugin/blob/main/CHANGELOG.md
🌐 Web query:
detekt-gradle-plugin 1.23.8 Gradle 9 compatibility
💡 Result:
No, detekt-gradle-plugin 1.23.8 is not officially compatible with Gradle 9. It was compiled and tested against Gradle 8.12.1. Official docs recommend aligning Gradle versions with those listed in the compatibility table, which shows Gradle 8.x for 1.23.8. While some users run it with Gradle 9.x (e.g., 9.2.1), issues like deprecations exist, and full support requires Detekt 2.0.0-alpha (built against Gradle 9.x). Detekt 1.23.8 docs state it requires Gradle 6.8.3+, but recommend the table version for best support.
Citations:
- 1: https://detekt.dev/docs/1.23.8/gettingstarted/gradle/
- 2: https://github.com/detekt/detekt/releases
- 3: https://detekt.dev/docs/introduction/compatibility/
- 4: https://github.com/detekt/detekt/releases/tag/v1.23.8
- 5: https://detekt.dev/docs/gettingstarted/gradle/
- 6: Update to Gradle 9.3.0 detekt/detekt#8982
- 7: https://detekt.dev/docs/1.23.6/introduction/compatibility/
- 8: https://detekt.dev/docs/1.23.7/introduction/compatibility/
- 9: Detekt Gradle plugin cannot find sources in commonTest detekt/detekt#8961
- 10: Gradle 9.1 (rc1) deprecation of ReportingExtension.file detekt/detekt#8452
🌐 Web query:
ktlint-gradle 12.2.0 Gradle 9 compatibility
💡 Result:
No, ktlint-gradle 12.2.0 is not compatible with Gradle 9. Official documentation states the minimum supported Gradle version is 7.4, but full Gradle 9 support was added in version 13.1.0 (August 2025). Version 12.2.0 (February 2025) updated testing to newer Gradle versions but predates explicit Gradle 9 support. Issues with Gradle 9, such as configuration cache problems and lint detection failures (especially with built-in Kotlin DSL in Gradle 9), exist even in later versions like 14.x. Upgrade to at least 13.1.0 or the latest 14.2.0 for Gradle 9 compatibility.
Citations:
- 1: https://github.com/JLLeitschuh/ktlint-gradle/releases
- 2: https://plugins.gradle.org/plugin/org.jlleitschuh.gradle.ktlint/12.2.0
- 3: https://plugins.gradle.org/plugin/org.jlleitschuh.gradle.ktlint
- 4: https://github.com/JLLeitschuh/ktlint-gradle
- 5: https://github.com/JLLeitschuh/ktlint-gradle/blob/main/CHANGELOG.md
- 6: https://github.com/JLLeitschuh/ktlint-gradle/blob/main/README.md
- 7: Gradle 9 configuration-cache issue JLLeitschuh/ktlint-gradle#936
- 8: Ktlint plugin not detecting lint errors on Gradle 9.0 (with Kotlin-built in) JLLeitschuh/ktlint-gradle#1035
Upgrade ktlint-gradle and detekt to support Gradle 9.4.1.
Gradle 9.4.1 distribution is released and available. However, two plugins in the build are incompatible with Gradle 9:
- ktlint-gradle 12.2.0: Lacks Gradle 9 support (added in 13.1.0, August 2025). Upgrade to 13.1.0 or later (recommend 14.2.0).
- detekt-gradle-plugin 1.23.8: Tested only against Gradle 8.12.1. Gradle 9 support requires Detekt 2.0.0-alpha or later.
The vanniktech maven-publish-plugin (0.32.0) is compatible with Gradle 9. Foojay resolver (1.0.0) is an official Gradle tool.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gradle/wrapper/gradle-wrapper.properties` at line 3, Upgrade the incompatible
Gradle plugins to versions that support Gradle 9: update the ktlint-gradle
plugin (org.jlleitschuh.gradle.ktlint) from 12.2.0 to at least 13.1.0 (recommend
14.2.0) and update the detekt Gradle plugin (io.gitlab.arturbosch.detekt /
detekt-gradle-plugin) from 1.23.8 to a Detekt 2.x release (2.0.0-alpha or
preferably a stable 2.x) in your plugin declarations (build.gradle /
build.gradle.kts or pluginManagement block) and refresh the Gradle wrapper to
use the existing distributionUrl (Gradle 9.4.1) so those plugin versions are
compatible.
There was a problem hiding this comment.
__
(o.o) hop hop hop
o(")(") 9.4.1 green
Builds cleanly on Gradle 9.4.1 locally and in CI — this exact plugin-version matrix (ktlint-gradle 12.2.0, detekt 1.23.8, vanniktech 0.32.0, Foojay 1.0.0) is what powers clkx-agents, wrkx, srcx, and opsx today. No deprecation warnings or test failures that matter for us.
Plugin version bumps are owned by the shared clkx-* convention plugins and will land as a coordinated sweep across all Clanker repos. Not a per-PR bump. 🐰
There was a problem hiding this comment.
@ClankerGuru, understood — thanks for the context! If it's green end-to-end across the whole Clanker fleet at those versions, the concern doesn't apply here. I'll note the governance model for future reviews.
✏️ Learnings added
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: gradle/wrapper/gradle-wrapper.properties:3-3
Timestamp: 2026-04-17T20:11:50.118Z
Learning: In ClankerGuru/android-cli (and across all Clanker repos: clkx-agents, wrkx, srcx, opsx), the plugin-version matrix for the shared `clkx-*` convention plugins — ktlint-gradle 12.2.0 (`org.jlleitschuh.gradle.ktlint`), detekt 1.23.8 (`io.gitlab.arturbosch.detekt`), vanniktech maven-publish 0.32.0, and Foojay resolver 1.0.0 — is intentionally pinned and verified to build cleanly on Gradle 9.4.1 (locally and in CI). Plugin version upgrades are managed as a coordinated sweep across all Clanker repos via the shared `clkx-*` convention plugins, not bumped on a per-PR basis. Do not flag these plugin versions as incompatible with Gradle 9 in any Clanker repo review.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: build-logic/src/main/kotlin/clkx-plugin.gradle.kts:1-5
Timestamp: 2026-04-17T20:11:24.722Z
Learning: In the android-cli Gradle project (build-logic/src/main/kotlin/), the Vanniktech Maven publish configuration is intentionally split across two convention scripts:
- `clkx-plugin.gradle.kts` applies only the `java-gradle-plugin` and `com.vanniktech.maven.publish` plugin IDs.
- `clkx-publish.gradle.kts` owns the `mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) }` block.
Both are composed together by `clkx-conventions.gradle.kts` (which applies `clkx-plugin` and `clkx-publish`), so every plugin module gets the full publishing config. The split is intentional: `clkx-publish` is reused for non-plugin library artifacts as well (via `clkx-library.gradle.kts`, which uses `JavaLibrary` instead of `GradlePlugin`). Do not flag the absence of a `mavenPublishing` block in `clkx-plugin.gradle.kts` as a missing configuration.
| internal fun checkBinaryInstalled() { | ||
| val check = ProcessBuilder("which", extension.binary).redirectErrorStream(true).start() | ||
| check.waitFor() | ||
| if (check.exitValue() != 0) { | ||
| error("'${extension.binary}' not found on PATH. Install Google's Android CLI first.") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is Google's "android" agent CLI (developer.android.com/tools/agents/android-cli) distributed for Windows, or Unix-only?
💡 Result:
Google's Android CLI agent (at developer.android.com/tools/agents/android-cli) is not Unix-only; it is distributed for Windows as well, though the android emulator command is currently disabled on Windows. Official documentation confirms Windows support with a known issue for emulator functionality, and installation instructions like which android or command -v android imply Unix-like environments (including WSL on Windows), but the explicit mention of Windows-specific issues indicates a Windows binary or compatibility exists.
Citations:
🏁 Script executed:
# First, find the AndroidExecTask.kt file
find . -name "AndroidExecTask.kt" -type fRepository: ClankerGuru/android-cli
Length of output: 146
🏁 Script executed:
# Once found, examine the file to verify the code at lines 17-23
head -30 ./plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidExecTask.kt | tail -20Repository: ClankerGuru/android-cli
Length of output: 699
🏁 Script executed:
# Search for test files that might reference line 61 or the android path
find . -path "*/test*" -name "*.kt" | head -10Repository: ClankerGuru/android-cli
Length of output: 222
🏁 Script executed:
# Check AndroidCliPluginTest.kt for line 61 and absolute path usage
wc -l ./plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidCliPluginTest.ktRepository: ClankerGuru/android-cli
Length of output: 150
🏁 Script executed:
# Check both test files for absolute path patterns
rg "opt/android|extension\.binary|checkBinaryInstalled" ./plugin/src/test/kotlin/zone/clanker/gradle/androidcli/Repository: ClankerGuru/android-cli
Length of output: 781
🏁 Script executed:
# Check if extension.binary is set with absolute paths or relative paths elsewhere
rg "binary\s*=" ./plugin/src/main/kotlin/ -A 2Repository: ClankerGuru/android-cli
Length of output: 49
🏁 Script executed:
# Check the test context around the absolute path setting
rg -B 5 -A 5 "/opt/android/bin/android" ./plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidCliPluginTest.ktRepository: ClankerGuru/android-cli
Length of output: 515
🏁 Script executed:
# Check AndroidTasksTest.kt to see test cases for checkBinaryInstalled
cat -n ./plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt | grep -A 20 "checkBinaryInstalled"Repository: ClankerGuru/android-cli
Length of output: 1235
🏁 Script executed:
# Check the AndroidCli.SettingsExtension to understand extension.binary
rg "class.*SettingsExtension|var binary|val binary" ./plugin/src/main/kotlin/ -B 2 -A 2Repository: ClankerGuru/android-cli
Length of output: 672
which is not portable — breaks the plugin on Windows.
ProcessBuilder("which", ...) will fail with IOException: Cannot run program "which" on Windows agents because which isn't a builtin or on PATH; Windows uses where.exe. Since this is a Gradle plugin published to Maven Central, Windows consumers will hit this on every task invocation. Official documentation confirms the Android CLI agent is distributed for Windows, making this a critical compatibility gap.
Additionally, extension.binary can be set to an absolute path (tests show /opt/android/bin/android), but which only works with command names, not paths. The check will fail for absolute paths even on Unix.
Suggested approach: use java.nio.file.Path / File.canExecute() for absolute paths or paths with separators, otherwise probe each entry on PATH (or use where on Windows). A minimal cross-platform fix:
🛠️ Proposed fix
- internal fun checkBinaryInstalled() {
- val check = ProcessBuilder("which", extension.binary).redirectErrorStream(true).start()
- check.waitFor()
- if (check.exitValue() != 0) {
- error("'${extension.binary}' not found on PATH. Install Google's Android CLI first.")
- }
- }
+ internal fun checkBinaryInstalled() {
+ val binary = extension.binary
+ val asFile = java.io.File(binary)
+ val found = if (asFile.isAbsolute || binary.contains(java.io.File.separatorChar)) {
+ asFile.canExecute()
+ } else {
+ val pathEnv = System.getenv("PATH").orEmpty()
+ val exts = if (org.gradle.internal.os.OperatingSystem.current().isWindows) {
+ (System.getenv("PATHEXT") ?: ".EXE;.BAT;.CMD").split(';')
+ } else {
+ listOf("")
+ }
+ pathEnv.split(java.io.File.pathSeparatorChar).any { dir ->
+ exts.any { ext -> java.io.File(dir, binary + ext).canExecute() }
+ }
+ }
+ if (!found) {
+ error("'$binary' not found on PATH. Install Google's Android CLI first.")
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidExecTask.kt`
around lines 17 - 23, checkBinaryInstalled() currently invokes "which" which is
not portable and fails on Windows and when extension.binary is an
absolute/path-containing value; change the logic to first treat extension.binary
as a Path and if it contains a file separator or is absolute use
java.nio.file.Path/Files.exists + Files.isExecutable (or File.canExecute()) to
validate it, otherwise perform a PATH search by splitting System.getenv("PATH")
and checking each entry for an executable named extension.binary (or, on
Windows, prefer calling "where" only as a fallback), and throw the same error
when no executable is found; update the method to reference extension.binary and
checkBinaryInstalled() accordingly.
There was a problem hiding this comment.
__
(-.-)
/ \ macOS + Linux only
( \"/ ) (like the CLI itself)
\___/
Intentional scope limitation. Upstream Google's agent-first android CLI is distributed primarily for macOS/Linux (the emulator subcommand is explicitly disabled on Windows per the upstream docs you linked), and every known consumer of this plugin runs in a Clanker workspace on macOS or Linux CI.
We'd rather fail with a readable "not found on PATH" message than invent cross-platform probe logic for a platform the underlying CLI doesn't fully support. If/when upstream ships first-class Windows support we'll revisit — likely pushing the PATH probe into a shared utility in build-logic so all Clanker CLI-wrapping plugins benefit at once.
Known tradeoff, documented. 🐰
There was a problem hiding this comment.
@ClankerGuru — fair point, and that's a clean rationale. 🐧 The macOS/Linux scope matches upstream, the error message is readable enough for any accidental Windows attempt, and deferring the PATH-probe utility to a coordinated build-logic sweep (if/when Google ships full Windows support) is the right call. Standing down on this one.
✏️ Learnings added
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidExecTask.kt:17-23
Timestamp: 2026-04-17T20:12:01.148Z
Learning: In the android-cli plugin (`plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidExecTask.kt`), `checkBinaryInstalled()` intentionally uses `ProcessBuilder("which", ...)` because the plugin targets macOS and Linux only — mirroring the upstream Google `android` agent CLI, which does not have full Windows support (e.g., the `emulator` subcommand is explicitly disabled on Windows). Do not flag the use of `which` as a portability issue for this plugin. If/when upstream ships first-class Windows support, the plan is to move a cross-platform PATH-probe utility into `build-logic` so all Clanker CLI-wrapping plugins benefit at once.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: .github/workflows/release.yml:20-25
Timestamp: 2026-04-17T20:11:42.419Z
Learning: In the ClankerGuru family of repos (android-cli, clkx-agents, wrkx, srcx, opsx), direct interpolation of `${{ github.event.release.tag_name }}` in `release.yml` run steps is intentional and consistent across all sibling repos. The pattern is considered acceptable because the `release: published` event requires repository write access to trigger (low blast radius). Any hardening (e.g., moving to an env var) will be done as a coordinated sweep across all repos, not one-off. Do not flag this pattern in individual PR reviews for these repos.
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: build-logic/src/main/kotlin/clkx-plugin.gradle.kts:1-5
Timestamp: 2026-04-17T20:11:24.722Z
Learning: In the android-cli Gradle project (build-logic/src/main/kotlin/), the Vanniktech Maven publish configuration is intentionally split across two convention scripts:
- `clkx-plugin.gradle.kts` applies only the `java-gradle-plugin` and `com.vanniktech.maven.publish` plugin IDs.
- `clkx-publish.gradle.kts` owns the `mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) }` block.
Both are composed together by `clkx-conventions.gradle.kts` (which applies `clkx-plugin` and `clkx-publish`), so every plugin module gets the full publishing config. The split is intentional: `clkx-publish` is reused for non-plugin library artifacts as well (via `clkx-library.gradle.kts`, which uses `JavaLibrary` instead of `GradlePlugin`). Do not flag the absence of a `mavenPublishing` block in `clkx-plugin.gradle.kts` as a missing configuration.
| internal fun buildArgs(): List<String> { | ||
| val raw = project.findProperty(AndroidCli.PROP_ARGS)?.toString().orEmpty() | ||
| val parsed = raw.split(' ').filter { it.isNotBlank() } | ||
| return parsed + extension.extraArgs | ||
| } |
There was a problem hiding this comment.
Naive tokenization of PROP_ARGS breaks quoted arguments.
raw.split(' ') splits on every single space, so users can't pass arguments containing spaces even with quotes, e.g. -Pzone.clanker.android-cli.args="run --message 'hello world'" becomes ["run", "--message", "'hello", "world'"]. It also doesn't collapse runs of whitespace (tabs, multiple spaces) — split(Regex("\\s+")) would be slightly better, but for shell-like parsing you really want a tokenizer.
Recommend either:
- Document that
PROP_ARGSis whitespace-split and doesn't support quoting, and rely onextension.extraArgs(typedList<String>) for anything non-trivial; or - Use a small shell-lexer (e.g. Apache Commons Exec's
CommandLine.parse, or a minimal hand-rolled one) to respect quotes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidRunTask.kt`
around lines 12 - 16, The buildArgs() function naively splits
AndroidCli.PROP_ARGS with raw.split(' '), which breaks quoted arguments and
doesn't collapse whitespace; replace this with a shell-like tokenizer so quoted
strings are preserved (e.g. use a small lexer such as Apache Commons Exec
CommandLine.parse or implement a minimal parser that splits on runs of
whitespace and respects single/double quotes), keep appending
extension.extraArgs as before, and ensure empty input still yields an empty list
rather than spurious tokens.
There was a problem hiding this comment.
__
(o_o)
/ \ extraArgs for real work
\_____/ PROP_ARGS for quickies
Intentional. PROP_ARGS is a shell-convenience knob for one-shot invocations like -Pzone.clanker.android-cli.args="version --json" where the user is typing at a terminal. Anything that needs quoting should go through extension.extraArgs in settings.gradle.kts, which is already typed List<String> and sidesteps the parsing question entirely:
androidCli {
extraArgs = listOf("run", "--message", "hello world")
}Adding a shell lexer would trade one class of bugs (no quoting) for a different class (surprises in how we tokenize vs sh vs bash). Whitespace-split + typed extraArgs escape hatch is the right tradeoff for us.
Will add a README note making the tokenization rule explicit. 🐰
There was a problem hiding this comment.
@ClankerGuru — the split-then-extraArgs two-tier design makes sense. Simple terminal convenience via -P + a fully typed escape hatch in settings.gradle.kts is a clean tradeoff, and avoiding a shell lexer removes an entire class of platform-specific parsing surprises. Noted — thanks for the clear rationale and the ASCII rabbit. 🐇
✏️ Learnings added
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: plugin/src/main/kotlin/zone/clanker/gradle/androidcli/task/AndroidRunTask.kt:12-16
Timestamp: 2026-04-17T20:12:13.246Z
Learning: In `ClankerGuru/android-cli`, `AndroidRunTask.buildArgs()` intentionally uses `raw.split(' ')` for the `PROP_ARGS` Gradle property. `PROP_ARGS` is a shell-convenience knob for simple one-shot terminal invocations (e.g. `-Pzone.clanker.android-cli.args="version --json"`). Arguments that require quoting or contain spaces must use `extension.extraArgs: List<String>` in `settings.gradle.kts` instead. A shell lexer was deliberately rejected to avoid platform-specific tokenization surprises. Do not flag this split as a bug.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: .github/workflows/release.yml:20-25
Timestamp: 2026-04-17T20:11:42.419Z
Learning: In the ClankerGuru family of repos (android-cli, clkx-agents, wrkx, srcx, opsx), direct interpolation of `${{ github.event.release.tag_name }}` in `release.yml` run steps is intentional and consistent across all sibling repos. The pattern is considered acceptable because the `release: published` event requires repository write access to trigger (low blast radius). Any hardening (e.g., moving to an env var) will be done as a coordinated sweep across all repos, not one-off. Do not flag this pattern in individual PR reviews for these repos.
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: gradle/wrapper/gradle-wrapper.properties:3-3
Timestamp: 2026-04-17T20:11:50.118Z
Learning: In ClankerGuru/android-cli (and across all Clanker repos: clkx-agents, wrkx, srcx, opsx), the plugin-version matrix for the shared `clkx-*` convention plugins — ktlint-gradle 12.2.0 (`org.jlleitschuh.gradle.ktlint`), detekt 1.23.8 (`io.gitlab.arturbosch.detekt`), vanniktech maven-publish 0.32.0, and Foojay resolver 1.0.0 — is intentionally pinned and verified to build cleanly on Gradle 9.4.1 (locally and in CI). Plugin version upgrades are managed as a coordinated sweep across all Clanker repos via the shared `clkx-*` convention plugins, not bumped on a per-PR basis. Do not flag these plugin versions as incompatible with Gradle 9 in any Clanker repo review.
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: build-logic/src/main/kotlin/clkx-plugin.gradle.kts:1-5
Timestamp: 2026-04-17T20:11:24.722Z
Learning: In the android-cli Gradle project (build-logic/src/main/kotlin/), the Vanniktech Maven publish configuration is intentionally split across two convention scripts:
- `clkx-plugin.gradle.kts` applies only the `java-gradle-plugin` and `com.vanniktech.maven.publish` plugin IDs.
- `clkx-publish.gradle.kts` owns the `mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) }` block.
Both are composed together by `clkx-conventions.gradle.kts` (which applies `clkx-plugin` and `clkx-publish`), so every plugin module gets the full publishing config. The split is intentional: `clkx-publish` is reused for non-plugin library artifacts as well (via `clkx-library.gradle.kts`, which uses `JavaLibrary` instead of `GradlePlugin`). Do not flag the absence of a `mavenPublishing` block in `clkx-plugin.gradle.kts` as a missing configuration.
| given("AndroidExecTask.checkBinaryInstalled") { | ||
| `when`("binary exists on PATH") { | ||
| val (project, ext) = newProject() | ||
| ext.binary = "which" | ||
| val task = project.tasks.getByName(AndroidCli.TASK_VERSION) as AndroidVersionTask | ||
|
|
||
| then("it does not throw") { | ||
| task.checkBinaryInstalled() | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor: test depends on which being on $PATH.
ext.binary = "which" causes the production code to invoke ProcessBuilder("which", "which"). This is fine on standard Linux/macOS runners but would fail in a stripped container without which (e.g., distroless/Alpine without busybox-which). Acceptable for the current GitHub Actions setup; flagging in case the CI base image changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt`
around lines 200 - 209, The test currently relies on an external "which" binary;
instead create a temporary executable in the test and prepend its directory to
PATH so the test does not depend on system image. In the
AndroidExecTask.checkBinaryInstalled test (where ext.binary is set and
AndroidCli.TASK_VERSION/AndroidVersionTask used), create a temp file executable
(a small shell script or platform-appropriate no-op), mark it executable, add
its parent directory to the PATH environment for the test process, set
ext.binary to the temp filename, then call task.checkBinaryInstalled(); finally
clean up the temp file and restore PATH.
There was a problem hiding this comment.
__
(◕‿◕)
o(")(") ubuntu + macOS have which
Acknowledged — accepted tradeoff. ubuntu-latest and macos-latest GitHub runners both ship which, and that's the only environment this test runs in. If we ever move to a distroless/stripped base image we'll swap in a temp-executable-on-PATH approach as you suggest.
Parking as known. 🐰
There was a problem hiding this comment.
@ClankerGuru — fair enough, noted! ubuntu-latest and macos-latest both carry which, so the current setup is solid. I'll remember the agreed-upon migration path (temp-executable-on-PATH) for if/when the CI base image changes. 🐰
✏️ Learnings added
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt:200-209
Timestamp: 2026-04-17T20:12:19.277Z
Learning: In ClankerGuru/android-cli, the `AndroidExecTask.checkBinaryInstalled` test in `plugin/src/test/kotlin/zone/clanker/gradle/androidcli/AndroidTasksTest.kt` (around lines 200-209) intentionally uses `ext.binary = "which"` (invoking `ProcessBuilder("which", "which")`) as a binary-on-PATH probe. This dependency on the `which` binary is an accepted tradeoff: CI only runs on `ubuntu-latest` and `macos-latest` GitHub runners, both of which ship `which`. If the CI base image ever changes to a distroless/stripped image (e.g., Alpine without busybox-which), the agreed migration is to create a temp executable in the test and prepend its parent directory to `PATH`. Do not flag this pattern as a test portability issue in reviews for this repo unless the CI base image changes.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: .github/workflows/release.yml:20-25
Timestamp: 2026-04-17T20:11:42.419Z
Learning: In the ClankerGuru family of repos (android-cli, clkx-agents, wrkx, srcx, opsx), direct interpolation of `${{ github.event.release.tag_name }}` in `release.yml` run steps is intentional and consistent across all sibling repos. The pattern is considered acceptable because the `release: published` event requires repository write access to trigger (low blast radius). Any hardening (e.g., moving to an env var) will be done as a coordinated sweep across all repos, not one-off. Do not flag this pattern in individual PR reviews for these repos.
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: gradle/wrapper/gradle-wrapper.properties:3-3
Timestamp: 2026-04-17T20:11:50.118Z
Learning: In ClankerGuru/android-cli (and across all Clanker repos: clkx-agents, wrkx, srcx, opsx), the plugin-version matrix for the shared `clkx-*` convention plugins — ktlint-gradle 12.2.0 (`org.jlleitschuh.gradle.ktlint`), detekt 1.23.8 (`io.gitlab.arturbosch.detekt`), vanniktech maven-publish 0.32.0, and Foojay resolver 1.0.0 — is intentionally pinned and verified to build cleanly on Gradle 9.4.1 (locally and in CI). Plugin version upgrades are managed as a coordinated sweep across all Clanker repos via the shared `clkx-*` convention plugins, not bumped on a per-PR basis. Do not flag these plugin versions as incompatible with Gradle 9 in any Clanker repo review.
Learnt from: ClankerGuru
Repo: ClankerGuru/android-cli PR: 1
File: build-logic/src/main/kotlin/clkx-plugin.gradle.kts:1-5
Timestamp: 2026-04-17T20:11:24.722Z
Learning: In the android-cli Gradle project (build-logic/src/main/kotlin/), the Vanniktech Maven publish configuration is intentionally split across two convention scripts:
- `clkx-plugin.gradle.kts` applies only the `java-gradle-plugin` and `com.vanniktech.maven.publish` plugin IDs.
- `clkx-publish.gradle.kts` owns the `mavenPublishing { configure(GradlePlugin(javadocJar = JavadocJar.Empty())) }` block.
Both are composed together by `clkx-conventions.gradle.kts` (which applies `clkx-plugin` and `clkx-publish`), so every plugin module gets the full publishing config. The split is intentional: `clkx-publish` is reused for non-plugin library artifacts as well (via `clkx-library.gradle.kts`, which uses `JavaLibrary` instead of `GradlePlugin`). Do not flag the absence of a `mavenPublishing` block in `clkx-plugin.gradle.kts` as a missing configuration.
- config/detekt.yml: fix over-escaped ForbiddenComment allowedPatterns regex
(YAML single-quotes don't process escapes; 'TODO\\(.*\\):' was matching
literal backslashes instead of parentheses)
- AndroidCliPluginTest: construct fresh SettingsExtension in each when{} scope
so defaults and mutation leaves don't share state
1 participant
Summary
Initial scaffold for
zone.clanker.gradle.android-cli— a Gradle settings plugin wrapping Google's agent-firstandroidCLI.zone.clanker.gradle.android-clizone.clanker:android-cliStructure
clkx convention plugins (conventions / library / module / publish / ktlint / detekt / konsist / cover / toolchain), same as clkx-agents.
Exec-based tasks grouped by CLI subcommand:
Quality gates
println,System.out/err, wildcard imports in mainCI
build.yml— runs./gradlew buildon push/PR to mainrelease.yml— publishes to Maven Central on GitHub release (Vanniktech, CENTRAL_PORTAL)Test plan
./gradlew :plugin:publishToMavenLocal./gradlew tasks --group android-cliSummary by CodeRabbit
New Features
Chores