ADFA-3580: (feat) Plugin Build Actions & Custom Scripts System#1150
Open
Daniel-ADFA wants to merge 2 commits intostagefrom
Open
ADFA-3580: (feat) Plugin Build Actions & Custom Scripts System#1150Daniel-ADFA wants to merge 2 commits intostagefrom
Daniel-ADFA wants to merge 2 commits intostagefrom
Conversation
Adds first-class API for plugins to declare build actions (shell commands and Gradle tasks), execute them with streaming output to the Build Output panel, and control toolbar action visibility. Includes custom scripts support via .codeonthego/scripts.json — plugins can auto-detect project type (Node.js, Python, Rust, Go, Make, Ruby) and bootstrap user-editable run scripts on first project open. New plugin-api interfaces: BuildActionExtension, IdeCommandService, CommandExecution. New plugin-manager implementations: IdeCommandServiceImpl (ProcessBuilder-based with Termux env injection), PluginBuildActionManager (singleton orchestrator). New app integration: PluginBuildActionItem
Adds first-class API for plugins to declare build actions (shell commands and Gradle tasks), execute them with streaming output to the Build Output panel, and control toolbar action visibility. Includes custom scripts support via .codeonthego/scripts.json — plugins can auto-detect project type (Node.js, Python, Rust, Go, Make, Ruby) and bootstrap user-editable run scripts on first project open. New plugin-api interfaces: BuildActionExtension, IdeCommandService, CommandExecution. New plugin-manager implementations: IdeCommandServiceImpl (ProcessBuilder-based with Termux env injection), PluginBuildActionManager (singleton orchestrator). New app integration: PluginBuildActionItem
Contributor
📝 WalkthroughRelease Notes: Plugin Build Actions & Custom Scripts SystemNew Features
Technical Implementation
API Changes
Risk Considerations
WalkthroughThis pull request introduces a comprehensive plugin build action system, enabling plugins to register and execute custom build actions within the Android IDE editor. The system includes a new plugin API contract, command execution service, centralized action manager, and editor toolbar integration for displaying and executing plugin-defined build actions. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Editor as Editor Activity
participant Action as PluginBuildActionItem
participant Manager as PluginBuildActionManager
participant Service as IdeCommandService
participant Process as Process/Executor
User->>Editor: Clicks plugin build action
Editor->>Action: execAction(data)
Action->>Manager: isActionRunning(pluginId, actionId)
Manager-->>Action: false
Action->>Manager: executeAction(pluginId, actionId, service)
Manager->>Manager: onActionStarted(actionId)
Manager->>Service: executeCommand(CommandSpec)
Service->>Process: spawn process / task
Process-->>Service: output stream
Service-->>Manager: CommandExecution(output flow, await, cancel)
Manager-->>Action: CommandExecution
Action->>Action: mark build in-progress
Action->>Action: toggle bottom sheet to build output
par Concurrent Output Collection
Action->>Service: collect output flow
Service->>Process: read stdout/stderr
Process-->>Service: output lines
Service-->>Action: emit CommandOutput events
Action->>Action: append lines to UI
and Process Waits
Process-->>Service: completion/timeout
Service->>Manager: notifyActionCompleted(result)
Manager->>Manager: onActionCompleted(actionId, result)
Manager-->>Action: action cleanup
Action->>Action: reset progress state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt (1)
18-19: Intermediate variable is unnecessary.The local
drawablevariable adds no value here—consider returning directly for conciseness.♻️ Suggested simplification
try { - val drawable = ContextCompat.getDrawable(pluginContext, resId) - return drawable + return ContextCompat.getDrawable(pluginContext, resId) } catch (_: Resources.NotFoundException) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt` around lines 18 - 19, In PluginDrawableResolver (the method that currently does "val drawable = ContextCompat.getDrawable(pluginContext, resId); return drawable"), remove the unnecessary local variable and return the result of ContextCompat.getDrawable(pluginContext, resId) directly to simplify the code and improve conciseness; update the method body to a single direct return of ContextCompat.getDrawable(pluginContext, resId).plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeCommandService.kt (1)
8-20: Consider adding KDoc for the public plugin API.This is a new public API surface for plugin developers. Adding documentation would improve discoverability and usage:
📝 Proposed documentation additions
+/** + * Service for executing shell commands and Gradle tasks from plugins. + */ interface IdeCommandService { + /** + * Starts command execution with the given specification. + * `@param` spec The command to execute (shell command or Gradle task) + * `@param` timeoutMs Maximum execution time in milliseconds (default: 10 minutes) + * `@return` A CommandExecution handle for monitoring and controlling the execution + */ fun executeCommand(spec: CommandSpec, timeoutMs: Long = 600_000): CommandExecution fun isCommandRunning(executionId: String): Boolean fun cancelCommand(executionId: String): Boolean fun getRunningCommandCount(): Int } +/** + * Handle for a running command execution. + */ interface CommandExecution { + /** Unique identifier for this execution */ val executionId: String + /** Flow emitting stdout, stderr, and exit code as they become available */ val output: Flow<CommandOutput> + /** Suspends until the command completes and returns the result */ suspend fun await(): CommandResult + /** Requests cancellation of the running command */ fun cancel() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeCommandService.kt` around lines 8 - 20, Add KDoc comments describing the public API surface: document the IdeCommandService interface and each method (executeCommand, isCommandRunning, cancelCommand, getRunningCommandCount) and the CommandExecution interface and its members (executionId, output, await, cancel) to explain purpose, parameters, return types, default values (e.g., timeoutMs default 600_000), possible exceptions/behaviour (cancellation semantics, lifecycle of output Flow), and usage examples or thread-safety notes as appropriate to help plugin authors discover and use these APIs.app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt (1)
135-140: Clarify the relationship betweenbuildServiceand plugin action state.
resetProgressIfIdlechecksbuildService?.isBuildInProgressbut plugin build actions don't appear to updatebuildServicestate. This could lead to inconsistent progress indicators if a Gradle build runs concurrently with a plugin action. Consider either:
- Having plugin actions update
buildService.isBuildInProgress- Or tracking plugin action progress separately with clear documentation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt` around lines 135 - 140, resetProgressIfIdle currently only checks buildService?.isBuildInProgress which is never updated by plugin actions; either update the plugin action lifecycle to flip buildService.isBuildInProgress when a plugin build starts/ends (set buildService?.isBuildInProgress = true at action start and false at completion in the PluginBuildActionItem action handlers) or add a separate flag (e.g., EditorHandlerActivity.editorViewModel.isPluginActionInProgress) and change resetProgressIfIdle to consider both buildService?.isBuildInProgress and isPluginActionInProgress so UI progress reflects both Gradle builds and plugin actions consistently.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginManifest.kt (1)
61-75: Consider adding@SerializedNameannotations for consistency.The
ManifestBuildActiondata class has inconsistent serialization annotations. Some fields use@SerializedName(e.g.,gradle_task,working_directory,timeout_ms) while others rely on Gson's default field name matching. For maintainability and clarity about the expected JSON schema, consider adding explicit annotations to all fields.♻️ Proposed fix for consistent annotations
data class ManifestBuildAction( + `@SerializedName`("id") val id: String, + `@SerializedName`("name") val name: String, + `@SerializedName`("description") val description: String = "", + `@SerializedName`("category") val category: String = "CUSTOM", + `@SerializedName`("command") val command: String? = null, + `@SerializedName`("arguments") val arguments: List<String> = emptyList(), `@SerializedName`("gradle_task") val gradleTask: String? = null, `@SerializedName`("working_directory") val workingDirectory: String? = null, + `@SerializedName`("environment") val environment: Map<String, String> = emptyMap(), `@SerializedName`("timeout_ms") val timeoutMs: Long = 600_000 )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginManifest.kt` around lines 61 - 75, ManifestBuildAction has mixed use of `@SerializedName` which can confuse the expected JSON schema; update the data class by adding `@SerializedName` annotations for all properties (id, name, description, category, command, arguments, environment) to match the JSON field names you expect (e.g., "id", "name", "description", "category", "command", "arguments", "environment") while retaining the existing annotations for gradleTask ("gradle_task"), workingDirectory ("working_directory") and timeoutMs ("timeout_ms") so serialization is consistent and explicit across the class.app/src/main/java/com/itsaky/androidide/utils/EditorActivityActions.kt (1)
192-193: Silent exception swallowing may hide plugin registration issues.Per project learnings, prefer narrow exception handling over broad catch-all. Silently swallowing exceptions makes debugging plugin registration failures difficult. Consider logging or catching only expected exception types.
♻️ Proposed fix to add minimal logging
- } catch (_: Exception) { + } catch (e: Exception) { + Log.w("plugin_debug", "Failed to register menu items for plugin: ${plugin.javaClass.simpleName}", e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/utils/EditorActivityActions.kt` around lines 192 - 193, The empty catch block swallowing all Exceptions should be replaced: catch only the expected exception types thrown during plugin registration (e.g., IllegalStateException/IOException or the concrete exceptions your plugin APIs throw) or, if you must catch Exception, log the error instead of ignoring it; update the catch in EditorActivityActions.kt (the catch (_: Exception) { } block used around plugin registration) to call the project logging utility (or Android Log.e/Timber) with a clear message like "plugin registration failed" and include the exception so failures are visible for debugging.plugin-api/build.gradle.kts (1)
32-33: Usingapi()is correct; consider upgrading to kotlinx-coroutines 1.10.0+.The
apiconfiguration is appropriate sinceFlow<CommandOutput>is exposed in the publicIdeCommandServiceinterface. Coroutines 1.9.0 is compatible with your project's Kotlin 2.1.21 and poses no compatibility issues. However, for Kotlin 2.1 projects, kotlinx-coroutines 1.10.0+ is the recommended version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-api/build.gradle.kts` around lines 32 - 33, Keep the api configuration but update the kotlinx-coroutines dependency version: replace the api("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.9.0") entry with a 1.10.0+ version (e.g., 1.10.0 or later) to follow Kotlin 2.1.21 recommendations and ensure compatibility for the public Flow<CommandOutput> in IdeCommandService.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt (2)
50-56: Validate thatgradlewexists and is executable before use.The code assumes
gradlewexists in the project root but doesn't verify this. If the wrapper is missing or not executable, the process will fail with a potentially confusing error.🛠️ Proposed fix
is CommandSpec.GradleTask -> { val gradleWrapper = projectRoot?.let { File(it, "gradlew") } ?: throw IllegalStateException("No project root available for Gradle task execution") + if (!gradleWrapper.exists()) { + throw IllegalStateException("Gradle wrapper not found at ${gradleWrapper.absolutePath}") + } + if (!gradleWrapper.canExecute()) { + throw IllegalStateException("Gradle wrapper is not executable: ${gradleWrapper.absolutePath}") + } ProcessBuilder(listOf(gradleWrapper.absolutePath, spec.taskPath) + spec.arguments).apply { directory(projectRoot) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt` around lines 50 - 56, CommandSpec.GradleTask currently assumes gradlew exists and is executable; modify the block that builds gradleWrapper so you explicitly validate File(projectRoot, "gradlew") exists and isFile and isExecutable (or attempt gradleWrapper.setExecutable(true) and re-check), and if those checks fail throw a clear IllegalStateException mentioning missing or non-executable gradlew; then proceed to construct the ProcessBuilder with gradleWrapper.absolutePath, spec.taskPath and spec.arguments as before. Ensure you reference gradleWrapper, projectRoot, CommandSpec.GradleTask and ProcessBuilder when locating and updating the code.
85-88: Minor race window between cancellation and clear.A command could be added between
forEachcompleting andclear()being called, causing it to be silently removed without cancellation. Consider iterating until the map is empty or usingremoveIf.♻️ Proposed fix
fun cancelAllCommands() { - runningCommands.values.forEach { it.cancel() } - runningCommands.clear() + runningCommands.entries.removeAll { (_, execution) -> + execution.cancel() + true + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt` around lines 85 - 88, cancelAllCommands has a race where a command can be added between iterating and clear(); fix by making the cancel-and-clear atomic: wrap the loop and clear in a single synchronized block (or use a dedicated mutex) so that runningCommands.values.forEach { it.cancel() } and runningCommands.clear() execute under the same lock; update the cancelAllCommands implementation to synchronize on the runningCommands instance (or the class-level lock) to prevent new entries being added while cancelling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt`:
- Around line 42-62: The override of prepare in PluginBuildActionItem is missing
a call to the base implementation; call super.prepare(data) at the start of
PluginBuildActionItem.prepare so BaseBuildAction.prepare can perform its context
validation and set visible/enabled based on buildService state before you apply
plugin-specific logic (or if you intentionally want to bypass that behavior, add
a comment explaining the reason and consider extracting a different base class
rather than skipping super.prepare); keep the rest of the method (using
PluginBuildActionManager.isActionRunning and setting label/icon/enabled)
unchanged.
- Around line 112-130: The coroutine launched on actionScope can outlive the
Activity and call withContext(Dispatchers.Main) after the Activity is destroyed;
change the launch to use the Activity lifecycle (replace actionScope.launch with
activity.lifecycleScope.launch or otherwise tie the Job to activity.lifecycle),
wrap the execution.output.collect and execution.await call in a try-catch that
handles CancellationException and other exceptions, and before calling
activity.appendBuildOutput(...) or resetProgressIfIdle(activity) check
activity.isFinishing/isDestroyed (or use lifecycleScope which cancels
automatically) so UI updates are only attempted when the Activity is alive; keep
manager.notifyActionCompleted(pluginId, actionId, result) behavior but ensure it
runs in a safe context if it touches UI.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/build/PluginBuildActionManager.kt`:
- Around line 81-96: The executeAction function can throw from
commandService.executeCommand after extension?.onActionStarted(actionId) was
called, leaving plugins without onActionCompleted and activeExecutions
inconsistent; wrap the call to commandService.executeCommand(action.command,
action.timeoutMs) in a try/catch, on success continue to store the execution in
activeExecutions as now, but on catch call
extension?.onActionCompleted(actionId) (and ensure nothing was added to
activeExecutions) and either rethrow the exception or return null so the caller
knows it failed; ensure the executionKey and activeExecutions[executionKey]
assignment remain only after a successful executeCommand.
- Around line 50-55: The current code in PluginBuildActionManager silently
swallows Throwable when iterating extension.getBuildActions() and at the other
two similar sites, which hides serious errors; change each broad catch (_:
Throwable) to catch a narrow, expected exception type (e.g.,
IllegalStateException or Exception) and log the error instead of ignoring it —
include pluginId and name (and action when available) in the log message to aid
debugging; apply the same change for the other two occurrences in the same class
(the blocks that create RegisteredBuildAction entries) so failures are visible
and do not suppress Errors like OutOfMemoryError.
- Around line 115-120: In cleanupPlugin(pluginId: String) first find all
matching entries in activeExecutions whose keys startWith("$pluginId:"), call
the appropriate cancellation/termination method on each execution handle (e.g.,
cancel()/terminate()/stop() depending on the execution handle API) to stop the
running processes, wait or handle failures if the API is asynchronous, and only
after canceling remove those entries from activeExecutions; then proceed to
remove pluginExtensions.remove(pluginId), manifestActions.remove(pluginId), and
pluginNames.remove(pluginId).
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt`:
- Around line 20-22: The catch-all `catch (_: Throwable)` in
PluginDrawableResolver.kt should be replaced with narrow, explicit catches for
known recoverable exceptions (e.g., `Resources.NotFoundException` is already
caught; add `IllegalArgumentException` or other specific exceptions you've
observed) and avoid swallowing `Error` types; for any unexpected exception types
that you do want to catch, add a minimal log (e.g., warning with the exception
message and context) so failures are visible instead of silent. Locate the
try/catch block in class PluginDrawableResolver and replace the broad Throwable
catch with explicit exception types relevant to drawable resolution (and add
logging in those unexpected-but-handled branches).
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt`:
- Around line 135-140: resetProgressIfIdle currently only checks
buildService?.isBuildInProgress which is never updated by plugin actions; either
update the plugin action lifecycle to flip buildService.isBuildInProgress when a
plugin build starts/ends (set buildService?.isBuildInProgress = true at action
start and false at completion in the PluginBuildActionItem action handlers) or
add a separate flag (e.g.,
EditorHandlerActivity.editorViewModel.isPluginActionInProgress) and change
resetProgressIfIdle to consider both buildService?.isBuildInProgress and
isPluginActionInProgress so UI progress reflects both Gradle builds and plugin
actions consistently.
In `@app/src/main/java/com/itsaky/androidide/utils/EditorActivityActions.kt`:
- Around line 192-193: The empty catch block swallowing all Exceptions should be
replaced: catch only the expected exception types thrown during plugin
registration (e.g., IllegalStateException/IOException or the concrete exceptions
your plugin APIs throw) or, if you must catch Exception, log the error instead
of ignoring it; update the catch in EditorActivityActions.kt (the catch (_:
Exception) { } block used around plugin registration) to call the project
logging utility (or Android Log.e/Timber) with a clear message like "plugin
registration failed" and include the exception so failures are visible for
debugging.
In `@plugin-api/build.gradle.kts`:
- Around line 32-33: Keep the api configuration but update the
kotlinx-coroutines dependency version: replace the
api("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.9.0") entry with a 1.10.0+
version (e.g., 1.10.0 or later) to follow Kotlin 2.1.21 recommendations and
ensure compatibility for the public Flow<CommandOutput> in IdeCommandService.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeCommandService.kt`:
- Around line 8-20: Add KDoc comments describing the public API surface:
document the IdeCommandService interface and each method (executeCommand,
isCommandRunning, cancelCommand, getRunningCommandCount) and the
CommandExecution interface and its members (executionId, output, await, cancel)
to explain purpose, parameters, return types, default values (e.g., timeoutMs
default 600_000), possible exceptions/behaviour (cancellation semantics,
lifecycle of output Flow), and usage examples or thread-safety notes as
appropriate to help plugin authors discover and use these APIs.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginManifest.kt`:
- Around line 61-75: ManifestBuildAction has mixed use of `@SerializedName` which
can confuse the expected JSON schema; update the data class by adding
`@SerializedName` annotations for all properties (id, name, description, category,
command, arguments, environment) to match the JSON field names you expect (e.g.,
"id", "name", "description", "category", "command", "arguments", "environment")
while retaining the existing annotations for gradleTask ("gradle_task"),
workingDirectory ("working_directory") and timeoutMs ("timeout_ms") so
serialization is consistent and explicit across the class.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt`:
- Around line 50-56: CommandSpec.GradleTask currently assumes gradlew exists and
is executable; modify the block that builds gradleWrapper so you explicitly
validate File(projectRoot, "gradlew") exists and isFile and isExecutable (or
attempt gradleWrapper.setExecutable(true) and re-check), and if those checks
fail throw a clear IllegalStateException mentioning missing or non-executable
gradlew; then proceed to construct the ProcessBuilder with
gradleWrapper.absolutePath, spec.taskPath and spec.arguments as before. Ensure
you reference gradleWrapper, projectRoot, CommandSpec.GradleTask and
ProcessBuilder when locating and updating the code.
- Around line 85-88: cancelAllCommands has a race where a command can be added
between iterating and clear(); fix by making the cancel-and-clear atomic: wrap
the loop and clear in a single synchronized block (or use a dedicated mutex) so
that runningCommands.values.forEach { it.cancel() } and runningCommands.clear()
execute under the same lock; update the cancelAllCommands implementation to
synchronize on the runningCommands instance (or the class-level lock) to prevent
new entries being added while cancelling.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt`:
- Around line 18-19: In PluginDrawableResolver (the method that currently does
"val drawable = ContextCompat.getDrawable(pluginContext, resId); return
drawable"), remove the unnecessary local variable and return the result of
ContextCompat.getDrawable(pluginContext, resId) directly to simplify the code
and improve conciseness; update the method body to a single direct return of
ContextCompat.getDrawable(pluginContext, resId).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c86fe83-7c35-439f-b672-371e07a39546
📒 Files selected for processing (12)
app/src/main/java/com/itsaky/androidide/actions/EditorActivityAction.ktapp/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.ktapp/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/utils/EditorActivityActions.ktplugin-api/build.gradle.ktsplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/BuildActionExtension.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeCommandService.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/build/PluginBuildActionManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginManifest.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt
Comment on lines
+42
to
+62
| override fun prepare(data: ActionData) { | ||
| val context = data.getActivity() | ||
| if (context == null) { | ||
| visible = false | ||
| return | ||
| } | ||
| visible = true | ||
|
|
||
| val manager = PluginBuildActionManager.getInstance() | ||
| val isRunning = manager.isActionRunning(registered.pluginId, registered.action.id) | ||
|
|
||
| if (isRunning) { | ||
| label = "Cancel ${registered.action.name}" | ||
| icon = ContextCompat.getDrawable(context, R.drawable.ic_stop) | ||
| enabled = true | ||
| } else { | ||
| label = registered.action.name | ||
| icon = resolvePluginIcon(context) | ||
| enabled = true | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Missing super.prepare(data) call.
BaseBuildAction.prepare() performs context validation and sets visible/enabled based on build service state. Not calling super.prepare(data) bypasses these checks. While you're handling visibility manually, the base class behavior should be considered.
🔧 Proposed fix
override fun prepare(data: ActionData) {
+ super.prepare(data)
val context = data.getActivity()
if (context == null) {
visible = false
return
}
visible = trueNote: If you intentionally want to bypass BaseBuildAction's buildService.isBuildInProgress check (since plugin actions have their own running state), consider documenting this or extracting a different base class.
📝 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.
Suggested change
| override fun prepare(data: ActionData) { | |
| val context = data.getActivity() | |
| if (context == null) { | |
| visible = false | |
| return | |
| } | |
| visible = true | |
| val manager = PluginBuildActionManager.getInstance() | |
| val isRunning = manager.isActionRunning(registered.pluginId, registered.action.id) | |
| if (isRunning) { | |
| label = "Cancel ${registered.action.name}" | |
| icon = ContextCompat.getDrawable(context, R.drawable.ic_stop) | |
| enabled = true | |
| } else { | |
| label = registered.action.name | |
| icon = resolvePluginIcon(context) | |
| enabled = true | |
| } | |
| } | |
| override fun prepare(data: ActionData) { | |
| super.prepare(data) | |
| val context = data.getActivity() | |
| if (context == null) { | |
| visible = false | |
| return | |
| } | |
| visible = true | |
| val manager = PluginBuildActionManager.getInstance() | |
| val isRunning = manager.isActionRunning(registered.pluginId, registered.action.id) | |
| if (isRunning) { | |
| label = "Cancel ${registered.action.name}" | |
| icon = ContextCompat.getDrawable(context, R.drawable.ic_stop) | |
| enabled = true | |
| } else { | |
| label = registered.action.name | |
| icon = resolvePluginIcon(context) | |
| enabled = true | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt`
around lines 42 - 62, The override of prepare in PluginBuildActionItem is
missing a call to the base implementation; call super.prepare(data) at the start
of PluginBuildActionItem.prepare so BaseBuildAction.prepare can perform its
context validation and set visible/enabled based on buildService state before
you apply plugin-specific logic (or if you intentionally want to bypass that
behavior, add a comment explaining the reason and consider extracting a
different base class rather than skipping super.prepare); keep the rest of the
method (using PluginBuildActionManager.isActionRunning and setting
label/icon/enabled) unchanged.
Comment on lines
+112
to
+130
| actionScope.launch { | ||
| execution.output.collect { output -> | ||
| val line = when (output) { | ||
| is CommandOutput.StdOut -> output.line | ||
| is CommandOutput.StdErr -> output.line | ||
| is CommandOutput.ExitCode -> | ||
| if (output.code != 0) "Process failed with code ${output.code}" else null | ||
| } | ||
| if (line != null) { | ||
| withContext(Dispatchers.Main) { | ||
| activity.appendBuildOutput(line) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| val result = execution.await() | ||
| manager.notifyActionCompleted(pluginId, actionId, result) | ||
| withContext(Dispatchers.Main) { resetProgressIfIdle(activity) } | ||
| } |
Contributor
There was a problem hiding this comment.
Consider lifecycle-safe coroutine handling.
The coroutine launched in actionScope may outlive the activity if the user navigates away during a long-running build. This could lead to crashes when withContext(Dispatchers.Main) tries to update a destroyed activity. Consider:
- Using
activity.lifecycleScopeinstead ofactionScopefor automatic cancellation. - Adding null-safety checks before UI updates.
- Wrapping the collect/await in try-catch for cancellation exceptions.
🛡️ Proposed lifecycle-safe pattern
- actionScope.launch {
- execution.output.collect { output ->
+ activity.lifecycleScope.launch {
+ try {
+ execution.output.collect { output ->
val line = when (output) {
is CommandOutput.StdOut -> output.line
is CommandOutput.StdErr -> output.line
is CommandOutput.ExitCode ->
if (output.code != 0) "Process failed with code ${output.code}" else null
}
if (line != null) {
- withContext(Dispatchers.Main) {
- activity.appendBuildOutput(line)
- }
+ activity.appendBuildOutput(line)
}
- }
+ }
- val result = execution.await()
- manager.notifyActionCompleted(pluginId, actionId, result)
- withContext(Dispatchers.Main) { resetProgressIfIdle(activity) }
+ val result = execution.await()
+ manager.notifyActionCompleted(pluginId, actionId, result)
+ resetProgressIfIdle(activity)
+ } catch (e: CancellationException) {
+ manager.cancelAction(pluginId, actionId)
+ throw e
+ }
}Note: Using lifecycleScope automatically runs on Main dispatcher and cancels when the activity is destroyed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt`
around lines 112 - 130, The coroutine launched on actionScope can outlive the
Activity and call withContext(Dispatchers.Main) after the Activity is destroyed;
change the launch to use the Activity lifecycle (replace actionScope.launch with
activity.lifecycleScope.launch or otherwise tie the Job to activity.lifecycle),
wrap the execution.output.collect and execution.await call in a try-catch that
handles CancellationException and other exceptions, and before calling
activity.appendBuildOutput(...) or resetProgressIfIdle(activity) check
activity.isFinishing/isDestroyed (or use lifecycleScope which cancels
automatically) so UI updates are only attempted when the Activity is alive; keep
manager.notifyActionCompleted(pluginId, actionId, result) behavior but ensure it
runs in a safe context if it touches UI.
Comment on lines
+50
to
+55
| try { | ||
| extension.getBuildActions().forEach { action -> | ||
| actions.add(RegisteredBuildAction(pluginId, name, action)) | ||
| } | ||
| } catch (_: Throwable) {} | ||
| } |
Contributor
There was a problem hiding this comment.
Swallowing Throwable hides critical errors and impedes debugging.
Catching and silently ignoring Throwable (which includes Error types like OutOfMemoryError) violates fail-fast principles and makes plugin failures invisible. This pattern repeats at lines 72-75 and 124-126.
Based on learnings, prefer narrow exception handling that catches only specific expected exceptions (e.g., IllegalStateException) rather than a broad catch-all.
🛠️ Proposed fix: Log errors and narrow the exception type
try {
extension.getBuildActions().forEach { action ->
actions.add(RegisteredBuildAction(pluginId, name, action))
}
- } catch (_: Throwable) {}
+ } catch (e: Exception) {
+ // Log plugin failure for debugging; consider surfacing to user
+ android.util.Log.w("PluginBuildActionManager", "Failed to get build actions from plugin $pluginId", e)
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/build/PluginBuildActionManager.kt`
around lines 50 - 55, The current code in PluginBuildActionManager silently
swallows Throwable when iterating extension.getBuildActions() and at the other
two similar sites, which hides serious errors; change each broad catch (_:
Throwable) to catch a narrow, expected exception type (e.g.,
IllegalStateException or Exception) and log the error instead of ignoring it —
include pluginId and name (and action when available) in the log message to aid
debugging; apply the same change for the other two occurrences in the same class
(the blocks that create RegisteredBuildAction entries) so failures are visible
and do not suppress Errors like OutOfMemoryError.
Comment on lines
+81
to
+96
| fun executeAction( | ||
| pluginId: String, | ||
| actionId: String, | ||
| commandService: IdeCommandService | ||
| ): CommandExecution? { | ||
| val action = findAction(pluginId, actionId) ?: return null | ||
| val extension = pluginExtensions[pluginId] | ||
|
|
||
| extension?.onActionStarted(actionId) | ||
|
|
||
| val execution = commandService.executeCommand(action.command, action.timeoutMs) | ||
| val executionKey = "$pluginId:$actionId" | ||
| activeExecutions[executionKey] = execution | ||
|
|
||
| return execution | ||
| } |
Contributor
There was a problem hiding this comment.
Missing error handling leaves actions in inconsistent state if executeCommand throws.
If commandService.executeCommand() throws an exception after onActionStarted() is called, the plugin never receives onActionCompleted(), leaving it in an inconsistent state.
🛠️ Proposed fix
fun executeAction(
pluginId: String,
actionId: String,
commandService: IdeCommandService
): CommandExecution? {
val action = findAction(pluginId, actionId) ?: return null
val extension = pluginExtensions[pluginId]
extension?.onActionStarted(actionId)
- val execution = commandService.executeCommand(action.command, action.timeoutMs)
- val executionKey = "$pluginId:$actionId"
- activeExecutions[executionKey] = execution
-
- return execution
+ return try {
+ val execution = commandService.executeCommand(action.command, action.timeoutMs)
+ val executionKey = "$pluginId:$actionId"
+ activeExecutions[executionKey] = execution
+ execution
+ } catch (e: Exception) {
+ extension?.onActionCompleted(actionId, CommandResult.Failure(-1, "", "", e.message, 0))
+ throw e
+ }
}📝 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.
Suggested change
| fun executeAction( | |
| pluginId: String, | |
| actionId: String, | |
| commandService: IdeCommandService | |
| ): CommandExecution? { | |
| val action = findAction(pluginId, actionId) ?: return null | |
| val extension = pluginExtensions[pluginId] | |
| extension?.onActionStarted(actionId) | |
| val execution = commandService.executeCommand(action.command, action.timeoutMs) | |
| val executionKey = "$pluginId:$actionId" | |
| activeExecutions[executionKey] = execution | |
| return execution | |
| } | |
| fun executeAction( | |
| pluginId: String, | |
| actionId: String, | |
| commandService: IdeCommandService | |
| ): CommandExecution? { | |
| val action = findAction(pluginId, actionId) ?: return null | |
| val extension = pluginExtensions[pluginId] | |
| extension?.onActionStarted(actionId) | |
| return try { | |
| val execution = commandService.executeCommand(action.command, action.timeoutMs) | |
| val executionKey = "$pluginId:$actionId" | |
| activeExecutions[executionKey] = execution | |
| execution | |
| } catch (e: Exception) { | |
| extension?.onActionCompleted(actionId, CommandResult.Failure(-1, "", "", e.message, 0)) | |
| throw e | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/build/PluginBuildActionManager.kt`
around lines 81 - 96, The executeAction function can throw from
commandService.executeCommand after extension?.onActionStarted(actionId) was
called, leaving plugins without onActionCompleted and activeExecutions
inconsistent; wrap the call to commandService.executeCommand(action.command,
action.timeoutMs) in a try/catch, on success continue to store the execution in
activeExecutions as now, but on catch call
extension?.onActionCompleted(actionId) (and ensure nothing was added to
activeExecutions) and either rethrow the exception or return null so the caller
knows it failed; ensure the executionKey and activeExecutions[executionKey]
assignment remain only after a successful executeCommand.
Comment on lines
+115
to
+120
| fun cleanupPlugin(pluginId: String) { | ||
| activeExecutions.entries.removeAll { it.key.startsWith("$pluginId:") } | ||
| pluginExtensions.remove(pluginId) | ||
| manifestActions.remove(pluginId) | ||
| pluginNames.remove(pluginId) | ||
| } |
Contributor
There was a problem hiding this comment.
cleanupPlugin should cancel active executions before removing them.
Simply removing entries from activeExecutions leaves processes running in the background without any way to manage them afterward. This can cause resource leaks and orphaned processes.
🛠️ Proposed fix
fun cleanupPlugin(pluginId: String) {
- activeExecutions.entries.removeAll { it.key.startsWith("$pluginId:") }
+ activeExecutions.entries
+ .filter { it.key.startsWith("$pluginId:") }
+ .forEach { (key, execution) ->
+ execution.cancel()
+ activeExecutions.remove(key)
+ }
pluginExtensions.remove(pluginId)
manifestActions.remove(pluginId)
pluginNames.remove(pluginId)
}📝 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.
Suggested change
| fun cleanupPlugin(pluginId: String) { | |
| activeExecutions.entries.removeAll { it.key.startsWith("$pluginId:") } | |
| pluginExtensions.remove(pluginId) | |
| manifestActions.remove(pluginId) | |
| pluginNames.remove(pluginId) | |
| } | |
| fun cleanupPlugin(pluginId: String) { | |
| activeExecutions.entries | |
| .filter { it.key.startsWith("$pluginId:") } | |
| .forEach { (key, execution) -> | |
| execution.cancel() | |
| activeExecutions.remove(key) | |
| } | |
| pluginExtensions.remove(pluginId) | |
| manifestActions.remove(pluginId) | |
| pluginNames.remove(pluginId) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/build/PluginBuildActionManager.kt`
around lines 115 - 120, In cleanupPlugin(pluginId: String) first find all
matching entries in activeExecutions whose keys startWith("$pluginId:"), call
the appropriate cancellation/termination method on each execution handle (e.g.,
cancel()/terminate()/stop() depending on the execution handle API) to stop the
running processes, wait or handle failures if the API is asynchronous, and only
after canceling remove those entries from activeExecutions; then proceed to
remove pluginExtensions.remove(pluginId), manifestActions.remove(pluginId), and
pluginNames.remove(pluginId).
Comment on lines
+186
to
+201
| } catch (e: kotlinx.coroutines.TimeoutCancellationException) { | ||
| process?.destroyForcibly() | ||
| outputChannel.close() | ||
| val duration = System.currentTimeMillis() - startTime | ||
| resultDeferred.complete( | ||
| CommandResult.Failure(-1, stdoutBuilder.toString(), stderrBuilder.toString(), "Command timed out after ${timeoutMs}ms", duration) | ||
| ) | ||
| } catch (e: Exception) { | ||
| process?.destroyForcibly() | ||
| outputChannel.close() | ||
| val duration = System.currentTimeMillis() - startTime | ||
| if (resultDeferred.isActive) { | ||
| resultDeferred.complete( | ||
| CommandResult.Cancelled(stdoutBuilder.toString(), stderrBuilder.toString()) | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Swallowed exceptions lose diagnostic information.
Both exception handlers discard the exception details. For TimeoutCancellationException (line 186), the message is reconstructed but the original exception is lost. For generic Exception (line 193), no error information is preserved in the Cancelled result, making debugging difficult.
Per retrieved learnings and static analysis hints, prefer narrow exception handling and preserve error context.
🛠️ Proposed fix: Preserve exception information and narrow catch type
} catch (e: kotlinx.coroutines.TimeoutCancellationException) {
process?.destroyForcibly()
outputChannel.close()
val duration = System.currentTimeMillis() - startTime
resultDeferred.complete(
- CommandResult.Failure(-1, stdoutBuilder.toString(), stderrBuilder.toString(), "Command timed out after ${timeoutMs}ms", duration)
+ CommandResult.Failure(-1, stdoutBuilder.toString(), stderrBuilder.toString(), "Command timed out after ${timeoutMs}ms: ${e.message}", duration)
)
- } catch (e: Exception) {
+ } catch (e: kotlinx.coroutines.CancellationException) {
process?.destroyForcibly()
outputChannel.close()
- val duration = System.currentTimeMillis() - startTime
if (resultDeferred.isActive) {
resultDeferred.complete(
CommandResult.Cancelled(stdoutBuilder.toString(), stderrBuilder.toString())
)
}
+ } catch (e: Exception) {
+ process?.destroyForcibly()
+ outputChannel.close()
+ val duration = System.currentTimeMillis() - startTime
+ if (resultDeferred.isActive) {
+ resultDeferred.complete(
+ CommandResult.Failure(-1, stdoutBuilder.toString(), stderrBuilder.toString(), "Unexpected error: ${e.message}", duration)
+ )
+ }
} finally {🧰 Tools
🪛 detekt (1.23.8)
[warning] 186-186: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 193-193: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
Comment on lines
+20
to
+22
| } catch (_: Resources.NotFoundException) { | ||
| } catch (_: Throwable) { | ||
| } |
Contributor
There was a problem hiding this comment.
Overly broad exception handling: prefer narrow catch clauses.
Catching Throwable silently swallows Error types (e.g., OutOfMemoryError, StackOverflowError) that should propagate, and makes debugging difficult due to the empty catch body. Consider catching only specific exceptions that are known to occur here.
If additional exception types beyond Resources.NotFoundException have been observed, catch those explicitly and consider adding minimal logging for unexpected cases.
🛡️ Proposed fix: Narrow the exception handling
try {
val drawable = ContextCompat.getDrawable(pluginContext, resId)
return drawable
} catch (_: Resources.NotFoundException) {
- } catch (_: Throwable) {
+ // Drawable not found in plugin context; fall through to fallback
}If a specific additional exception has been identified (e.g., IllegalArgumentException), add it explicitly:
} catch (_: Resources.NotFoundException) {
} catch (_: IllegalArgumentException) {
}Based on learnings: "In Kotlin files across the AndroidIDE project, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all. This aligns with fail-fast behavior during development."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt`
around lines 20 - 22, The catch-all `catch (_: Throwable)` in
PluginDrawableResolver.kt should be replaced with narrow, explicit catches for
known recoverable exceptions (e.g., `Resources.NotFoundException` is already
caught; add `IllegalArgumentException` or other specific exceptions you've
observed) and avoid swallowing `Error` types; for any unexpected exception types
that you do want to catch, add a minimal log (e.g., warning with the exception
message and context) so failures are visible instead of silent. Locate the
try/catch block in class PluginDrawableResolver and replace the broad Throwable
catch with explicit exception types relevant to drawable resolution (and add
logging in those unexpected-but-handled branches).
|
is it possible to run .java and .kt files like in Intellij Idea? the results of which appear on the console |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds first-class API for plugins to declare build actions (shell commands and Gradle tasks), execute them with streaming output to the Build Output panel, and control toolbar action visibility.
Screen.Recording.2026-04-03.at.01.13.52.mov