Skip to content

ADFA-3550: allow plugins to contribute Tier 3 documentation#1215

Merged
Daniel-ADFA merged 6 commits intostagefrom
ADFA-3550
Apr 21, 2026
Merged

ADFA-3550: allow plugins to contribute Tier 3 documentation#1215
Daniel-ADFA merged 6 commits intostagefrom
ADFA-3550

Conversation

@Daniel-ADFA
Copy link
Copy Markdown
Contributor

@Daniel-ADFA Daniel-ADFA commented Apr 20, 2026

Plugins can declare a Tier 3 docs asset path via the new DocumentationExtension.getTier3DocsAssetPath() override. On install, PluginDocumentationManager walks the plugin CGP assets and inserts them(after brotlli compresssing) into documentation.db under plugin//. Removal and verify-and-recreate mirror the existing Tier 1/2 lifecycle and plug into PluginManager's install/uninstall hooks.

Screen_recording_20260420_074857.webm

  Plugins can declare a Tier 3 docs asset path via the new
  DocumentationExtension.getTier3DocsAssetPath() override. On install,
  PluginDocumentationManager walks the plugin APK assets and inserts
  them into documentation.db under plugin/<pluginId>/. Removal
  and verify-and-recreate mirror the existing Tier 1/2 lifecycle and
  plug into PluginManager's install/uninstall hooks.
@Daniel-ADFA Daniel-ADFA requested a review from a team April 20, 2026 07:01
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Release Notes: Tier 3 Documentation Support for Plugins

New Features

  • Plugin-Declared Tier 3 Documentation: Plugins can now contribute Tier 3 documentation assets by implementing the new optional DocumentationExtension.getTier3DocsAssetPath() hook to declare an assets/ subdirectory
  • Automatic Asset Compression: Tier 3 documentation assets are automatically Brotli-compressed on plugin install and stored in the documentation.db under the plugin/<pluginId>/ namespace
  • Complete Plugin Documentation Lifecycle: Tier 3 documentation installation, removal, and verify-and-recreate operations are fully integrated with the existing PluginManager install/uninstall hooks
  • Improved URI Resolution: Updated documentation semantics for PluginTooltipButton.uri with proper scoping:
    • Relative paths are resolved to plugin/<pluginId>/<path>
    • Absolute paths (leading /) are normalized and treated as absolute within the local web server
    • URLs with :// are passed through unchanged

Implementation Details

  • Large Tier 3 assets (≥1 MB) are automatically chunked across multiple Content rows with -<fragment> suffixes
  • Plugin APK assets are accessed via a dedicated AssetManager instance restricted to the plugin's APK
  • File extensions are mapped to MIME types and content types via ExtensionToContentTypeResolver with built-in caching

⚠️ Risks and Best Practice Considerations

  • Silent Asset Failures: The Tier3AssetWalker silently skips assets that cannot be read due to I/O errors; failed assets are only logged as warnings and are not installed. This could lead to incomplete documentation without clear user feedback.
  • Per-Asset Size Limit: Individual Tier 3 assets are limited to 10 MiB. Assets exceeding this limit are logged and skipped silently rather than failing loudly.
  • Asynchronous Documentation Setup: Plugin documentation installation (Tier 1/2/3) now occurs asynchronously via coroutines. Timing-dependent operations or code relying on immediate documentation availability may be affected.
  • Database Transaction Atomicity: Tier 3 content replacement deletes all existing content for a plugin namespace and inserts new rows within a single transaction. A failed transaction could leave the plugin's documentation partially or completely absent.
  • Chunk Size Behavior: Payloads that are exact multiples of the 1 MB chunk size result in insertion of a zero-length terminator row, which may impact database footprint and query logic downstream.

Build and Dependency Changes

  • Added brotli4j dependency for compression support
  • Updated plugin-api build configuration: compileSdk = 36, minSdk = 28, Java 17 and Kotlin JVM 17 targeting

Walkthrough

Adds optional Tier‑3 documentation support: plugins can declare an APK-scoped assets subdirectory; manager installs/verifies/removes Tier‑3 assets into documentation DB with MIME resolution, optional brotli compression, chunked storage, and lifecycle hooks in plugin loading/unloading.

Changes

Cohort / File(s) Summary
Plugin API Extensions
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/DocumentationExtension.kt
Added fun getTier3DocsAssetPath(): String? (default null) and clarified PluginTooltipButton.uri resolution semantics (relative → plugin/<id>/..., leading / → stripped absolute local path, :// URLs passthrough).
Plugin Manager Core
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt
Refactored load/unload flow: added apkPath to LoadedPlugin, always register fragment/service and persist loaded plugin, early-return on failed init, added activateLoadedPlugin and async documentation install steps; remove Tier‑3 docs on unload.
Documentation Manager
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/PluginDocumentationManager.kt
New suspend APIs for Tier‑3: installPluginTier3Documentation, removePluginTier3Documentation, verifyAndRecreateTier3Documentation, isPluginTier3DocumentationInstalled. Implements APK-scoped AssetManager walk, extension→content-type resolution, optional Brotli compression, 1 MiB chunking with terminator rows, transactional replace, and tooltip URI normalization.
Compression Utility
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/BrotliCompressor.kt
New internal singleton BrotliCompressor using brotli4j; compress(input: ByteArray): ByteArray (quality 11, window 24).
Content Type Resolver
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/ExtensionToContentTypeResolver.kt
New resolver caching extension→ContentTypeRow(id, compression), uses MimeTypeMap fallback and DB ContentTypes lookup; caches misses.
Asset Walker
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/Tier3AssetWalker.kt
New Tier3AssetWalker.walk(assets, root) yields Tier3Asset(relativePath, bytes) sequence; enforces 10 MiB cap and skips unreadable/oversized assets.
Build / Gradle
plugin-manager/build.gradle.kts, plugin-api/build.gradle.kts
Added libs.brotli4j dependency; updated plugin-api Android/Kotlin JVM target settings (compileSdk 36, minSdk 28, Java/Kotlin target JVM 17).

Sequence Diagram

sequenceDiagram
    participant PM as PluginManager
    participant DocM as PluginDocumentationManager
    participant Assets as Tier3AssetWalker
    participant Resolver as ExtensionToContentTypeResolver
    participant Brotli as BrotliCompressor
    participant DB as SQLite Database

    PM->>DocM: verifyAndRecreateTier3Documentation(pluginId, plugin, apkPath)
    DocM->>DocM: open APK-scoped AssetManager
    DocM->>Assets: walk(assets, rootAssetPath)
    Assets-->>DocM: Tier3Asset(relativePath, bytes) sequence

    loop per Tier3Asset
        DocM->>Resolver: resolve(db, extension)
        Resolver->>DB: query ContentTypes by mime
        DB-->>Resolver: ContentTypeRow(id, compression)
        alt compression == "brotli"
            DocM->>Brotli: compress(bytes)
            Brotli-->>DocM: compressedBytes
        end
        DocM->>DB: insert Content rows (1MiB chunking, terminator row if needed)
    end

    DocM-->>PM: installation result (success/failure)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

Suggested reviewers

  • itsaky-adfa
  • jatezzz

Poem

🐰 I hopped through APKs by moonlight’s grin,
Gathered assets, tucked them safe within,
Brotli hummed while bytes grew small,
Chunked and stored — I did it all,
A rabbit’s patch, now docs stand tall.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: enabling plugins to contribute Tier 3 documentation, which is the primary objective of the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the new Tier 3 documentation feature, the asset path declaration mechanism, and how it integrates with the plugin lifecycle.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ADFA-3550

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
Copy Markdown
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: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (2)

205-217: ⚠️ Potential issue | 🟠 Major

Use apkPath when verifying docs for already loaded plugins.

LoadedPlugin.apkPath is added, but verifyDocumentationForLoadedPlugins() still only recreates Tier 1/2 docs. After documentation.db is recreated, the public manual verify path will leave Tier 3 rows missing for already loaded plugins.

Suggested fix
-        val pluginsWithDocs = loadedPlugins.values
+        val loadedDocsPlugins = loadedPlugins.values
             .filter { it.plugin is DocumentationExtension }
-            .associate { it.manifest.id to it.plugin as DocumentationExtension }
+        val pluginsWithDocs = loadedDocsPlugins
+            .associate { it.manifest.id to it.plugin as DocumentationExtension }
 
         if (pluginsWithDocs.isNotEmpty()) {
             logger.info("Verifying documentation for ${pluginsWithDocs.size} plugins")
             val recreatedCount = documentationManager.verifyAllPluginDocumentation(pluginsWithDocs)
+            loadedDocsPlugins.forEach { loadedPlugin ->
+                documentationManager.verifyAndRecreateTier3Documentation(
+                    loadedPlugin.manifest.id,
+                    loadedPlugin.plugin as DocumentationExtension,
+                    loadedPlugin.apkPath
+                )
+            }
             if (recreatedCount > 0) {
                 logger.info("Recreated missing documentation for $recreatedCount plugins")
             }

Also applies to: 1266-1272

🤖 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/core/PluginManager.kt`
around lines 205 - 217, verifyDocumentationForLoadedPlugins builds
pluginsWithDocs only from it.plugin and ignores the newly added
LoadedPlugin.apkPath, so verification recreates only Tier 1/2 docs and leaves
Tier 3 missing; update the map construction in
verifyDocumentationForLoadedPlugins to include each LoadedPlugin.apkPath (e.g.,
associate it.manifest.id to a data tuple/object containing both it.plugin as
DocumentationExtension and it.apkPath) and then call
documentationManager.verifyAllPluginDocumentation with that enriched payload (or
adjust that method signature to accept apkPath alongside the
DocumentationExtension); apply the same change to the other occurrence mentioned
so both code paths pass apkPath when invoking verifyAllPluginDocumentation.

429-454: ⚠️ Potential issue | 🟠 Major

Skip Tier 3 install when the documentation install hook declines.

verifyAndRecreateDocumentation() returns false when onDocumentationInstall() vetoes installation, but Line 443 still proceeds with Tier 3 installation. Preserve the existing lifecycle contract by gating Tier 3 on the Tier 1/2 result.

Suggested fix
+                                var documentationReady = false
                                 try {
                                     val docResult = documentationManager.verifyAndRecreateDocumentation(manifest.id, plugin)
+                                    documentationReady = docResult
                                     if (docResult) {
                                         logger.info("Documentation verified/installed for plugin: ${manifest.id}")
                                     } else {
                                         logger.warn("Failed to verify/install documentation for plugin: ${manifest.id}")
                                     }
                                 } catch (e: Exception) {
                                     logger.error("Error verifying/installing documentation for plugin: ${manifest.id}", e)
                                 }
 
+                                if (!documentationReady) {
+                                    return@launch
+                                }
+
                                 try {
                                     val tier3Result = documentationManager.verifyAndRecreateTier3Documentation(
                                         manifest.id, plugin, pluginApkPath
                                     )
🤖 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/core/PluginManager.kt`
around lines 429 - 454, The code currently always calls
documentationManager.verifyAndRecreateTier3Documentation(...) even when
documentationManager.verifyAndRecreateDocumentation(...) returned false (which
indicates onDocumentationInstall() vetoed install); update the coroutine block
so that after calling verifyAndRecreateDocumentation(manifest.id, plugin) you
only attempt verifyAndRecreateTier3Documentation(manifest.id, plugin,
pluginApkPath) when docResult is true (preserve existing try/catch/logging), and
when docResult is false skip the Tier 3 call and log a warning that Tier 3 was
skipped due to the primary documentation install being declined; reference
verifyAndRecreateDocumentation, verifyAndRecreateTier3Documentation, manifest,
pluginApkPath, and DocumentationExtension to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/DocumentationExtension.kt`:
- Around line 94-101: Documentation currently claims URIs containing "://" are
passed through unchanged, but ToolTipManager unconditionally prefixes stored
paths with "http://localhost:6174/", causing external URLs to break; fix by
changing the tooltip URL construction in ToolTipManager (the prefixing logic
around the code at/near line 133) to only prepend "http://localhost:6174/" when
the stored path is a local path (i.e., does not contain "://" and/or is not an
absolute server path), and update DocumentationExtension.kt wording to
explicitly state that only local/relative paths are prefixed while URIs
containing "://" are left unchanged.

In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/PluginDocumentationManager.kt`:
- Around line 214-215: Reject dot-segment/traversal in asset-relative paths
before concatenating into basePath: validate asset.relativePath in
PluginDocumentationManager (where basePath is built and insertContentChunked is
called) by normalizing the path (e.g., with java.nio.file.Path#normalize or URI
resolution) and ensure it does not start with "/" or contain ".." segments or
resolve outside the intended "plugin/<pluginId>/" namespace; if validation
fails, throw/log and skip storage. Apply the same validation to the other
occurrence around the insertContentChunked call referenced (the block at the
other location mentioned) so both uses of asset.relativePath are protected.
- Around line 286-289: The rawQuery in PluginDocumentationManager.kt uses
"plugin/$pluginId/%" directly in a LIKE pattern so pluginId characters '%' or
'_' act as wildcards; escape pluginId before building the LIKE parameter (or
validate/enforce allowed characters) and use an ESCAPE character in the SQL so
the bound parameter is treated literally. Update the rawQuery call(s) (the one
shown and the similar occurrences around lines 314-320) to sanitize pluginId by
replacing '%' and '_' (and the chosen escape char) with escaped versions and
pass the escaped parameter plus an ESCAPE clause in the SQL, or alternatively
validate pluginId against an allowed alphabet before running the query.
- Around line 356-362: The Content insert can silently fail because
SQLiteDatabase.insert(...) returns -1 on constraint errors; after calling
db.insert("Content", null, values) in PluginDocumentationManager (the block that
builds ContentValues with path, content/blob, contentTypeId, languageId), check
the returned Long and if it equals -1 throw an exception (e.g., SQLiteException
or SQLException) with a message including the path so the surrounding
transaction is aborted/rolled back; this ensures path collisions or schema
constraint failures do not leave the transaction marked successful and missing
Tier 3 chunks.

In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/Tier3AssetWalker.kt`:
- Around line 43-44: The code currently does val bytes = try {
assets.open(absolute).use { it.readBytes() } } which reads entire asset into
memory; replace this with a bounded streaming approach: implement a bounded
reader helper (e.g., readWithLimit or BoundedInputStream) and use
assets.open(absolute) to stream through the compressor/chunk insertion logic
instead of readBytes(), enforcing a per-file max size (and track a running total
cap for the whole install). If a file exceeds the per-file or total cap, abort
processing for that asset (log or throw a clear exception) so large plugin docs
cannot OOM the installer; update places referencing the bytes variable to accept
streamed/chunked input or to process chunks incrementally.

---

Outside diff comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 205-217: verifyDocumentationForLoadedPlugins builds
pluginsWithDocs only from it.plugin and ignores the newly added
LoadedPlugin.apkPath, so verification recreates only Tier 1/2 docs and leaves
Tier 3 missing; update the map construction in
verifyDocumentationForLoadedPlugins to include each LoadedPlugin.apkPath (e.g.,
associate it.manifest.id to a data tuple/object containing both it.plugin as
DocumentationExtension and it.apkPath) and then call
documentationManager.verifyAllPluginDocumentation with that enriched payload (or
adjust that method signature to accept apkPath alongside the
DocumentationExtension); apply the same change to the other occurrence mentioned
so both code paths pass apkPath when invoking verifyAllPluginDocumentation.
- Around line 429-454: The code currently always calls
documentationManager.verifyAndRecreateTier3Documentation(...) even when
documentationManager.verifyAndRecreateDocumentation(...) returned false (which
indicates onDocumentationInstall() vetoed install); update the coroutine block
so that after calling verifyAndRecreateDocumentation(manifest.id, plugin) you
only attempt verifyAndRecreateTier3Documentation(manifest.id, plugin,
pluginApkPath) when docResult is true (preserve existing try/catch/logging), and
when docResult is false skip the Tier 3 call and log a warning that Tier 3 was
skipped due to the primary documentation install being declined; reference
verifyAndRecreateDocumentation, verifyAndRecreateTier3Documentation, manifest,
pluginApkPath, and DocumentationExtension to locate the code.
🪄 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: efd117a4-05db-472f-9b46-e791b7796627

📥 Commits

Reviewing files that changed from the base of the PR and between 173d7f2 and 86a5fff.

📒 Files selected for processing (7)
  • plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/DocumentationExtension.kt
  • plugin-manager/build.gradle.kts
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/BrotliCompressor.kt
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/ExtensionToContentTypeResolver.kt
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/PluginDocumentationManager.kt
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/Tier3AssetWalker.kt

Copy link
Copy Markdown
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: 3

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

Inline comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 475-482: The code launches fire-and-forget IO coroutines which let
installs and uninstalls race (e.g., the CoroutineScope(Dispatchers.IO).launch
block calling runDocStep and
documentationManager.verifyAndRecreateTier3Documentation), so make documentation
install/removal per-plugin serialized: add a manager-owned CoroutineScope
(instead of ad-hoc CoroutineScope(Dispatchers.IO)), store a per-plugin
synchronizer (a Mutex or a per-plugin Job keyed by pluginId) on PluginManager,
and use that synchronizer in installPluginDocumentationAsync (the runDocStep
calls and documentationManager.verifyAndRecreateDocumentation /
verifyAndRecreateTier3Documentation) and in the uninstall/remove docs path so
uninstall awaits any ongoing install before deleting rows; ensure you
acquire/release the Mutex or chain Jobs around the runDocStep blocks to
serialize operations for the same pluginId.
- Around line 426-431: A plugin loaded while disabled returns before calling
activateLoadedPlugin, so later enablePlugin only calls plugin.activate and skips
contribution registration (docs, Tier 3 docs, snippets, build actions) performed
in activateLoadedPlugin; fix by reusing the same activation path: either have
enablePlugin call activateLoadedPlugin(loadedPlugin) instead of only
plugin.activate(), or extract the contribution-registration logic from
activateLoadedPlugin into a shared helper (e.g., registerContributions or
registerPluginContributions) and invoke that helper from both
activateLoadedPlugin and enablePlugin (affecting code paths around
activateLoadedPlugin, enablePlugin, loadedPlugin, manifest and plugin.activate;
also apply same change to the similar block mentioned for lines 442-455).
- Around line 490-500: The runCatching around block() currently swallows
CancellationException in the .onFailure { e -> ... } handler; update the
.onFailure handler used with runCatching in PluginManager (the runCatching {
block() } / .onSuccess / .onFailure sequence) to detect if e is a
CancellationException (or has one as a cause) and re-throw it immediately before
logging, so coroutine cancellation propagates correctly and structured
concurrency is preserved.
🪄 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: 2e3aefa9-ddb3-4c93-b3f1-4a7364f72761

📥 Commits

Reviewing files that changed from the base of the PR and between 7a3f840 and 534abc4.

📒 Files selected for processing (2)
  • plugin-api/build.gradle.kts
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt

@Daniel-ADFA Daniel-ADFA requested a review from jatezzz April 20, 2026 20:25
Copy link
Copy Markdown
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

♻️ Duplicate comments (2)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (2)

501-516: ⚠️ Potential issue | 🟡 Minor

Propagate coroutine cancellation from doc steps.

runCatching catches cancellation and this handler logs/suppresses it. Re-throw CancellationException before logging so doc verification remains cancellable.

Minimal fix
+import kotlinx.coroutines.CancellationException
+
 private suspend fun runDocStep(
     label: String,
     pluginId: String,
     block: suspend () -> Boolean
 ) {
     runCatching { block() }
         .onSuccess { result ->
             if (result) {
                 logger.info("$label verified/installed for plugin: $pluginId")
             } else {
                 logger.warn("Failed to verify/install $label for plugin: $pluginId")
             }
         }
         .onFailure { e ->
+            if (e is CancellationException) throw e
             logger.error("Error verifying/installing $label for plugin: $pluginId", e)
         }
 }
Does Kotlin runCatching catch CancellationException, and should coroutine cancellation be rethrown from coroutine error handlers?
🤖 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/core/PluginManager.kt`
around lines 501 - 516, In runDocStep, do not swallow coroutine cancellation:
detect and rethrow CancellationException before logging; update the onFailure
handler (or replace runCatching with try/catch) inside the runDocStep suspend
function so that if the caught exception is CancellationException it is rethrown
immediately, otherwise log the error with logger.error("Error
verifying/installing $label for plugin: $pluginId", e). Ensure you reference
runDocStep and its onFailure handler (or the block invocation) when making the
change.

491-498: ⚠️ Potential issue | 🟠 Major

Serialize documentation install/removal for the same plugin.

Install and unload still launch independent IO coroutines. An uninstall can remove documentation rows first, then an in-flight install can recreate plugin/<pluginId>/... rows after the CGP is deleted. Use a manager-owned scope plus a per-plugin Mutex/Job, and make removal wait behind any in-flight install.

Also applies to: 525-547

🤖 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/core/PluginManager.kt`
around lines 491 - 498, The documentation install/uninstall coroutines in
PluginManager are started directly with CoroutineScope(Dispatchers.IO).launch,
causing races between install and uninstall; change PluginManager to use a
manager-owned CoroutineScope and serialize per-plugin doc operations by tracking
a per-plugin Mutex or Job map keyed by pluginId (e.g., a MutableMap<String,
Mutex> or MutableMap<String, Job>) and wrap calls to runDocStep/
documentationManager.verifyAndRecreateDocumentation and
verifyAndRecreateTier3Documentation so that an uninstall acquires or awaits the
per-plugin lock/job before proceeding, and installs also acquire it while
running runDocStep("documentation", pluginId) and runDocStep("Tier 3 docs",
pluginId) to ensure removals wait behind in-flight installs and vice versa.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 461-483: When activation or subsequent registration fails, roll
back any partial contributions: track whether you registered a snippet (via
PluginSnippetManager.getInstance().registerPlugin), documentation
(installPluginDocumentationAsync), and build actions
(PluginBuildActionManager.getInstance().registerPlugin /
registerManifestActions) during the runCatching block, and in the onFailure
handler explicitly undo them (call
PluginSnippetManager.getInstance().unregisterPlugin(manifest.id), remove/cleanup
documentation via the corresponding uninstall/cleanup routine for
installPluginDocumentationAsync, and call
PluginBuildActionManager.getInstance().unregisterPlugin(manifest.id) and/or
remove manifest actions for manifest.id) before setting loadedPlugin.isEnabled =
false and savePluginState(manifest.id, false); also attempt to call any plugin
deactivation API if available (e.g., plugin.deactivate()) to ensure no active
state remains.

---

Duplicate comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 501-516: In runDocStep, do not swallow coroutine cancellation:
detect and rethrow CancellationException before logging; update the onFailure
handler (or replace runCatching with try/catch) inside the runDocStep suspend
function so that if the caught exception is CancellationException it is rethrown
immediately, otherwise log the error with logger.error("Error
verifying/installing $label for plugin: $pluginId", e). Ensure you reference
runDocStep and its onFailure handler (or the block invocation) when making the
change.
- Around line 491-498: The documentation install/uninstall coroutines in
PluginManager are started directly with CoroutineScope(Dispatchers.IO).launch,
causing races between install and uninstall; change PluginManager to use a
manager-owned CoroutineScope and serialize per-plugin doc operations by tracking
a per-plugin Mutex or Job map keyed by pluginId (e.g., a MutableMap<String,
Mutex> or MutableMap<String, Job>) and wrap calls to runDocStep/
documentationManager.verifyAndRecreateDocumentation and
verifyAndRecreateTier3Documentation so that an uninstall acquires or awaits the
per-plugin lock/job before proceeding, and installs also acquire it while
running runDocStep("documentation", pluginId) and runDocStep("Tier 3 docs",
pluginId) to ensure removals wait behind in-flight installs and vice versa.
🪄 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: 909b37bb-f5f9-43fd-bd94-aa0989db9a3a

📥 Commits

Reviewing files that changed from the base of the PR and between 534abc4 and 3826a0a.

📒 Files selected for processing (1)
  • plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt

@Daniel-ADFA Daniel-ADFA merged commit a8adf35 into stage Apr 21, 2026
2 checks passed
@Daniel-ADFA Daniel-ADFA deleted the ADFA-3550 branch April 21, 2026 16:34
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.

2 participants