Skip to content

ADFA-2849: do not overwrite repositories in Gradle Plugin#995

Merged
itsaky-adfa merged 8 commits intostagefrom
fix/ADFA-2849
Mar 2, 2026
Merged

ADFA-2849: do not overwrite repositories in Gradle Plugin#995
itsaky-adfa merged 8 commits intostagefrom
fix/ADFA-2849

Conversation

@itsaky-adfa
Copy link
Contributor

@itsaky-adfa itsaky-adfa commented Feb 19, 2026

See ADFA-2849 for more details.

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa requested a review from a team February 19, 2026 11:42
@itsaky-adfa itsaky-adfa self-assigned this Feb 19, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new Settings plugin (COTGSettingsPlugin) and refactors AndroidIDEInitScriptPlugin to register that Settings plugin before settings evaluation, simplify repository handling, keep daemon-log cleanup, and apply the cogo-plugin classpath and plugin to eligible subprojects.

Changes

Cohort / File(s) Summary
Init script plugin (refactor)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt
Reworked apply flow: register a beforeSettings hook that applies COTGSettingsPlugin, retain root-project classpath wiring for the cogo-plugin, perform daemon log cleanup, and apply the plugin to evaluated subprojects. Removed prior test-env and multi-variant repo logic and related imports.
New settings plugin (local Maven repos)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt
Added COTGSettingsPlugin that registers MAVEN_LOCAL_REPOSITORY, reads StartParameter/project properties (_PROPERTY_IS_TEST_ENV, _PROPERTY_MAVEN_LOCAL_REPOSITORY), resolves and validates local repo paths, and idempotently adds them to dependencyResolutionManagement and pluginManagement with logging and safe-add helpers.

Sequence Diagram(s)

sequenceDiagram
    participant Init as InitScriptPlugin
    participant Settings as GradleSettings
    participant COTG as COTGSettingsPlugin
    participant Repo as RepositoryManagement
    participant Sub as Subprojects

    Init->>Settings: register beforeSettings hook
    Note right of Settings: beforeSettings hook executes during settings evaluation
    Settings->>COTG: apply COTGSettingsPlugin
    COTG->>Repo: read StartParameter / project properties
    Repo->>Repo: ensure MAVEN_LOCAL_REPOSITORY present (idempotent)
    alt test env detected
        COTG->>Repo: resolve additional local paths -> add repos (idempotent)
    end
    Init->>Init: add root project classpath for cogo-plugin
    Init->>Sub: afterEvaluate -> apply cogo-plugin to eligible subprojects
    Init->>Init: remove daemon logs
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped into settings with a tiny, bright tap,
I placed local jars gently along the map.
Hooks whisper, repos align, subprojects start to sing,
Daemon logs swept clean—now watch the builds take wing.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing repository overwriting in the Gradle plugin, which aligns with the refactoring to use a separate Settings plugin.
Description check ✅ Passed The description references the issue ticket ADFA-2849 and aligns with the PR objective of preventing repository overwriting in Gradle plugin configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ADFA-2849

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt (2)

170-175: Consider using GradleException / InvalidUserDataException instead of require.

require(dir.isDirectory) throws kotlin.IllegalArgumentException. In Gradle plugin code, using GradleException (or InvalidUserDataException) produces a cleaner "BUILD FAILED with an exception" summary in build output rather than an undecorated stack trace. The message could also distinguish the two failure modes.

♻️ Proposed refactor
 val dir = File(path)
-require(dir.isDirectory) { "Repo not found: $path" }
+if (!dir.exists()) throw InvalidUserDataException("Maven local repo path does not exist: $path")
+if (!dir.isDirectory) throw InvalidUserDataException("Maven local repo path is not a directory: $path")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
around lines 170 - 175, Replace the use of Kotlin's require in
addLocalMavenRepoIfMissing with a Gradle-specific exception to produce proper
"BUILD FAILED" output: check File(path).exists() and File(path).isDirectory()
and throw either org.gradle.api.InvalidUserDataException (for missing path) or
org.gradle.api.GradleException (for not a directory) with clear, distinct
messages; update the function (addLocalMavenRepoIfMissing) to import and throw
these exceptions instead of calling require so Gradle surfaces the error
correctly.

142-142: Nit: .toList() is redundant; consider documenting the POSIX-only path separator assumption.

String.split(delimiter: Char) already returns List<String>, so the chained .toList() is a no-op.

Additionally, splitting on : works correctly on POSIX systems but would corrupt Windows-style paths (e.g., C:\repo splits into ["C", "\\repo"]). Since this only executes in test/CI environments on Linux/Android, risk is low, but a brief comment would make the constraint explicit.

♻️ Proposed fix
-isTestEnv to mavenLocalRepos.split(':').toList().filter { it.isNotBlank() }
+// NOTE: ':' is the POSIX multi-path separator; Windows paths are not supported here.
+isTestEnv to mavenLocalRepos.split(':').filter { it.isNotBlank() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
at line 142, The expression mavenLocalRepos.split(':').toList().filter {
it.isNotBlank() } contains a redundant .toList() and should be simplified to
mavenLocalRepos.split(':').filter { it.isNotBlank() }; remove the .toList() from
the mapping (the code around the isTestEnv mapping where mavenLocalRepos is
split) and add a short comment above that split noting the POSIX-only assumption
(splitting on ':' will break Windows paths like C:\repo, but this runs only in
test/CI on Linux/Android).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`:
- Around line 170-175: Replace the use of Kotlin's require in
addLocalMavenRepoIfMissing with a Gradle-specific exception to produce proper
"BUILD FAILED" output: check File(path).exists() and File(path).isDirectory()
and throw either org.gradle.api.InvalidUserDataException (for missing path) or
org.gradle.api.GradleException (for not a directory) with clear, distinct
messages; update the function (addLocalMavenRepoIfMissing) to import and throw
these exceptions instead of calling require so Gradle surfaces the error
correctly.
- Line 142: The expression mavenLocalRepos.split(':').toList().filter {
it.isNotBlank() } contains a redundant .toList() and should be simplified to
mavenLocalRepos.split(':').filter { it.isNotBlank() }; remove the .toList() from
the mapping (the code around the isTestEnv mapping where mavenLocalRepos is
split) and add a short comment above that split noting the POSIX-only assumption
(splitting on ':' will break Windows paths like C:\repo, but this runs only in
test/CI on Linux/Android).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt (2)

149-149: Redundant .toList() call — String.split() already returns List<String>

String.split(Char) in Kotlin's stdlib returns List<String> directly; chaining .toList() allocates a second list for no benefit.

♻️ Proposed fix
-    isTestEnv to mavenLocalRepos.split(':').toList().filter { it.isNotBlank() }
+    isTestEnv to mavenLocalRepos.split(':').filter { it.isNotBlank() }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
at line 149, The mapping that builds the value for isTestEnv uses
mavenLocalRepos.split(':').toList().filter { it.isNotBlank() } which
unnecessarily calls .toList() because String.split(':') already returns a
List<String>; remove the redundant .toList() so the expression becomes
mavenLocalRepos.split(':').filter { it.isNotBlank() } (locate this in
AndroidIDEInitScriptPlugin.kt where mavenLocalRepos is used to construct
isTestEnv).

165-172: URI equality check may miss duplicates if external scripts register repos with non-normalized paths

File.toURI() appends a trailing slash for directories (e.g., file:///path/to/repo/). URI.equals() performs literal string comparison without normalization, so the same physical directory registered via a differently-formatted URI (missing trailing slash, containing .. segments, using a symlink path, or differing in case on case-insensitive filesystems) will not be detected as a duplicate.

Within this plugin, the risk is minimal since all repository additions via addLocalMavenRepoIfMissing() consistently use File.toURI(). The concern applies primarily to repositories added externally through a user's own settings script, where URIs may not be normalized. For greater robustness, consider comparing canonical or normalized URIs instead of relying on literal equality.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
around lines 165 - 172, The current literal URI equality in
addMavenRepoIfMissing can miss duplicates due to non-normalized or
filesystem-varying URIs; change the comparison to normalize/resolve both the
candidate uri and existing repository URLs before comparing: for file:// URIs
convert to File and use canonicalFile (or toRealPath) and compare their
canonical file URIs, for non-file URIs use uri.normalize() and trim any trailing
slash before comparing; update the none { it is MavenArtifactRepository &&
it.url == uri } check to perform this normalized/ canonicalized comparison
against MavenArtifactRepository.url so duplicate repos added with different
string forms are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`:
- Around line 157-158: The require check in AndroidIDEInitScriptPlugin.kt using
dir.isDirectory() has a misleading message for when the path exists but is a
file; update the validation around the val dir = File(path) and require to
differentiate cases: if dir.exists() and !dir.isDirectory() then throw/require
with a message like "Expected directory but found file: $path", if !dir.exists()
then keep "Repo not found: $path"; ensure you reference the existing val dir and
path variable and replace the single require(dir.isDirectory) call with explicit
conditional checks that produce the correct, distinct error messages.

---

Nitpick comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`:
- Line 149: The mapping that builds the value for isTestEnv uses
mavenLocalRepos.split(':').toList().filter { it.isNotBlank() } which
unnecessarily calls .toList() because String.split(':') already returns a
List<String>; remove the redundant .toList() so the expression becomes
mavenLocalRepos.split(':').filter { it.isNotBlank() } (locate this in
AndroidIDEInitScriptPlugin.kt where mavenLocalRepos is used to construct
isTestEnv).
- Around line 165-172: The current literal URI equality in addMavenRepoIfMissing
can miss duplicates due to non-normalized or filesystem-varying URIs; change the
comparison to normalize/resolve both the candidate uri and existing repository
URLs before comparing: for file:// URIs convert to File and use canonicalFile
(or toRealPath) and compare their canonical file URIs, for non-file URIs use
uri.normalize() and trim any trailing slash before comparing; update the none {
it is MavenArtifactRepository && it.url == uri } check to perform this
normalized/ canonicalized comparison against MavenArtifactRepository.url so
duplicate repos added with different string forms are detected.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt (1)

60-70: Misleading error message when path exists but is not a directory.

File.isDirectory() returns false both when the path doesn't exist and when it's a regular file, but the error message always says "Repo not found." Consider splitting into two checks for clearer diagnostics.

🛡️ Proposed fix
-  require(dir.isDirectory) { "Repo not found: $path" }
+  require(dir.exists()) { "Repo not found: $path" }
+  require(dir.isDirectory) { "Repo path is not a directory: $path" }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`
around lines 60 - 70, In addLocalMavenRepoIfMissing(logger, path) the single
require(dir.isDirectory) message "Repo not found" is misleading for the case
where the path exists but is a file; change the checks to first verify
dir.exists() and throw/require with "Repo not found: {path}" if missing, then
check dir.isDirectory() and throw/require with a clearer message like "Path
exists but is not a directory: {path}" if not a directory, keeping the
subsequent addMavenRepoIfMissing(logger, uri) call unchanged.
🧹 Nitpick comments (3)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt (1)

48-57: Minor: mavenLocalRepos is always computed even when isTestEnv is false.

This is harmless since the result is discarded, but you could short-circuit by returning early when isTestEnv is false to avoid the unnecessary string split. Not a functional issue.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`
around lines 48 - 57, getTestEnvProps currently computes mavenLocalRepos even
when isTestEnv is false; modify getTestEnvProps to check isTestEnv first and
return false with empty list immediately if not set, otherwise read and split
the _PROPERTY_MAVEN_LOCAL_REPOSITORY value; update the logic inside
getTestEnvProps (referencing isTestEnv, mavenLocalRepos, and the function name
getTestEnvProps) to short-circuit and avoid calling split when unnecessary.
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt (2)

86-94: logFile.delete() return value is silently ignored.

File.delete() returns false on failure. While this is acceptable for best-effort log cleanup, logging the failure would aid debugging in environments with restrictive file permissions.

Proposed improvement
 				?.forEach { logFile ->
-					logger.lifecycle("deleting log: ${logFile.name}")
-					logFile.delete()
+					val deleted = logFile.delete()
+					if (deleted) {
+						logger.lifecycle("Deleted log: ${logFile.name}")
+					} else {
+						logger.warn("Failed to delete log: ${logFile.name}")
+					}
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
around lines 86 - 94, The cleanup loop in AndroidIDEInitScriptPlugin using
logsDir...drop(MAX_LOGFILE_COUNT).forEach currently calls logFile.delete() and
ignores its boolean result; update the loop (referencing logsDir,
MAX_LOGFILE_COUNT, the forEach block and logFile.delete()) to check the return
value and log a warning or error via logger (e.g., logger.warn or
logger.lifecycle) when delete() returns false, including the logFile.name and
reason context so failures due to permissions are visible; keep the operation
best-effort (do not throw), just log failures for debugging.

82-100: Else-branch log message is slightly misleading when the directory doesn't exist.

When logsDir doesn't exist at all (e.g., fresh Gradle install or mismatched version), the message says "number of log files does not exceed…" which implies the directory was checked for file count. Consider differentiating the two cases for easier troubleshooting.

Proposed improvement
 		} else {
 			logger.lifecycle(
-				"No deletions made, number of log files does not" +
-					" exceed ($MAX_LOGFILE_COUNT) for gradle ($currentGradleVersion).",
+				"No deletions made. Logs directory does not exist or is not" +
+					" a directory: ${logsDir.absolutePath}",
 			)
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
around lines 82 - 100, The else-branch message is ambiguous when logsDir doesn't
exist; update the conditional to distinguish the two cases: check
logsDir.exists() and logsDir.isDirectory separately and log one message when the
directory is missing or not a directory (mention logsDir path and
currentGradleVersion) and a different message when the directory exists but file
count does not exceed MAX_LOGFILE_COUNT; use the existing logsDir,
MAX_LOGFILE_COUNT, currentGradleVersion and logger.lifecycle calls to report the
appropriate, clear status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`:
- Around line 60-70: In addLocalMavenRepoIfMissing(logger, path) the single
require(dir.isDirectory) message "Repo not found" is misleading for the case
where the path exists but is a file; change the checks to first verify
dir.exists() and throw/require with "Repo not found: {path}" if missing, then
check dir.isDirectory() and throw/require with a clearer message like "Path
exists but is not a directory: {path}" if not a directory, keeping the
subsequent addMavenRepoIfMissing(logger, uri) call unchanged.

---

Nitpick comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`:
- Around line 86-94: The cleanup loop in AndroidIDEInitScriptPlugin using
logsDir...drop(MAX_LOGFILE_COUNT).forEach currently calls logFile.delete() and
ignores its boolean result; update the loop (referencing logsDir,
MAX_LOGFILE_COUNT, the forEach block and logFile.delete()) to check the return
value and log a warning or error via logger (e.g., logger.warn or
logger.lifecycle) when delete() returns false, including the logFile.name and
reason context so failures due to permissions are visible; keep the operation
best-effort (do not throw), just log failures for debugging.
- Around line 82-100: The else-branch message is ambiguous when logsDir doesn't
exist; update the conditional to distinguish the two cases: check
logsDir.exists() and logsDir.isDirectory separately and log one message when the
directory is missing or not a directory (mention logsDir path and
currentGradleVersion) and a different message when the directory exists but file
count does not exceed MAX_LOGFILE_COUNT; use the existing logsDir,
MAX_LOGFILE_COUNT, currentGradleVersion and logger.lifecycle calls to report the
appropriate, clear status.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`:
- Around line 48-57: getTestEnvProps currently computes mavenLocalRepos even
when isTestEnv is false; modify getTestEnvProps to check isTestEnv first and
return false with empty list immediately if not set, otherwise read and split
the _PROPERTY_MAVEN_LOCAL_REPOSITORY value; update the logic inside
getTestEnvProps (referencing isTestEnv, mavenLocalRepos, and the function name
getTestEnvProps) to short-circuit and avoid calling split when unnecessary.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d11bfb2 and 8d87843.

📒 Files selected for processing (2)
  • gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt
  • gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt

@jatezzz jatezzz self-requested a review February 24, 2026 13:49
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt (1)

93-93: Consider checking the result of delete().

File.delete() returns false if deletion fails (e.g., due to permissions or file in use), but the result is currently ignored. For cleanup operations this may be acceptable, but logging failures could help with troubleshooting.

💡 Optional: Log deletion failures
 				?.forEach { logFile ->
-					logger.lifecycle("deleting log: ${logFile.name}")
-					logFile.delete()
+					val deleted = logFile.delete()
+					if (deleted) {
+						logger.lifecycle("deleted log: ${logFile.name}")
+					} else {
+						logger.warn("failed to delete log: ${logFile.name}")
+					}
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`
at line 93, The call to logFile.delete() in AndroidIDEInitScriptPlugin.kt
currently ignores the boolean result; update the cleanup to check the return
value of logFile.delete() and log a warning or error when it returns false so
failed deletions are visible (e.g., use the plugin logger or project.logger in
the AndroidIDEInitScriptPlugin to emit a message referencing logFile.path).
Ensure this check is added where logFile.delete() is invoked so failures are
reported without changing control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`:
- Around line 95-100: The else branch in AndroidIDEInitScriptPlugin.kt currently
logs an inaccurate message when (!logsDir.exists() || !logsDir.isDirectory);
update the logic around logsDir so the code distinguishes the cases: if logsDir
does not exist log that the daemon logs directory for currentGradleVersion is
missing, if logsDir exists but is not a directory log that the path is not a
directory, otherwise (when it is a valid directory) log that no deletions were
made because the number of log files does not exceed MAX_LOGFILE_COUNT; use the
existing symbols logsDir, currentGradleVersion, MAX_LOGFILE_COUNT and
logger.lifecycle to implement these precise messages.

---

Nitpick comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt`:
- Line 93: The call to logFile.delete() in AndroidIDEInitScriptPlugin.kt
currently ignores the boolean result; update the cleanup to check the return
value of logFile.delete() and log a warning or error when it returns false so
failed deletions are visible (e.g., use the plugin logger or project.logger in
the AndroidIDEInitScriptPlugin to emit a message referencing logFile.path).
Ensure this check is added where logFile.delete() is invoked so failures are
reported without changing control flow.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d87843 and 39fd9be.

📒 Files selected for processing (1)
  • gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`:
- Around line 51-60: getTestEnvProps currently splits the
_PROPERTY_MAVEN_LOCAL_REPOSITORY string with a hardcoded ':' which breaks on
Windows; change the split to use the OS-aware path separator
(File.pathSeparatorChar or File.pathSeparator) so repository paths like "C:\..."
or semicolon-separated lists are parsed correctly; update the split call in
getTestEnvProps (referencing StartParameter and
_PROPERTY_MAVEN_LOCAL_REPOSITORY) to use File.pathSeparatorChar and ensure the
import/reference to java.io.File is available.
- Around line 75-83: In RepositoryHandler.addMavenRepoIfMissing, remove the
unnecessary addLast(...) wrapper around the maven call; instead, when the none {
it is MavenArtifactRepository && it.url == uri } check passes, call maven {
it.url = uri } directly (keep the logger.info("Adding maven repository: $uri")
and the same uri/Logger parameters intact) so the repository is added by the
maven(Action) call itself.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39fd9be and 88a549b.

📒 Files selected for processing (2)
  • gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt
  • gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.kt

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@jatezzz jatezzz self-requested a review March 2, 2026 13:14
@itsaky-adfa itsaky-adfa merged commit fe99d3f into stage Mar 2, 2026
2 checks passed
@itsaky-adfa itsaky-adfa deleted the fix/ADFA-2849 branch March 2, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants