ADFA-4584 | Extract AI features into plugins and expand the plugin API#1489
Open
jatezzz wants to merge 9 commits into
Open
ADFA-4584 | Extract AI features into plugins and expand the plugin API#1489jatezzz wants to merge 9 commits into
jatezzz wants to merge 9 commits into
Conversation
Remove the embedded agent module, llama.cpp native libraries, bundled llama.aar, and all AI UI/actions/commands from the main app. The AI functionality now lives in the plugin-examples repo as ai-core-plugin (LLM backend) and ai-assistant-plugin (chat UI). Clean up all dangling agent references (fragments, layout, bootstrap assets) that broke the build after extraction.
Broaden the plugin service surface (project manipulation, file listing, build tracking, IDEApiFacade) so extracted plugins can do what the built-in features used to. All changes are additive: keep original interface names, add default implementations, and drop no-op stubs so plugins compiled against the previous API keep loading unchanged.
Add EditorContentChangeListener plus showInlineSuggestion/dismissInlineSuggestion to IdeEditorService (default no-ops for compatibility), bridge editor content changes to plugin callbacks, and render accepted completions as inline ghost text in content coordinates. Also enable cross-plugin service resolution in PluginContext.
Add ToolbarAction.iconProvider and IdeUIService.refreshToolbarActions so plugins can drive state-based animated toolbar icons; declare RECORD_AUDIO for voice plugins. Collapse the duplicate editor renderer overrides into a single GhostTextRenderer that extends TracingEditorRenderer (keeps the ADFA-2468 block-line data-race guard), and regenerate the plugin-api golden file so apiCheck passes.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
📝 Walkthrough
WalkthroughThe PR expands plugin/editor integration with toolbar refresh hooks, editor content-change callbacks, inline ghost-text suggestions, and shared plugin lifecycle/service APIs. It also adds a manifest audio permission, starts animated toolbar icons, and hardens fragment fallback instantiation. ChangesPlugin editor and toolbar plumbing
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant PluginManager
participant IdeUIServiceImpl
participant EditorHandlerActivity
participant PluginToolbarHost
PluginManager->>IdeUIServiceImpl: refreshToolbarActions()
IdeUIServiceImpl->>EditorHandlerActivity: getCurrentActivity()
EditorHandlerActivity-->>IdeUIServiceImpl: PluginToolbarHost
IdeUIServiceImpl->>PluginToolbarHost: refreshPluginToolbarActions()
PluginToolbarHost->>EditorHandlerActivity: prepareOptionsMenu()
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
jatezzz
changed the title
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![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: 9
🧹 Nitpick comments (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/context/PluginContextImpl.kt (1)
61-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a narrower exception type than blanket
Exception.Based on learnings, this codebase's convention is to catch the specific exception type observed rather than a broad catch-all, to keep fail-fast behavior during development.
♻️ Suggested narrowing
override fun getAppSharedPreferences(prefsName: String): SharedPreferences? { return try { androidContext.getSharedPreferences(prefsName, Context.MODE_PRIVATE) - } catch (e: Exception) { + } catch (e: IllegalStateException) { logger.error("Failed to access app SharedPreferences: $prefsName", e) null } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/context/PluginContextImpl.kt` around lines 61 - 68, The getAppSharedPreferences method currently catches blanket Exception, which is too broad for this codebase’s fail-fast convention. Update the try/catch around androidContext.getSharedPreferences to handle only the specific exception type actually expected from this call path, and keep the existing logger.error and null fallback behavior unchanged.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/AndroidManifest.xml`:
- Around line 29-32: `PermissionsHelper.getRequiredPermissions()` is missing
`Manifest.permission.RECORD_AUDIO`, so mic access is not requested during
onboarding even though the manifest declares it. Update the required-permissions
list in `PermissionsHelper` to include `RECORD_AUDIO`, and ensure the onboarding
flow uses that updated list so voice/speech plugins receive the runtime
permission prompt.
In `@app/src/main/java/com/itsaky/androidide/app/EditorProviderImpl.kt`:
- Around line 75-97: The inline suggestion flow in EditorProviderImpl still
resolves the visible editor via inspectableEditor()/current(), so content
changes from non-active files can anchor ghost text on the wrong tab. Update
showInlineSuggestion/dismissInlineSuggestion to accept or derive the target file
from the document-change path, or guard on the current file before invoking
them, using the same file context handled by onDocumentChange and the
contentCallbacks callback.
In `@editor/src/main/java/com/itsaky/androidide/editor/ui/GhostTextRenderer.kt`:
- Around line 85-95: GhostTextRenderer is calling a non-existent Sora API via
editor.getCharOffsetX in the ghost text drawing path, so replace that offset
calculation with the editor’s supported public layout/offset method in
GhostTextRenderer.render/drawText handling. Use the existing editor/text
positioning symbols in this class to compute x consistently with Sora 0.23.6,
while keeping the baseline and single-line substring behavior unchanged.
In `@editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt`:
- Around line 1002-1013: The Tab acceptance flow in IDEEditor.onKeyDown is
committing a different payload than what GhostTextRenderer previews: the
renderer shows only the first line, but takeSuggestion() and
commitText(suggestion) use the full multi-line string. Update the
GhostTextRenderer/takeSuggestion path so the accepted text matches the visible
preview, or change GhostTextRenderer to render and clearly indicate the full
multi-line suggestion before committing it.
In
`@plugin-api/src/test/kotlin/com/itsaky/androidide/plugins/PluginContextTest.kt`:
- Around line 48-54: The PluginContext test stub is not exercising the actual
registerService/unregisterService contract because TestPluginContext methods are
no-op and the tests call context.services directly. Update PluginContextTest so
the assertions go through PluginContext.registerService and
PluginContext.unregisterService on the TestPluginContext/PluginContextImpl path,
and verify the effect on the service registry and per-plugin scoping rather than
testing ServiceRegistry in isolation.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/context/PluginContextImpl.kt`:
- Around line 21-47: Scope service access in PluginContextImpl so shared
services are keyed by the provider plugin, not just by service class. Update
getPluginService, registerService, and unregisterService to use the passed
pluginId together with the serviceClass, and adjust ServiceRegistryImpl
accordingly so one plugin cannot read or remove another plugin’s service of the
same type. Also implement getProvidedServices() to return the services actually
registered by this PluginContextImpl rather than an empty list.
- Around line 74-80: `PluginContextImpl` still leaves
`addPluginLifecycleListener` and `removePluginLifecycleListener` as no-ops, so
lifecycle listeners are never retained or dispatched. Wire these methods into
the existing listener storage used by `PluginContextImpl` so
`addPluginLifecycleListener` registers the listener and
`removePluginLifecycleListener` unregisters it, and make sure the lifecycle
dispatch path actually iterates over that stored set/list when plugin events
occur.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/fragment/PluginFragmentFactory.kt`:
- Around line 85-90: The fallback in PluginFragmentFactory.instantiate is
swallowing ClassNotFoundException by returning a plain Fragment(), which
bypasses PluginScreenActivity.loadPluginFragment()’s runCatching failure path
and can leave a blank screen. Remove or narrow this placeholder behavior so
instantiate() still surfaces the failure to callers, and if a placeholder is
needed, restrict it to the state-restoration path only in
FragmentFactory/PluginFragmentFactory.
- Around line 85-90: The fallback in PluginFragmentFactory.instantiate is
catching the wrong exception, so the placeholder Fragment() path never runs when
defaultFactory.instantiate(classLoader, className) fails at runtime. Update the
catch to handle Fragment.InstantiationException (and keep the log/fallback
there) so the actual AndroidX failure is intercepted; if this flow should remain
fatal like PluginScreenActivity, remove the placeholder and let the exception
propagate instead.
---
Nitpick comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/context/PluginContextImpl.kt`:
- Around line 61-68: The getAppSharedPreferences method currently catches
blanket Exception, which is too broad for this codebase’s fail-fast convention.
Update the try/catch around androidContext.getSharedPreferences to handle only
the specific exception type actually expected from this call path, and keep the
existing logger.error and null fallback behavior unchanged.
🪄 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: 1a1481ef-bd43-4ce5-b0ae-75e6684344f7
📒 Files selected for processing (19)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/itsaky/androidide/actions/PluginToolbarActionItem.ktapp/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/app/EditorProviderImpl.ktcommon/src/main/java/com/itsaky/androidide/ui/ProjectActionsToolbar.kteditor/src/main/java/com/itsaky/androidide/editor/ui/GhostTextRenderer.kteditor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kteditor/src/main/java/com/itsaky/androidide/editor/ui/TracingEditorRenderer.ktplugin-api/api/plugin-api.apiplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/PluginContext.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/UIExtension.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeServices.ktplugin-api/src/test/kotlin/com/itsaky/androidide/plugins/PluginContextTest.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/context/PluginContextImpl.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/fragment/PluginFragmentFactory.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeEditorServiceImpl.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeUIServiceImpl.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginToolbarHost.kt
…ess death When a plugin dialog fragment (e.g. the AI settings dialog) is restored from saved state after process death — before the plugin has reloaded and registered its classloader — the factory's fallback was meant to substitute a placeholder Fragment. It caught ClassNotFoundException, but FragmentFactory.loadFragmentClass wraps that in a Fragment.InstantiationException, so the catch never matched and the exception propagated into a fatal crash loop. Also catch Fragment.InstantiationException so the placeholder substitution actually runs.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/fragment/PluginFragmentFactory.kt (1)
85-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix correctly addresses the crash.
Catching
Fragment.InstantiationExceptionresolves the issue flagged in the previous review —defaultFactory.instantiate()wrapsClassNotFoundExceptioninFragment.InstantiationException, so the oldClassNotFoundException-only catch was dead code on the standard path. The placeholder fallback now fires as intended for process-death restoration.The
ClassNotFoundExceptioncatch (lines 90-93) is now redundant for the standard AndroidXFragmentFactorysince it wrapsClassNotFoundExceptionbefore throwing. It's only useful if a customdefaultFactoryimplementation throwsClassNotFoundExceptiondirectly. Consider adding a brief comment explaining why both catches are kept, or removing theClassNotFoundExceptioncatch ifdefaultFactoryis always the standard factory.Based on learnings, narrow exception handling that catches specific exception types is preferred over broad catch-all in this project. The changed code correctly uses narrow catches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/fragment/PluginFragmentFactory.kt` around lines 85 - 93, The fallback in PluginFragmentFactory.instantiate currently handles both Fragment.InstantiationException and ClassNotFoundException, but the ClassNotFoundException branch is redundant for the standard AndroidX defaultFactory path because instantiate() already wraps it; either remove the dead catch if defaultFactory is always the standard factory, or add a short comment in instantiate() explaining that both catches are kept only for custom defaultFactory implementations that may throw ClassNotFoundException directly.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/fragment/PluginFragmentFactory.kt`:
- Around line 85-93: The fallback in PluginFragmentFactory.instantiate currently
handles both Fragment.InstantiationException and ClassNotFoundException, but the
ClassNotFoundException branch is redundant for the standard AndroidX
defaultFactory path because instantiate() already wraps it; either remove the
dead catch if defaultFactory is always the standard factory, or add a short
comment in instantiate() explaining that both catches are kept only for custom
defaultFactory implementations that may throw ClassNotFoundException directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f3e0d06-674d-47d9-b98f-93c2ba5a2af1
📒 Files selected for processing (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/fragment/PluginFragmentFactory.kt
Introduce a thread-safe SharedServices registry so plugins can share service instances when PluginContext cross-plugin service access is unavailable. Backed by a ConcurrentHashMap with register/get/unregister/ clear plus reified extension helpers. Update the API baseline (plugin-api.api) for the new public API.
…plugin' into refactor/ADFA-4584-ai-hackathon-plugin
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/SharedServices.kt (2)
9-27: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftEnsure
clear()is invoked on plugin unload to avoid memory leaks.
SharedServicesis a global singleton holding strong references to service implementations. If those implementations retain plugin classloaders, activity contexts, or other heavyweight objects, failing to callclear()at the right lifecycle point will leak them across plugin reload cycles.Verify that
PluginManageror the plugin lifecycle shutdown path callsSharedServices.clear()(or selectivelyunregister) when plugins are unloaded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/SharedServices.kt` around lines 9 - 27, `SharedServices` keeps strong references in a global singleton, so make sure its cleanup path is wired into plugin shutdown. Update the plugin unload/lifecycle teardown in `PluginManager` (or the relevant unload hook) to call `SharedServices.clear()` when a plugin is unloaded, or use `unregister()` for targeted cleanup if only specific services should be removed. Verify the unload path runs reliably so service instances don’t survive reload cycles.
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConstrain
TtoAnyto prevent null registration at compile time.
Thas no upper bound, so callers could passnullasimplementation. Theas Anycast on line 13 would throw aNullPointerExceptionat runtime. AddingT : Anyto both the method and the reified extension shifts this to a compile-time error.♻️ Proposed fix
- fun <T> register(serviceClass: Class<T>, implementation: T) { + fun <T : Any> register(serviceClass: Class<T>, implementation: T) { services[serviceClass] = implementation as Any }And the reified extension:
-inline fun <reified T> SharedServices.register(implementation: T) { +inline fun <reified T : Any> SharedServices.register(implementation: T) { register(T::class.java, implementation) }Also applies to: 31-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/SharedServices.kt` around lines 12 - 14, Constrain the generic type in SharedServices.register to non-nullable by adding an Any upper bound so null implementations are rejected at compile time instead of failing at the cast. Update both register overloads in SharedServices, including the reified extension mentioned in the diff, so their T declarations use T : Any and the existing services map assignment no longer relies on a null-unsafe cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/SharedServices.kt`:
- Around line 9-27: `SharedServices` keeps strong references in a global
singleton, so make sure its cleanup path is wired into plugin shutdown. Update
the plugin unload/lifecycle teardown in `PluginManager` (or the relevant unload
hook) to call `SharedServices.clear()` when a plugin is unloaded, or use
`unregister()` for targeted cleanup if only specific services should be removed.
Verify the unload path runs reliably so service instances don’t survive reload
cycles.
- Around line 12-14: Constrain the generic type in SharedServices.register to
non-nullable by adding an Any upper bound so null implementations are rejected
at compile time instead of failing at the cast. Update both register overloads
in SharedServices, including the reified extension mentioned in the diff, so
their T declarations use T : Any and the existing services map assignment no
longer relies on a null-unsafe cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c79b2be0-5fb3-40f5-826f-98709a2892a8
📒 Files selected for processing (2)
plugin-api/api/plugin-api.apiplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/SharedServices.kt
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.
Description
Extracts the AI/agent functionality (chat assistant, LLM inference, native llama libraries, agent UI/actions) out of the main app and into standalone, optional plugins. To support that, the plugin API is expanded so plugins can do
everything the built-in features used to do — access project/file/build services, listen to editor content changes, render inline (ghost text) suggestions, contribute dynamic toolbar icons, and request mic input.
All API additions are additive with default no-op implementations, so binary compatibility with existing plugins is preserved (no interface renames, no removed signatures).
Details
agent/module (~50 classes), llama.cpp native libs, and the bundledllama.aar; cleaned up all dangling agent references (fragments, commands, actions, layout, bootstrap assets) and CI cache to fix the resulting build/D8 failures.PluginContextnow supports cross-plugin service registration/resolution and lifecycle listeners;IdeEditorServicegains content-change listeners +showInlineSuggestion/dismissInlineSuggestion;ToolbarActiongains aniconProviderandIdeUIService.refreshToolbarActionsfor state-driven animated icons;RECORD_AUDIOdeclared for voice plugins.GhostTextRenderer.GhostTextRenderernow extendsTracingEditorRendererso the single editor renderer keeps the ADFA-2468 block-line data-race guards while also drawing ghost text (bothonCreateRendereroverrides collapsed into one).plugin-api/api/plugin-api.api(additive only) soapiCheckpasses.Ticket
ADFA-4584
Observation
plugin-examplesrepo asai-core-plugin(LLM backend) andai-assistant-plugin(chat UI).