ADFA-2602: Move the on-device toolchain to Gradle 9.6.1, AGP 9.3.1, Kotlin 2.3.21 - #1647
Open
Daniel-ADFA wants to merge 41 commits into
Open
ADFA-2602: Move the on-device toolchain to Gradle 9.6.1, AGP 9.3.1, Kotlin 2.3.21#1647Daniel-ADFA wants to merge 41 commits into
Daniel-ADFA wants to merge 41 commits into
Conversation
…otlin 2.3.21
Collapses the duplicate Kotlin compilers the device was shipping. Gradle
9.6.1 embeds Kotlin 2.3.21, and AGP 9's built-in Kotlin resolves the same
2.3.21 compiler, so the build-script compiler and the app compiler are one
artifact instead of two at different versions.
Version choice is constrained from both ends and is not free:
- AGP 9.3.1 requires Gradle 9.5.0+
- AGP 8.x fails on Gradle 9.6.0+ (it uses InternalProblems, removed there)
- Gradle's embedded Kotlin is fixed per version: 9.4.1 -> 2.3.0,
9.5.1 -> 2.3.20, 9.6.1 -> 2.3.21, 9.7.0 -> 2.4.0
Bumping agp-tooling to match the device forces a model migration, so it
lands here rather than separately -- splitting it would leave a commit that
does not compile. AGP 9 removed PrivacySandboxSdkInfo and
AndroidProject.PROPERTY_ANDROID_SUPPORT_VERSION (value inlined as
"android.injected.studio.version"), and added mappingR8TextFile,
mappingR8PartitionFile, keepRulesDirectories and aarKeepRulesDirectories.
app/build.gradle.kts now derives the bundled asset filenames from the
version constants instead of repeating "8.14.3" in six string literals.
Note the asset rename: builds resolve gradle-9.6.1-bin.zip, which must be
published to dev-assets before this lands or local assetsDownloadDebug
returns 404.
… opt-out consent (#1617) * ADFA-4942: Gate GlitchTip and Firebase analytics behind an onboarding opt-out consent * Fix spotless check * Fixes from PR review * fix strict mode violations on analytics manager --------- Co-authored-by: Daniel Alome <astrocoder007@gmail.com>
#1634) * ADFA-5035: Fix WebServer occasionally failing to start with EADDRINUSE start() binds serverSocket on a background thread (launched from MainActivity.startWebServer()); stop() (called from onDestroy(), main thread) only closes serverSocket if it's already initialized. If stop() runs before start() reaches bind(), it's a silent no-op -- start() then binds anyway a moment later, orphaned, holding the port until the process dies. The next start() attempt on that port fails with "Address already in use." Synchronize start()'s bind and stop()'s close on a shared lock, and have stop() record that a stop was requested so start() can abort before binding if one arrived first. Closes the race window instead of relying on timing. Also renamed HTTP_INTERNAL_SERVER_ERROR/HTTP_NOT_FOUND to camelCase (pre-existing ktlint property-naming violations, unrelated to this fix but required once this file falls under the Spotless ratchet). * ADFA-5035: Apply Spotless ratchet reformat to WebServer.kt WebServer.kt was space-indented and had several pre-existing ktlint violations (max-line-length, snake_case sql_query). Touching the file in the previous commit pulled it under the Spotless ratchet, so bring it into compliance: tabs, wrapped long lines/comments, and sql_query -> sqlQuery. No behavioral change. * ADFA-5035: Address code review findings - Close database in start()'s finally alongside serverSocket. It was opened before the stopRequested check that can now abort start() early, and was never closed on any other shutdown path either (normal accept-loop exit, exception) -- isInitialized guards the case where opening it failed and this finally still runs. - Correct stop()'s doc comment: it's no longer a full no-op before start() binds -- it still records the stop request so start() can abort before binding, which is the fix itself. Only the socket-close side stays a no-op in that case. (Reverting the behavior back to a literal no-op, as literally suggested, would reopen the exact EADDRINUSE race this ticket fixes.) - Add WebServerTest: deterministic coverage for both lifecycle orderings (stop-before-start aborts the bind; start-then-stop frees the port for reuse), synchronized via the port's own bind/connect behavior rather than fixed sleeps. Skipped: the outputStarted-timing suggestion for realHandleBsEndpoint/ realHandlePrEndpoint is the same CodeRabbit finding already considered and explicitly rejected in an existing code comment ("I disagree... --DS, 23-Feb-2026"); out of scope to unilaterally revisit here. * ADFA-5035: Fix outputStarted timing in handleBsEndpoint/handlePrEndpoint outputStarted was only set after realHandleBsEndpoint/realHandlePrEndpoint returned, so if writeNormalToClient threw partway through (e.g. after the status line but mid-body), the catch block still saw outputStarted=false and sent a second, well-formed response on top of the already-partially-written one. Pass a markOutputStarted callback into both functions and invoke it right before the first write, so the caller's flag reflects reality even when the write itself then fails. Removes the "I disagree with CodeRabbit's message" comment that had left this finding unaddressed.
Daniel-ADFA
marked this pull request as ready for review
August 11, 2026 13:59
![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.
itsaky-adfa
approved these changes
Aug 11, 2026
| // POM stays dependency-free: forcing it as a transitive would make the coordinate | ||
| // unresolvable offline whenever the harvested AGP differs from a pinned version. | ||
| compileOnly("com.android.tools.build:gradle:8.11.0") | ||
| compileOnly("com.android.tools.build:gradle:9.3.1") |
Contributor
There was a problem hiding this comment.
Re-use the lib.versions.agpTooling property here - it is not recommended to define version numbers at multiple sites.
itsaky-adfa
requested changes
Aug 11, 2026
itsaky-adfa
left a comment
itsaky-adfa
left a comment
Contributor
There was a problem hiding this comment.
There's one minor issue to be fixed (see the previous review).
* feat(ADFA-4692): Detect project language dynamically * test(ADFA-4692): Test project language detection * fix(ADFA-4692): Avoid localized strings for unknown project language * feat(ADFA-4692): Preserve coroutine cancellation * test(ADFA-4692): Add missing test cases
|
also there's a issue when assembleRelease the apk with AGP 9.3.1 and Gradle 9.7.0 |
After doing some tests I found that the problem had nothing to do with AGP or Gradle |
* ADFA-5097: Clean up telemetry choice screen The consent dialog was a stock MaterialAlertDialog built from setTitle + setMessage + three buttons. Every complaint in the ticket was that widget's behavior once the message overflowed at large font scale: Material draws scroll-indicator dividers around the message pane, the borderless text buttons read as plain text, and the message scrolls while the button bar stays fixed. On the reported device the button bar was pushed off-screen entirely, leaving "Keep offline" half-cut and "Learn more" invisible - the decline option was unreachable on a non-cancelable dialog. Styling could not fix that, so the dialog now takes a custom view holding the body and both choices in one NestedScrollView: - No dividers. Material only draws them around the message pane, and there is no longer a message. - Choices are real filled/outlined MaterialButtons, full width, stacked. - Body and choices scroll together, and the scrollbar is non-fading so scrollability is visible before the user touches anything. - The choice panel sits on colorSurfaceVariant to set it off from the body. Not a surfaceContainer role: this app's themes define colorSurfaceVariant but leave the container roles to the Material3 defaults, and a Material3 dialog is itself colorSurfaceContainerHigh, so such a panel would be invisible. Copy condensed to two paragraphs, dropping the Firebase/GlitchTip names and the privacy-policy paragraph. At font scale 2.0 it now fits without scrolling at all. "Learn more" is removed. It opened an off-device PDF via ACTION_VIEW - exactly what the app tries to avoid - and the content is legal boilerplate already on the website. privacy_policy_url had no other consumer and is deleted too. Also retranslates the zh-rCN and in-rID strings, which were not merely stale: both were missing privacy_disclosure_decline entirely and rendered "accept" as "I understand" / "Saya mengerti", so those users saw a one-button dialog whose button said something the English never said. Verified on an arm64 emulator at font scale 1.0 and 2.0, plus 3.0 to force overflow and confirm the scrollbar appears and both buttons scroll into reach. Light and dark themes checked; accept persists GRANTED and decline persists DECLINED. * ADFA-5097: Italicize the choice names in the consent body Paragraph 2 names the two choices; setting them in italics makes them stand out and hints at the buttons below. Inline <i> markup in the string resource. The layout binds via android:text, so TextView picks up the style spans directly - getString() would have stripped them, but nothing here calls it. Not applied to zh-rCN: synthetic obliquing of Han characters renders poorly, and that translation already sets the two choice names off with the conventional CJK quotation marks.
Replace the hardcoded AGP 9.3.1 in plugin-builder's compileOnly with a tooling-agp catalog alias so the version is defined once. plugin-builder is also an included/standalone build, so import the root catalog in its settings (same pattern as composite-builds/build-logic).
* feat(plugin-api): add remote-peer editor decoration API Add IdeEditorService.addRemotePeerMarker/removeRemotePeerMarker/clearRemotePeerMarkers as default-implemented (backward-compatible) methods so a plugin can draw a remote collaborator's caret/badge inside the editor. Backed by a new EditorDecorationManager + RemotePeerMarkerWindow (an EditorPopupWindow overlay that tracks scroll via FEATURE_SCROLL_AS_CONTENT) in the app module; EditorProviderImpl resolves the live editor via EditorHandlerActivity.getEditorForFile and marshals onto the main thread, clearing markers on dispose. IdeEditorServiceImpl exposes read-gated overrides and the parallel EditorProvider contract methods. The new interface methods are default-implemented so this is an additive, non-breaking change for the generated plugin-api lib and existing implementers. Consumed by the Pair pair-programming plugin. * WIP: Add cursor markers to plugin * Implement pair programming plugin * fix(ADFA-4419): remove stray token breaking PluginManager compilation * refactor(ADFA-4419): distinguish peer-presence overlay from #1448 decorations Rename EditorDecorationManager -> PeerPresenceOverlayManager and extract a focused PeerPresenceProvider interface out of the broad EditorProvider, so the pair-programming peer-cursor overlay (floating named badges) reads as a distinct concern from the generic EditorDecorationProvider (additive color spans) added in #1448 (ADFA-4436). Host-internal only: no plugin-api contract changed and the merged rainbow- brackets plugin is unaffected. Verified with :app:compileV8DebugKotlin. * fix(ADFA-4419): address CodeRabbit review + drop dev-trace logging - PeerPresenceOverlayManager: clamp peer badge on exact-fit width (maxX >= 0) - PluginManager.loadPlugins: rethrow CancellationException instead of recording cancellation as a plugin load failure - IdeEditorServiceImpl: don't gate hidePeerCursor/clearPeerCursors on file accessibility, so overlay cleanup still works after a tab closes - IdeProjectServiceImpl.openProject: use the validated canonical path and run it through PathValidator before switching projects - PluginRepositoryImpl: delete the broken artifact when an upgraded plugin fails to load, so loadPlugins() doesn't keep retrying it - Drop PairTrace / [HOST] dev-trace Log.d (kept warn/error diagnostics) * ADFA-4419: Fix spotless/ktlint violations in branch-touched files spotlessApply reformatting across the seven files this branch touched, plus the three lints ktlint cannot auto-fix: expand the wildcard import in PluginManager, move the orphaned KDoc onto delegatingEditorProvider, and rename INSTANCE/Loader to instance/loader per property-naming. --------- Co-authored-by: Daniel Alome <astrocoder007@gmail.com>
…#1661) * ADFA-5107 | Add Claude-facing reference for the documentation database Combines the Confluence design doc and docdb-studio's CLAUDE.md/SCHEMA.md into docs/documentation-database.md, cross-checked against WebServer.kt, ToolTipManager.kt, and PluginDocumentationManager.kt so it reflects actual behavior. Linked from ARCHITECTURE.md and ADR 0001 for discoverability. * ADFA-5107 | Fix docdb doc inaccuracies flagged in review - List all three raw-SQLite exceptions (tooltips, in-app/plugin-help, local web server) as documentation.db consumers in ARCHITECTURE.md, not just two. - Stop calling Tier 3 content "tooltips" -- tooltips are Tier 1/2; Tier 3 is the web content they link to. - Fix a self-contradiction: Content.UNIQUE(path) rejects any duplicate path regardless of languageID, it does not permit same-path rows that differ only by language. One passage said otherwise.
* fix(ADFA-4852): Fix dialog dismissal on long-press * docs(ADFA-4852): Add KDoc to function * refactor(ADFA-4852): Show dialog before configuring its views
…lytics and GlitchTip (#1663) * ADFA-4394: Report attached input devices and external displays to analytics and GlitchTip * ADFA-4394: Address CodeRabbit review on attached devices metric Narrow the collector guards to RuntimeException and log fallbacks, move metric collection and tracking off the main dispatcher, and assert the exact attached_devices context map in the test. --------- Co-authored-by: Daniel Alome <astrocoder007@gmail.com>
ADFA-5109 | Remove unused Room deps from idetooltips ToolTipManager reads documentation.db via raw SQLiteDatabase, not Room (ADR 0001, exception 1) -- no source file in the module imports androidx.room. Drops the now-pointless kapt(room.compiler)/room.ktx deps and the kotlin-kapt plugin they were the only user of, and closes out the ADR 0001 follow-up that flagged this.
* ADFA-5123: Document docdb SQL-authoring gotchas from ADFA-5088 Add two lessons learned writing SQL migration scripts against documentation.db: a local copy's on-disk schema/content can be stale independent of git history (a stale copy caused a real near-miss in ADFA-5088 - a script validated against it would have silently overwritten curated production content), and the .bail on / guard-table pattern needed for a BEGIN/COMMIT script to actually be atomic against a bad or empty Brotli payload. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5123: Document the owner-only-workdir temp-file pattern Insecure /tmp filenames (CWE-377) were found and fixed in the ADFA-5088 docdb scripts after this doc's first pass - a fixed, guessable name under world-writable /tmp lets another local user pre-plant a symlink or race the write/read pair. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…1668) UseSytemShell was never instantiated anywhere - no addPreference(UseSytemShell()) call exists in any screen builder, so it was unreachable from the Preferences UI. The setting it read/wrote (GeneralPreferences.useSystemShell / TERMINAL_USE_SYSTEM_SHELL) was likewise never consulted anywhere else in the codebase. Remove the class, the backing constant and property, and the title/summary strings across all 13 locale files (used only by this row). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…b SQL) (#1667) * docs: retro for ADFA-5088 (Preferences/Plugin Manager tooltips + docdb SQL) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: add insecure-temp-file learning from ADFA-5088 follow-up Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: address review feedback on retro docs - learnings.md: note that mkdir can itself fail the same way .bail can't see other .system failures, and that mktemp -d isn't a drop-in fix here since each .system line is its own subshell with no state carried to the next one. - retrospective.md: add the blank lines markdownlint (MD058) wants around the three new tables. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
itsaky-adfa
approved these changes
Aug 14, 2026
Users can't find our instructional videos on YouTube/Bilibili since we don't rank near the top in search. Add both channels to the About page's Socials section so they're one tap away. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…e contract (#1660) * feat(ADFA-5095): Let an LLM backend own its prompt, config and tool calling An LlmBackend could only stream a single prompt, so anything model-specific -- prompt wording, temperature, settings UI -- had to be guessed by the consumer or read out of another plugin's preferences. Add, all as defaults so existing backends are untouched: - generateStreamingWithHistory / generateStreamingWithTools, degrading to plain streaming rather than failing - getSystemPrompt(SystemPromptRequest) and getDefaultTemperature: the consumer supplies the tool contract, the backend supplies the wording - getConfigSpecs (ConfigFieldSpec / ConfigFieldType) or getSettingsFragmentClassName for a backend that owns its own settings screen - CancellableBackend for stopping an in-flight stream - LlmInferenceService.getPreferredBackendId, so a costly backend can tell whether it is the one about to be used * fix(ADFA-5095): Annotate nullability and document the tool-calling models getDefaultTemperature() returns a boxed Float where LlmConfig.temperature is primitive, so the obvious assignment unboxes null and throws, now annotated and stated in the Javadoc, along with the four other unannotated members. * refactor(ADFA-5095): Declare backend capabilities by type, not by flag Review on #1660: capability interfaces replace the flag/method pairs that could disagree, tool results get a return path, describe-only config specs drop ToolDefinition and ToolCallRequest turn immutable, and :plugin-api's tests compile for the first time. * fix(ADFA-5095): Keep the tool models mutable and log the additions The last commit tightened ToolCallRequest and ToolDefinition to final fields with defensive copies. Both shipped non-final in 26.28, so that turned an additive PR into an ABI break: an already-built .cgp assigning one of those fields throws IllegalAccessError on putfield, and the null-checks and unmodifiable maps change behaviour for callers that were within contract before. Reverted to the published shape; the hazard the tightening was aimed at is now an ownership rule in the Javadoc instead -- who owns an instance, and that SystemPromptRequest copies only the list spine. Also adds the 26.33 changelog entry the additions were missing, so a plugin author has a min_ide_version to floor at, including the note that Role.TOOL can break an exhaustive Kotlin `when`. * docs(ADFA-5095): Correlate tool results by call id and tool name * docs(ADFA-5095): File Role.TOOL and the nullability sweep as breaking Both need a source change in plugin repos, so drop the blanket "every change is additive" claim, define the `breaking` legend entry, and document the previously unlisted nullability change.
* ADFA-5156: Roll back R8 shrinking to unbreak plugins Restores the blanket -dontshrink that ADFA-3604 (#1596) removed on 2026-07-29. This is a temporary rollback to restore plugin functionality; a targeted fix follows. Plugins are loaded parent-first through a stock DexClassLoader (PluginLoader.kt:92-116, parent passed at PluginManager.kt:603), so every kotlin.** class a plugin references resolves from the IDE's dex, not from the ~1058 stdlib classes the plugin bundles. R8 cannot see plugin call sites, so it strips every stdlib member the IDE itself does not call. The net effect is that a plugin can only call the subset of the Kotlin standard library that the IDE also calls; anything else throws NoSuchMethodError at runtime. Sketch to UI fails on every image load with "No static method maxOrNull([F)Ljava/lang/Float; in class ArraysKt". -dontobfuscate and -dontoptimize were already set, so restoring -dontshrink reduces R8 to a pass-through and returns the release build to the configuration shipped before ADFA-3604. isMinifyEnabled and isShrinkResources are deliberately left alone, keeping resource shrinking and the build wiring unchanged. Verified by dex-scanning both APKs (baseline pulled from a release install on Samsung RFCT704HEAL): kotlin/kotlinx method declarations 30,669 -> 45,371 ArraysKt/CollectionsKt/MapsKt/ FilesKt/SequencesKt facades absent -> present 10 sketch-to-ui stdlib call sites all stripped -> all present CompletableJob$DefaultImpls.plus stripped -> present Sketch to UI now loads an image and completes detection on-device with no NoSuchMethodError in logcat. APK size: 659,307,160 -> 706,829,817 bytes (+47.5 MB, +7.2%). Note: R8 was buying less than #1596 advertised. That PR measured dex at 28.8 MB / 24,853 classes, but the shipped APK is 85 MB / 78,757 classes -- the URGENT follow-ups (#1609, #1610) added -dontoptimize plus a set of keep rules that clawed most of it back. * ADFA-5156: Add R8 plugin-impact analysis tooling The ADFA-5156 failure mode is invisible at build time -- assemblePlugin is green, the manifest is fine, the .cgp is correct, and only on-device execution of a specific code path reveals that R8 stripped a stdlib member the plugin needs. These scripts make it measurable from build artifacts instead. scripts/r8-plugin-impact/ README.md what the bug is, how to run, how to read output, the known false positives, and the ADFA-5156 baseline numbers to measure future builds against dex-dump.sh extract + dexdump an APK or .cgp analyze-plugin-impact.py simulate parent-first resolution of every kotlin.*/kotlinx.* call site in each plugin's own code against two host dexes and diff the verdicts Three subcommands: impact (the before/after table), explain-method (trace one resolution chain, showing where it leaves the APK), explain-absent (inspect fall-throughs for the split-brain shape that caused the original bug). Documents three traps that produce wrong conclusions if analysis is done ad hoc, all of which bit during this investigation: - Methods inherited from the Android boot classpath read as missing, because java.util.* is not in the APK. Eight such false positives are enumerated. - Kotlin multifile facades (ArraysKt, StringsKt) declare nothing themselves; they extend a part class whose underscore count varies (StringsKt__StringsKt vs ArraysKt___ArraysKt). Checking a facade directly always fails. - D8 build-time synthetics ($$ExternalSyntheticBackport0 and friends) never exist in the host and always show as absent. Stdlib-only Python, no third-party dependencies. Run with uv run --no-project. * ADFA-5156: Apply spotless formatting to plugin-impact scripts * ADFA-5156: Point plugin-impact baseline at the deployed plugin set The first baseline measured a local folder of .cgp files that turned out to be a pre-rename snapshot -- 5 stale filenames and 3 plugins missing. Re-runs against the artifact from the last update-libs.yml deploy (26 plugins, 4,261 call sites) and documents how to obtain that artifact, so the next person does not measure the wrong set. Conclusion is unchanged: zero regressions, 67 real failures to 0.
* fix(ADFA-4808): Re-sync app log output * fix(ADFA4808): Prevent logs from reappearing after clearing * FIX(ADFA-4808): Return an append failure to the log renderer * fix(ADFA-4808): Exception handling
* fix(ADFA-5177): Exclude Unknown from template languages * refactor(ADFA-5177): Enforce language exclusion after config
…1682) * ADFA-4934: Introduce shared PLUGIN_ARCHIVE_EXTENSION constant Consolidates the ".cgp" literal duplicated across ~7 sites into a single constant, mirroring the existing TEMPLATE_ARCHIVE_EXTENSION. Prep work for the external file-install feature, which needs a canonical way to recognize .cgp files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Install a .cgp or .cgt file opened from outside the app Adds a VIEW intent-filter (ExternalFileInstallActivity) so opening a .cgp/.cgt attachment (e.g. from email) prompts to install it, instead of doing nothing. .cgp files are copied to a temp file and forwarded into PluginManagerActivity, reusing its existing install/conflict/signature-check flow verbatim rather than duplicating it. .cgt files get a new TemplateCollectionRepository, since no import/conflict backend existed for template collections before now: it validates the archive via the existing ZipTemplateReader, and on a filename collision offers overwrite / rename-and-install / ignore (the ticket's requested UX), using the archive's filename as its identity since templates.json has no collection-level name field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Split file:// intent-filter into typed/untyped variants On-device testing (dumpsys package) showed a mimeType on any <data> tag applies to the WHOLE intent-filter, not just that tag - so a single filter mixing content's mimeType-bearing variant with file's mimeType-less variant silently broke matching for untyped file:// intents (confirmed via `pm query-activities`: 0 matches before this fix, 2 after). content:// keeps a single filter (the OS resolves an implicit type for it regardless), but file:// now gets two dedicated filters, one typed and one not. Verified end-to-end on a physical device: the "Open with" chooser lists Code on the Go for both .cgp and .cgt, and the full install/conflict-resolve flow (fresh install, rename, overwrite, invalid-file rejection) works. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Rebuild the .cgt install dialogs in Jetpack Compose ADR 0009 requires new dialogs to be Compose, not MaterialAlertDialogBuilder - caught by an architecture-review pass before opening the PR. Enables Compose in the app module (mirroring floating-window's setup) and rewrites the three .cgt dialogs (install-confirm, name-conflict, rename) as composables, reusing FloatingTheme so they stay visually consistent with the IDE's XML theme. The .cgp path is untouched: it still forwards into PluginManagerActivity's existing (pre-ADR) dialog rather than duplicating it. Fixed two things surfaced by this rewrite: - compose-rules ktlint caught the ViewModel being forwarded into a nested composable; fixed via state hoisting (a plain suspend lambda instead). - The rename dialog's suggested name no longer visually clips its first character - that was a View EditText auto-scroll artifact from selectAll(), gone now that Compose's TextFieldValue sets the cursor position explicitly. Re-verified end-to-end on the physical device: fresh install, rename, overwrite, and the .cgp forwarding path all work with the new dialogs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Dedupe FileProvider authority; fix manifest case/coverage gaps Code review (PR #1682) findings, mechanical/data half: - The app's FileProvider authority string ("<packageName>.providers.fileprovider") was duplicated inline across 7 call sites (IntentUtils, ApkInstaller, FileDragStarter, DragAndDropExtensions, FeedbackManager, FeedbackEmailHandler, and the new IDEFileProvider helper) - a rename would have needed 7 manual updates with no compiler check. Consolidated into common/FileProviderUtils.kt, shared across app and common (which can't depend on app's IDEFileProvider). - Manifest: android:pathPattern has no case-insensitive mode, so a .CGP/.CGT (uppercase) attachment previously never matched. Added uppercase variants, and combined .cgp/.cgt into 3 shared filters (down from 6) since every <data> tag within one filter already had to share the same mimeType-bearing shape. Documented, as an explicit known limitation, that a sender whose content:// Uri path never carries the filename (e.g. some email providers' attachment Uris) can't match a pathPattern-based filter regardless of type - the alternative (a pathPattern-less mimeType="*/*" filter) would register this app as a candidate for every file-view intent on the device, which is a worse tradeoff than missing those senders. Verified on a physical device: `pm query-activities` now matches both cases of both extensions via content:// and file://, typed and untyped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix correctness bugs found by code review (PR #1682) Behavioral half of the review findings: - "Delete installation file after install" silently did nothing for a .cgp forwarded from ExternalFileInstallActivity: DocumentsContract.deleteDocument() only works against a real SAF DocumentsProvider (it calls a special METHOD_DELETE_DOCUMENT via ContentProvider.call(), returning true unconditionally unless an exception is thrown), and our own IDEFileProvider doesn't implement that call. Now dispatches to plain contentResolver.delete() for our own authority (confirmed via decompiling FileProvider.class that its delete() correctly deletes the mapped file) and keeps deleteDocument() for real picker-sourced Uris. Forwarded installs also no longer show the checkbox at all - there's no source worth optionally keeping, since it's our own hidden temp copy - so it's now always cleaned up. - Cold-start race: isPluginManagerAvailable()/isTemplatesFeatureAvailable() could run before IDEApplication's async setup finishes if the OS cold-starts straight into ExternalFileInstallActivity. Both are now polled briefly (up to 3s) instead of failing on the first check. - Two process-death drops: ExternalFileInstallActivity and PluginManagerActivity both only acted `if (savedInstanceState == null)`, which also (incorrectly) skips a process-death-recreated instance - the one case that most needs to reprocess the restored intent, since it lost all in-memory state. Replaced with idempotency tracked inside each ViewModel instance (survives rotation, resets on process death, matching real recreation semantics). - Rename dialog: an in-flight async name suggestion could clobber whatever the user had already started typing. - installCollection()'s own collision check was case-sensitive, bypassing findExistingCollision()'s case-insensitive matching - both now share one lookup. - A failed template install used to delete the temp file and close the screen, forcing the user to re-open the original attachment to retry. Failure now just shows an error and leaves the current dialog open. - Wired ShowTemplateNameConflict.info into the conflict dialog instead of dropping it silently (it now shows contained template names, matching the fresh-install dialog). - Hardened temp/session file naming from timestamp to UUID (collision risk under rapid concurrent opens), moved UriFileImporter.getDisplayName() onto Dispatchers.IO (was running unguarded on Main), and stopped conflating CancellationException with real copy failures (now always cleans up the partial file either way, and doesn't show a bogus error for an ordinary cancellation). - installCollection() also tried File.renameTo() as an "atomic move" - this round-tripped through on-device testing: it silently fails on this device even within the app's own private storage (a well-known Android unreliability), so a real .cgt install regressed to always failing until a copy+delete fallback was added back. Re-verified end-to-end on a physical device after each fix: fresh install, rename, overwrite, and the delete-checkbox's absence for forwarded installs all confirmed working; the retry-after-failure behavior was directly triggered and observed holding the dialog open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix correctness/reuse findings from max-effort code review Correctness: - TemplateCollectionRepositoryImpl: refuse to install/overwrite the reserved "core" basename so an external .cgt named "core" can no longer delete the bundled default templates archive; wrap findExistingCollision() in try/catch like its siblings. - ExternalFileInstallViewModel: guard confirmTemplateInstall() against a double-tap race; give the Flashbar entrance animation time to render before Finish tears the activity down. - Add uiMode/locale/fontScale/density to both activities' configChanges so a config change mid-dialog can't strand the forwarded-install flow behind a one-shot guard that already fired. - PluginManagerViewModel: clean up the forwarded source file on install failure, a conflict abort, or the user cancelling either confirmation dialog - not just on success. Reuse/simplification: - Forward a .cgp as a plain file path instead of a minted FileProvider Uri, so PluginManagerViewModel can install directly from it instead of copying it a second time; drops the now-redundant IDEFileProvider.getUriForFile wrapper. - Replace Uri-authority sniffing in deleteSourceDocument() with an explicit PluginInstallSource (ContentUri vs LocalFile) from the caller. - Extract a shared LastValueGate for the two "run at most once per forwarded value" guards that were previously duplicated with slightly different shapes. - Dedupe the template-name joinToString() formatting between dialogs. - Wire long-press help into ExternalFileInstallScreen.kt via a small reusable Compose/idetooltips interop helper, per ADR 0009/REVIEW.md guidance for a first Compose screen ahead of the ADFA-4381 bridge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Address architecture-review findings - Use collectAsStateWithLifecycle() instead of collectAsState() per ADR 0009's explicit guidance, adding the lifecycle-runtime-compose dependency it calls for (the app had none yet). - Update ARCHITECTURE.md's PluginManagerUiEvent.InstallPlugin example to match the PluginInstallSource change from the prior commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Address CodeRabbit review findings - findCollisionFile: match the .cgt extension case-insensitively, consistent with the already-case-insensitive base-name match (the manifest accepts uppercase .CGT); added a regression test. - findExistingCollision: rethrow CancellationException instead of swallowing it as a null result, preserving coroutine cancellation. - longPressTooltip: add an onLongClickLabel (new cd_show_help string) so screen readers can discover the long-press help action. - LastValueGate: document that consume() is not thread-safe. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Address remaining CodeRabbit findings on PR #1682 - Reject path-traversal in TemplateCollectionRepositoryImpl.installCollection (targetBaseName can no longer contain a separator, and the resolved path is verified to stay directly under templatesDir). - Stage the incoming archive fully under templatesDir before deleting an existing collection, so a failed write can no longer destroy it. - Rethrow CancellationException before the broad catch in PluginManagerViewModel.installPlugin, and run its temp-file cleanup under NonCancellable, matching the pattern already used elsewhere. - Switch the two new files' logging (ExternalFileInstallViewModel, TemplateCollectionRepositoryImpl) from android.util.Log to SLF4J, per REVIEW.md's logging convention. - Add unit tests for FileProviderUtils, the new path-traversal guard, and the preserve-existing-on-failed-write behavior. - Use TemporaryFolder instead of the real Robolectric filesDir for ExternalFileInstallViewModelTest's output files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from fresh CodeRabbit review on PR #1682 - Rethrow CancellationException in inspectCollection and installCollection (both used runCatching, which was swallowing it into a Result.failure). - Fix a retry-ability regression the previous commit's staging fix introduced: installCollection now copies (rather than moves) the candidate into staging and only deletes it after the whole install succeeds, so a failed install leaves the source file intact for the caller to retry with the same file. - Add KDoc to TemplateCollectionRepositoryImpl and FileProviderUtilsTest. - Broaden test coverage: uppercase-collision install/overwrite, the remaining invalid targetBaseName cases (backslash, ".", blank), byte- content assertions on install/overwrite, and a retry-ability test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from max-effort code review of PR #1682 - PluginRepositoryImpl: case-insensitive .cgp extension check, fixing permanent data loss for uppercase-named plugin files. - PluginManagerViewModel: await the first loadPlugins() completion before checking for a same-ID conflict, closing a race that could skip the signature check on a cold-started install; only ever delete a forwarded LocalFile temp copy on decline/failure, never a user-picked ContentUri (matches the "delete after install" checkbox's success-only meaning); corrected a comment that overstated the deletion invariant. - PluginManagerActivity: route back-press/tap-outside through CancelPendingInstall on both install dialogs, so a forwarded temp file is never leaked by a silently-cancelable dialog. - TemplateCollectionRepositoryImpl: replace the existing collection via a backup-swap-restore instead of delete-then-write, so a failed final copy can no longer destroy it; give staging/backup files unique names so concurrent installs of the same collection don't race on the same path. - TemplateProviderImpl: case-insensitive .cgt scan, matching the repository's case-insensitive install/collision handling. - AndroidManifest: add keyboard/keyboardHidden/navigation to both install-flow activities' configChanges, closing the same dialog-dropped-on-recreation class of bug for another config axis. - ExternalFileInstallScreen: disable dismiss/cancel on all three dialogs while an install is in flight, so a fast tap can't race a delete against the in-progress install; new Flashbar await-shown helpers replace a fixed delay with the real animation-complete signal before finishing the activity. - ExternalFileInstallViewModel: widen the setup-wait budget from ~2.7s to ~8s to better match a real cold-start's unbounded init chain. Deferred: TemplateProviderImpl's per-archive parse errors are still only logged, not surfaced to installCollection's caller - closing that requires exposing per-archive load state across the templates-api/impl module boundary, which is disproportionate for this PR relative to how speculative the failure mode is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from high-effort code review of PR #1682 - TemplateCollectionRepositoryImpl: escalate (rather than discard) a failed backup restore after a swap failure, and no longer report a spurious install failure when only the post-swap provider reload throws - the file swap is the operation's real postcondition. - ExternalFileInstallViewModel: cap suggestUniqueBaseName's search so a pathological repository can't hang the Rename dialog forever. - New InstallTempFiles util: shared filesDir/temp staging (extracted from near-identical code in ExternalFileInstallViewModel and PluginManagerViewModel's ContentUri branch) that also sweeps hour-old orphans - covers a temp file left behind if a forwarded .cgp's hand-off to PluginManagerActivity never completes. - FlashbarActivityUtils: extracted a shared configureFlashbar() helper so showFlashBar() and showFlashBarAwaitShown() can't silently diverge in their builder setup. - AndroidManifest.xml: documented two known, accepted limitations rather than fixing them - pathPattern can't match a mixed-case extension without an disproportionate enumeration of every case permutation, and suppressing recreation on uiMode/locale/etc. for dialog continuity means already-inflated View content can look stale until back-and-return (narrower on the Compose screen, which recomposes reactively on those axes). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from second high-effort code review of PR #1682 - CodeEditorView, FileTreeActionHandler: this PR's own new "cgt" archive type was missing from two pre-existing extension allowlists. Opening a .cgt from the file tree would edit its raw zip bytes as text (silent corruption on save) and get blocked by the 10MB file-size guard that every other archive type is exempt from. - ExternalFileInstallActivity: singleTask + onNewIntent, so a rapid double-tap on the same external file collapses into one Activity/ViewModel instance (whose receivedUriGate already dedupes by Uri) instead of spinning up a second instance that mints an independent temp file PluginManagerViewModel's path-based dedup can't recognize as the same source. Verified on-device: a duplicate launch now hits the same instance and shows one dialog, not two. - PluginManagerActivity: check the forwarded temp file still exists before showing the install-confirmation dialog, so a file removed by InstallTempFiles' stale-file sweep surfaces a clear message instead of a generic install failure. Also merged the forced/normal dialog branches into one builder so a future button/copy change can't be applied to only one and reintroduce a leaked-temp-file bug. - PluginManagerViewModel: clean up a forwarded LocalFile temp copy on cancellation too (previously only success/failure paths did), and stopped deleteSourceDocument() from swallowing CancellationException. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from third high-effort code review of PR #1682 - ExternalFileInstallViewModel: fix a regression the singleTask change introduced - a rapid second VIEW intent for a *different* file could have its confirm-dialog effect overwritten by a slower first request that happened to finish its async work later, since the two onReceived() calls run as independent coroutines with no ordering guarantee. Added a generation counter, assigned synchronously so it always reflects real intent-arrival order; a request whose generation is no longer current abandons itself (and its temp file) instead of emitting a stale effect. Verified on-device: the previous ("clean up at start") approach left both files' temp copies on disk; this one leaves exactly one, matching whichever file's dialog is showing. - PluginManagerViewModel: fixed an ownedTempFile assignment race (a cancellation landing exactly as the ContentUri copy finished could skip the `.also{}` that recorded it, leaking the copy past `finally`'s cleanup) by assigning it as a plain statement before the copy runs, not after the whole block returns. - PluginManagerViewModel/PluginManagerActivity/PluginManagerUiState: extracted a deleteIfLocalFile() helper, replacing 6 copies of the same `if (source is PluginInstallSource.LocalFile) deleteInstallSource(source)` guard, and removed CancelPendingInstall's now-dead deleteSourceAfterInstall field (the handler stopped reading it once an earlier fix switched to checking the source type directly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from fourth high-effort code review of PR #1682 - TemplateCollectionRepositoryImpl: the backup step (moving an existing destFile aside before the swap) had no copy+delete fallback, unlike the swap and restore steps a few lines below - meaning an overwrite install could always fail on a device where renameTo() is unreliable even for a same-directory move (the exact issue this PR already fixed for the swap/restore steps). Applied the same fallback here too. - ExternalFileInstallViewModel: extend the generation-gating from the previous fix to installation completion, not just dialog dispatch - confirmTemplateInstall() now captures its generation and checks it before sending Finish (so a slow install for an abandoned dialog can't tear down the Activity out from under a newer, unrelated dialog) and before touching `_isInstalling` (so a stale install completing can't re-lock a newer dialog's buttons). dispatchTemplateInstall() now also resets `_isInstalling` when committing to show a new dialog, so it isn't left stuck "true" by an abandoned generation's still-running install. - InstallTempFiles: throttle sweepStale() to once per 10 minutes instead of a full directory scan on every single temp-file creation - stale entries can only appear once per hour (MAX_AGE_MS) regardless. Verification note: the physical test device was unreachable this round (disconnected mid-session) - verified via the full relevant unit test suite (including a new test locking in the isInstalling-scoping fix) and careful tracing of the generation-check logic, which builds directly on the already on-device-verified mechanism from the previous commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from fifth high-effort code review of PR #1682 - PluginManagerActivity: a .cgp forwarded from ExternalFileInstallActivity could stack a second Plugin Manager instance on top of one the user already had open/backgrounded. ForwardToPluginManager's launch Intent now carries FLAG_ACTIVITY_CLEAR_TOP|FLAG_ACTIVITY_SINGLE_TOP, and PluginManagerActivity gained an onNewIntent() override (mirroring ExternalFileInstallActivity's own singleTask handling) so a reused instance still processes the forwarded install instead of silently dropping it. - PluginManagerActivity: the forwarded-install file.exists() check ran synchronously on the main thread during onCreate()/onNewIntent(); moved onto Dispatchers.IO like the other file-system checks in this flow. - PluginManagerActivity: dropped showInstallConfirmation's redundant forceDeleteSource parameter - it was 100% determined by source's runtime type at both call sites, so compute it internally instead. - ExternalFileInstallViewModel: confirmTemplateInstall's success message now includes the target base name, so the toast is unambiguous even when a slow install completes after a newer, unrelated dialog has already taken over the screen (it must still fire per the existing isInstalling-scoping test - the install genuinely succeeded). - ExternalFileInstallViewModel: collapsed the two structurally-identical plugin/template availability-check blocks into one. - InstallTempFiles: lastSweepAtMs is read/written from coroutines PluginManagerViewModel and ExternalFileInstallViewModel can launch on different dispatchers - switched to AtomicLong with compareAndSet so two near-simultaneous callers can't both pass the throttle check. Not fixed (out of scope / pre-existing, not regressions from this PR): - InstallFileAction not recognizing .cgt for the in-editor "Install" action - a new feature (wiring TemplateCollectionRepository into that action), not a bug in the external-open flow this ticket covers. - ITemplateProvider.getInstance(reload=true) rescanning all installed collections on every install - existing reload API behavior, not something introduced here. - FeedbackManager/FeedbackEmailHandler's duplicated PixelCopy capture logic - pre-existing, unrelated duplication only touched by this PR's formatting pass. Verification: full app unit test suite green; on-device (R5CN80KZCKD) reproduction of the duplicate-instance scenario (two .cgp VIEW intents in quick succession while the first's confirm dialog is still open) confirms a single PluginManagerActivity instance (same ActivityRecord/ task) handles both, no crash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix ktlint line-length wrap in ExternalFileInstallUiModels Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix findings from sixth high-effort code review of PR #1682 - ExternalFileInstallViewModel: confirmTemplateInstall() captured the live currentRequestGeneration counter instead of the generation the on-screen dialog actually belongs to. A second VIEW intent bumps that counter synchronously before its own dialog is shown, so tapping Install/Overwrite/Rename on the still-visible (but now stale) prior dialog in that window got misattributed to the newer generation - on success this incorrectly sent Finish, tearing the Activity down (and its viewModelScope) out from under the newer, still in-flight request. Fixed by tracking pendingConfirmationGeneration alongside pendingConfirmationTempFile and keying confirmTemplateInstall() off that; also guards against acting on a tempFile that's already been superseded (and deleted) entirely. - ExternalFileInstallViewModel: IgnoreTemplateInstall had the same root cause - a stale Cancel tap unconditionally sent Finish regardless of whether pendingConfirmationTempFile still matched. Now a no-op when it doesn't. - ExternalFileInstallViewModel: suggestUniqueBaseName()'s attempt-bound check ran before the collision check, so the final candidate returned when MAX_SUGGESTION_ATTEMPTS is hit was never actually checked for collision. Reordered the && operands so the bound only short-circuits after that last check has run. - ExternalFileInstallViewModel: template install failures showed a generic, non-actionable message; now includes the underlying reason (reserved name / already exists / swap failure) via a %1$s arg, matching PluginManagerViewModel's equivalent error path. - PluginManagerViewModel: _uiEffect used the default rendezvous channel, the same latent drop-before-collector-attaches bug class this PR already fixed for ExternalFileInstallViewModel's channel. Switched to Channel.BUFFERED for consistency; not user-visible today, but the channel now also serves the forwarded-.cgp path. - TemplateCollectionRepositoryImpl: extracted the renameTo()+copyTo() fallback (triplicated across the backup/swap/restore steps) into a single moveFile() helper. Not fixed (narrow races / low-value at this stage, not regressions): - installCollection() has no per-destination-name locking, so two concurrent installs to the same target name could race on the final swap. Requires two attachments sharing a name AND overlapping generations to hit; accepted as last-write-wins for now. - InstallTempFiles' hour-old sweep could delete a pending confirmation's temp file if the user leaves a dialog open that long; would need cross-ViewModel "file in use" tracking to fix properly. - installPlugin() awaits initialLoadCompleted before copying the incoming URI, when the two could run concurrently - a cold-start-only latency nicety, not a correctness issue. - dispatchTemplateInstall() does its zip-read/collision-check I/O before its own generation check - inherent to check-then-act, the I/O can't be skipped without knowing in advance it'll be superseded. Verification: full app unit test suite green, including two new regression tests for the generation-capture fix (confirming a stale dialog surfaces success without Finish-ing over a newer request; ignoring a stale dialog doesn't Finish over a newer one) and a strengthened suggestUniqueBaseName test asserting the give-up candidate was actually checked. On-device (R5CN80KZCKD): fresh install and overwrite (via the new moveFile()) both verified with a real .cgt archive - correct dialog content, clean success, no crash, no stray .tmp/.bak files left in the templates directory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Address new CodeRabbit findings on PR #1682 - TemplateCollectionRepositoryImpl: installCollection() had no per-destination-name locking, so two concurrent installs targeting the same case-insensitive base name could both pass the collision check before either wrote destFile, and the later swap would silently clobber the earlier one. Wrapped the whole operation in a Mutex keyed by the lowercased target base name (independently flagged by both CodeRabbit and the prior code-review round, which this addresses). - InstallTempFiles: newTempFile() ran mkdirs() and its periodic directory sweep/delete on whatever dispatcher the caller happened to be on - ExternalFileInstallViewModel.onReceived() called it without a surrounding withContext(Dispatchers.IO), so that filesystem work ran on the main thread. Made newTempFile() suspend and dispatch to Dispatchers.IO internally, so no caller can repeat the mistake. Not changed (already-deliberated design decisions / false positive): - CodeRabbit suggested a superseded install's completion should suppress its ShowSuccess effect entirely, since it can render over a newer request's dialog. This was already addressed in the prior commit by including the collection's name in the message so the toast is unambiguous regardless of what's currently on screen - suppressing it outright would mean a genuinely successful install never gets reported to the user. Replied on the thread with this reasoning. - CodeRabbit's JUnit Jupiter migration suggestion doesn't correspond to any actual change in this PR - both flagged test files still use @RunWith(RobolectricTestRunner::class) and plain JUnit4 @Test/runTest, unchanged. Replied noting this appears to be a false positive. Verification: full app unit test suite green. On-device (R5CN80KZCKD): fresh install of a real .cgt archive after these changes - correct dialog, clean success, no crash, no stray .tmp/.bak files in the templates directory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Fix a self-inflicted stuck-dialog regression from the sixth review round - ExternalFileInstallViewModel: confirmTemplateInstall() clears pendingConfirmationTempFile on entry (transferring tempFile's "ownership" to the install attempt, per the sixth-round generation fix), but never restored it on install failure. The dialog is deliberately left open so the user can retry - but with pendingConfirmationTempFile left null, every subsequent tap (Install/Overwrite/Rename again, or Cancel/back) silently no-ops forever, since both confirmTemplateInstall() and IgnoreTemplateInstall key off it matching. The excludeFromRecents=true trampoline Activity has no other way out at that point short of force-stopping the app. Fixed by restoring pendingConfirmationTempFile/Generation in the onFailure branch (gated on isCurrentGeneration, same as everything else there) so a retry or cancel on the still-open dialog matches again. - ApkInstaller: isValidApk's extension check was case-sensitive (`== "apk"`), inconsistent with every other extension check this PR touched. An uppercase .APK (common from browsers/email/file managers that preserve sender casing) silently failed with no error shown. Not changed (pre-existing gaps, not regressions from this PR, larger lifts than warranted at this point): - InstallFileAction's file-tab "Install" for .cgp calls PluginRepository.installPluginFromFile() directly, bypassing the signature-mismatch/overwrite-confirmation check every other plugin install entry point goes through - pre-existing behavior, would need routing this action through the same ViewModel-level conflict resolution. - PluginRepositoryImpl.installPluginFromFile has no backup/rollback or per-target locking, unlike TemplateCollectionRepositoryImpl - a structurally similar but substantially larger lift given plugin install's uninstall/restart semantics. - Two distinct .cgp files forwarded to an already-open PluginManagerActivity in quick succession can each pass markPendingInstallHandled's per-value dedup and stack two native AlertDialogs - would need the same generation-tracking machinery ExternalFileInstallViewModel has, ported to the plugin flow's more complex dialog chain. Verification: full app unit test suite green, including two new regression tests (retrying Install after a failed install actually re-attempts it; cancelling after a failed install still finishes) that fail against the pre-fix code and pass against the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Stack the name-conflict dialog's three buttons vertically AlertDialog's default confirmButton/dismissButton row can't fit three actions (Overwrite / Rename & Install / Cancel) on one line, so it wrapped awkwardly - one button alone on the first row, the other two crammed together on a second row. Moved all three into the confirmButton slot as a right-aligned Column instead, so they stack one per row. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4934: Add a mimeType-only manifest fallback for opaque content:// Uris QA (Daniel Alome, ticket comment) found that opening a real .cgp attachment from the Files app failed silently: Storage Access Framework providers (Android's Downloads app, most file managers) hand out opaque document IDs like content://.../document/msf%3A19, with no filename anywhere in the Uri. Every existing intent-filter here requires a pathPattern match, so none of them can ever match this - the OS instead fell through to an unrelated app that happened to declare an unconstrained VIEW+content+application/octet-stream filter (Google Pay's pkpass handler), which claimed the single unambiguous match and opened/closed with no chooser and no visible error. This exact tradeoff was already called out as a "known limitation, not fixable via manifest matching" in this file's own comment, on the reasoning that the only pathPattern-less alternative was mimeType="*/*" - which would register this app as a candidate for every file view intent on the device. That reasoning missed a middle ground: a pathPattern-less filter matching only the small, specific set of mimeTypes a binary/zip attachment actually carries (application/octet-stream, application/zip, application/x-zip-compressed) is narrow enough to be worth it. Added as a fourth intent-filter block and updated the manifest's own "known limitations" comment accordingly. This does mean the app now offers itself as an "Open with" candidate for any octet-stream/zip content from any app, not just .cgp/.cgt - accepted since the real extension is still re-validated from DISPLAY_NAME once opened (ExternalFileInstallViewModel.onReceived), so a mismatched file is rejected gracefully rather than mishandled. Verification: `./gradlew :app:processV8DebugMainManifest` succeeds. On a physical device (R5CN80KZCKD), simulated a real Downloads-provider- style opaque content Uri (content://com.android.providers.downloads. documents/document/msf%3A999, type application/octet-stream) via `am start` - confirmed via logcat/dumpsys and a screenshot that "Code on the Go" now appears in the "Open with" chooser, where before this fix it was completely absent from the candidate list (reproducing exactly the bug QA reported). No crash when actually opened. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* New assetlinks.json generation workflow * Serve assetlinks.json from R2 via a Worker, not an Origin Rule The Origin Rule approach the previous commit assumed cannot work on our Cloudflare Free plan: R2 selects a bucket from the Host header, and host header, SNI and DNS record overrides are all Enterprise-only. Free exposes only the destination-port override. A Worker replaces the origin fetch rather than retargeting it, and reaches the bucket through an R2 binding - an in-network handle, not a URL - so no DNS, TLS or Host header is involved and the bucket keeps public access off. - infra/well-known-worker: the Worker, wrangler.toml and a README covering the plan constraint, the bucket/token prerequisites and how to verify. - deploy-well-known-worker.yml: deploys it via cloudflare/wrangler-action. Needs a new CLOUDFLARE_WORKERS_DEPLOY_TOKEN secret; the existing CLOUDFLARE_KEY_ID / CLOUDFLARE_SECRET_ACCESS_KEY pair is an R2 S3 credential and cannot deploy a Worker. - signing-fingerprint.yml: comments and failure hints now name the Worker. No functional change - it already writes the key the Worker reads. Routes match exact paths rather than /.well-known/*, so certificate renewal via /.well-known/acme-challenge/ still reaches the origin, and a request with no matching object falls through to the origin as well. ADFA-5067 * Document the R2 read scope the Worker deploy actually needs Run 32197717567 failed with "Authentication error [code: 10000]" on GET /accounts/<id>/r2/buckets/well-known: wrangler resolves the bucket named in the r2_buckets binding before finishing the deploy, so the token needs Workers R2 Storage -> Read on top of Workers Scripts and Workers Routes. Records the API call and the exact error so the next person does not have to rediscover it from a failed run. ADFA-5067 * TEMP: probe Cloudflare token scopes in the Worker deploy The deploy fails on GET /accounts/<id>/r2/buckets/well-known even with Workers R2 Storage Read on the token. Probe /user/tokens/verify plus the bucket list and bucket detail endpoints to find which grant is missing, and to confirm the stored secret is the token we think it is. To be reverted. ADFA-5067 * Revert the token-scope probe; note the scope propagation delay The probe answered the question: Workers R2 Storage Read is both required and sufficient, and the stored secret was always the right token. The two failures came from re-running about a minute after the scope was added, before it had taken effect. Reverts the TEMP diagnostic step and records the delay next to the scope list so the next person does not read the stale error as a wrong scope. ADFA-5067
#1657) * ADFA-5098: Require font-scale verification for new and changed screens CoGo is built for developers with limited vision, but nothing in our docs asked anyone to check a screen at a large system font. REVIEW.md covered TalkBack semantics only, and CLAUDE.md had no accessibility guidance at all. Text units are already correct repo-wide (0 dp text sizes, 65 sp), so this is a layout reflow problem, not a units problem. The failure mode is text that grows into a container that cannot: an sp dimen used as a margin, a 40dp box around a label, ellipsize="none", or content with nowhere to scroll. - CLAUDE.md: new constraint bullet requiring 1.0/2.0 verification, plus the adb recipe under Build & test -> Emulator / device. - REVIEW.md: widen section 8 to cover scaling, add a checklist item and an evidence-ledger entry. Required for new/changed screens, with a one-line opt-out for surfaces with no text. - architecture-review skill: rule 12, so the section 10 deep pass checks font scale the way it checks the system bars. Manual verification rather than a test: the repo has no screenshot testing, no Compose UI-test dependency, and has never used Robolectric qualifiers. This mirrors how section 11 already handles offline verification. Every command in the new CLAUDE.md block was run against an API 36 emulator before being written down. * ADFA-5098: Close three gaps in the font-scale guidance Follow-ups to acb76fe, all in the docs it touched. The rule named only 2.0 in the architecture-review rule 12 and the REVIEW.md checklist item, and the REVIEW.md evidence line asked for a screenshot at 2.0 alone. All three now require 1.0 and 2.0, matching what CLAUDE.md and the REVIEW.md summary row already said. Verifying at one scale does not show that a layout reflows; it shows one snapshot of it. The architecture-review applicability line mapped 9 of its 12 rules to a change type, orphaning rules 3 (Koin), 7 (module boundaries) and 10 (strings) -- a reviewer following the line literally would skip them. Each now has a scope, plus a catch-all so a rule added to the table later is not dropped by omission. The CLAUDE.md adb recipe printed the current font_scale but never captured it, then restored a hard-coded 1.0. A device sitting at 1.15 was left at 1.0, and a device that never had the setting had one created. The value is now captured (stripping the CRLF adb shell returns), an EXIT trap restores it so the restore also fires if screencap dies partway, and the null case deletes the setting rather than inventing a value. No device was attached, so unlike the parent commit the recipe was verified against a stub adb that records writes and replays a seeded value. Seeds 1.15, null and 2.0 each end at their starting state; the old block failed the first two.
* ADFA-5046: Add Java code action: surround with try/catch * ADFA-5046: Apply Spotless formatting to SurroundWithTryCatch.kt --------- Co-authored-by: Daniel Alome <astrocoder007@gmail.com>
…with link to webhelp (#1687) Co-authored-by: Daniel Alome <astrocoder007@gmail.com>
* feat(plugin-api): add tool contribution contract for the AI agent Versioned, additive-only registry contract; docs, changelog and ABI dump updated. * docs(plugin-api): correct contract-version and ownership claims, test defaults
* ADFA-4826: Add shared IDE Compose theming in common-compose New leaf module holding the Compose theme any module can opt into: IdeColorScheme derives a Material3 scheme from the IDE's own colour resources, IdeTheme applies it and seeds LocalContentColor so text on a themed surface inherits the right colour. Compose types are exposed as `api` because consumers write Compose against them. Modules that are not Compose depend on nothing new. * ADFA-4826: Move profiler and floating-window onto the shared theming Both modules carried their own near-identical copy of the IDE colour derivation. They now delegate to common-compose, so there is one place where the IDE's Compose colours are defined.
* ADFA-4826: Enable Compose in lsp/kotlin
The refactoring bottom sheets are Compose (ADR 0009) and live in this module
rather than a UI module because `editor` depends on it, not the reverse (ADR 0011).
Adds the lifecycle-runtime-compose catalog entry for collectAsStateWithLifecycle().
* ADFA-4826: Add extract-variable analysis, plan and rewrite
One background analysis pass produces a plain-data ExtractionPlan covering every
candidate expression - its legal scope chain, occurrence set and suggested name -
so the UI does pure offset arithmetic and never touches PSI (ADR 0011).
Occurrence matching is symbol-aware, not textual: two sites match only when they
are structurally equal and every name reference resolves to the same declaration.
Sites made unsound by an intervening write are excluded rather than warned about.
* ADFA-4826: Add the extract-variable Compose sheet
One surface holding every choice - expression, name, scope, replace-all - because
they are interdependent: a different expression changes the scope list and the
occurrence count, and sequential dialogs would hide that.
Each chooser is hidden when it has nothing to ask. State derives entirely from the
plan, so the ViewModel is a plain unit test with no editor, activity or Compose.
Uses the shared IdeTheme from common-compose.
* ADFA-4826: Wire up the extract-variable code action
execAction runs the analysis off the UI thread and returns the plan; postExec shows
the sheet and turns the user's choice into one spanning TextEdit. The document
version is re-read on confirm - the editor stays reachable while the sheet is open,
and applying spans computed against older text would corrupt the file.
No prepare() visibility gate: deciding extractability needs an analysis session,
far too costly for the UI thread. Records the placement decision as ADR 0011.
* ADFA-4826: Document the extract-variable requirements
Requirements, scope, non-goals, acceptance criteria and the test split, following
the kotlin-goto-definition.md template. Also carries the Language section for the
whole refactoring family - extract method, inline variable and rename all reuse
this vocabulary rather than restating it.
* ADFA-4826: Stop offering the lambda that wraps the expression
* ADFA-4826: Label a block rung by the construct that owns it
* ADFA-4826: Fix misleading KDoc and add else block test
Remove dead code path (owner.then === branch can never be true). Correct
the KDoc to accurately describe that getThen()/getElse() return unwrapped
body expressions, not containers, so branch identity is checked via
owner.then?.parent === container. Add test for braced else branch to
prevent regression.
* ADFA-4826: Write the return type when converting an expression body
* ADFA-4826: Anchor the declaration in the scope the user picked
* ADFA-4826: Cover contentSpanOf and fix a nested-block fixture
* ADFA-4826: Expand a block written on one line
* ADFA-4826: Expand only a block that is really written on one line
* ADFA-4826: Split the type-text renderer from its catching form
* ADFA-4826: Decline a block whose statement shares the brace line
A block whose first served statement shares the opening-brace line but
whose content spans several lines fell through the one-line-expansion
check into the normal hoist path, anchoring above the block's own
opening delimiter -- outside the scope the user picked. For a lambda
this put the declaration where `it` is unresolved, emitting Kotlin that
does not compile.
Also fix contentSpanOf: it decided brace ownership by sniffing the
block's own text for a leading `{` and trailing `}`, which misreads a
lambda whose sole statement is itself a lambda literal
(`{ x -> { x + 1 } }`) as owning its braces, returning the inner
lambda's interior instead of the outer body's content. Ownership is now
decided structurally, from the block's parent.
* ADFA-4826: Tidy the expression-body conversion and its docs
Nothing was folded into the Unit case when deciding whether an
expression-body conversion needs a `return`, so a Nothing-returning
function (`fun boom() = error(...)`) lost both its `return` and its
inferred return type, silently narrowing it to Unit and breaking a
caller that uses it in a Nothing position (`x ?: boom()`). Only Unit is
excluded now; Nothing goes through the normal return-type-writing path.
Also:
- Dedupe the symbol-to-return-type lookup into one
KaSession.returnTypeOf, dropping the always-succeeding
`as? KtDeclaration` cast.
- ScopeChain: drop the unread ScopeFrame.statementSpan field and the
dead `branch` local.
- TypeText: document that the "anonymous"/"ERROR" substring checks in
isUnrenderableTypeText are ambiguous but fail safe, and stop
shortening a star-imported type when the file also imports a
different type of the same simple name.
- docs/features/kotlin-extract-variable.md: reword the Status line,
the "Refactoring plan" glossary entry and a code comment that
referenced the RefactoringPlan supertype and ADR 0013 as already
landed -- both arrive with extract method (ADFA-5080); fix the
"Anchor point" glossary entry to match the current anchoring
behaviour; renumber the 9a/9b acceptance criteria into real ordered
items.
* chore: remove plan docs
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
* ADFA-4826: Refuse an unhostable block rung at plan time
* ADFA-4826: Keep replace-all off an unhostable anchor
* ADFA-4826: Validate the variable name against names actually in scope
* ADFA-4826: Decide Unit-ness from the type text that gets written
* ADFA-4826: Resolve a whitespace-only selection like a caret
* ADFA-4826: Offer the expression chooser for an exact selection too
* ADFA-4826: Apply Spotless formatting
* ADFA-4826: Address whole-branch review findings
---------
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
* ADFA-5080: Add extract-method requirements and ADR 0013
Requirements only - no implementation yet. R1 to R16 plus non-goals, 21 acceptance
criteria, the design and the test split; shared vocabulary and primitives come from
kotlin-extract-variable.md rather than being restated.
ADR 0013 records the principle most of those requirements are an application of:
the refactoring moves code, never edits the interior of what it moved, and declines
with a specific reason where it cannot transform faithfully. Two limitations it
creates are tracked separately - ADFA-5081 (multi-edit undo) and ADFA-5082
(reassigned outer var as the single output).
* ADFA-5080: Hoist the shared refactoring plan supertype
* ADFA-5080: Resolve a selection to an extraction region
* ADFA-5080: Fix KDoc and pin fallthrough for extraction region
* ADFA-5080: Add the extract-method plan model and its two rewrites
* ADFA-5080: Strengthen the CRLF test and cover the Unit-expression case
* ADFA-5080: Derive the extracted signature, or a typed refusal
* ADFA-5080: Close the extract-method refusal gaps that emit broken Kotlin
* ADFA-5080: Refuse the receiver, label and smart-cast cases that emit broken Kotlin
* ADFA-5080: Declare a local extracted function before its call site
* ADFA-5080: Add the extract-method sheet state and strings
* ADFA-5080: Reword two extract-method refusal messages
* ADFA-5080: Add the extract-method Compose sheet
* ADFA-5080: Wire up the extract-method code action
* ADFA-5080: Stop the analysis emitting Kotlin that does not compile
Five ordinary shapes produced a broken file rather than a refusal, which ADR
0012 rules out: the refactoring moves code and declines where it cannot.
- Signature types render fully qualified. A short name resolves only when the
file already imports it, and a local's type usually comes from inference, so
`val d = java.util.Date()` emitted an unresolved `Date`. `usedTypeOf` moves to
the same renderer, or every capture would read as a smart cast.
- A platform type is emitted as its flexible type's lower bound instead of
`String!`, which does not parse. `!` anywhere in a rendered type now counts as
unrenderable, catching the nested `List<String!>` the lower bound leaves.
- `suspend` is no longer added for a call the region only makes inside a nested
suspend-typed lambda. `launchIt { work() }` in a non-suspend function emitted
a `suspend fun` its own call site could not call. Inline lambdas still
propagate, and extracting from inside such a lambda still adds `suspend`.
- The capture loop skips the selector of a qualified expression, refuses a value
whose type is a class local to the enclosing declaration, and refuses rather
than drops a local class or object used as a qualifier. `h.n` used to emit
both a parameter named `n` that no call site had and a `Holder` type the new
function could not see.
- A tail return from a secondary constructor takes `Unit`, not the constructed
class. `return extracted(...)` on a Unit call is legal in a constructor.
Refusal quality and failure isolation, in the same pass:
- `MultipleOutputs` split. It fired for three situations, two of which are one
value, and rendered "produces more than one value: result". A single output
the call site cannot receive back is now `OutputNotReturnable`.
- `CouldNotAnalyse` added. A missing environment, an unreachable KtFile and a
thrown error all reported "Select an expression, or whole statements inside
one block" - the most confident message in the set, aimed at a selection
nothing had looked at. Cancellation is re-thrown rather than swallowed.
- `takenNamesFor` tests the local-`fun` target before the containing class, so a
new local validates against its siblings instead of the class's members.
- `applyChoice` runs from a Compose click handler outside every framework guard;
its body is now wrapped.
The feature doc's R4, R5, R7, R10, R14 and R15 are corrected to match, and its
claim that the version guard lives on `RefactoringPlan` is dropped - the two
actions each do their own comparison.
* ADFA-5080: Revert the qualified-selector capture guard
The guard added in 0a20c0d76 dropped a selector that still needed capturing,
and emitted a broken file in two shapes the previous behaviour refused:
- a local extension `fun` called as `h.twice()` was skipped, so nothing
refused and the moved body called a function out of scope there.
- pointing `innerImplicitReceiver` at the same helper skipped a *call*
selector, losing the `with`-receiver refusal for a member extension -- the
pervasive Compose shape, `with(density) { size.toPx() }`.
Both were reproduced before the revert and re-checked after it. The shape the
guard was meant to fix, a member of a local class reached as `h.f()`, refuses
identically without it: the capture loop is offset-ordered, so the receiver is
refused for its local type before the selector is ever reached.
`innerImplicitReceiver` gets its original guard back, with a comment on why it
must stay shallow. The capture loop gets a comment on why it has none, so the
guard does not come back. The refusals for a local class type and for a local
class or object used as a qualifier are untouched.
The test that defended the guard used a top-level class, whose members the
pre-existing ancestor test already skips, so it passed either way. It is
replaced by one test per broken shape, both of which fail against the guard.
Three doc statements the previous commit should have moved and did not:
- R8 said the extracted function takes the enclosing function's return type;
the secondary-constructor exception lived only in a code comment.
- R16 promised a refusal for anything thrown; cancellation is now re-thrown
deliberately, and the reason it is safe belongs next to the promise.
- R11's preview example predated fully-qualified rendering.
* ADFA-5080: Use the shared type-text helpers
* ADFA-5080: Stop anchoring an extraction on an anonymous function
* ADFA-5080: Detect Composable property getters when annotating
* ADFA-5080: Stop counting a nested declaration's return as a region exit
* ADFA-5080: Decline an extraction inside an anonymous extension function
* ADFA-5080: Tighten the Composable check and the anchor assertions
* ADFA-5080: Keep multi-line string literals verbatim when re-indenting
* ADFA-5080: Collect only raw string literals as protected spans
* ADFA-5080: Apply spotless formatting
* ADFA-5080: Record the new extract-method test coverage
* ADFA-5080: Require a tail return to return from the enclosing declaration
* ADFA-5080: Give the anonymous-extension-function refusal its own message
* ADFA-5080: Tighten the Composable lookup and the extract-method docs
* ADFA-5080: Link the type-text shortening follow-up
* ADFA-5080: Always offer the expression chooser
The extract-variable PR below this one deleted CandidateSyntax.selectionMatchedInnermost
outright, so suppressing the chooser on an exact selection is no longer expressible: with
the source of that flag gone, keeping the behaviour would mean reintroducing the deleted
computation. QA found the suppression actively harmful there - long-press selects one
token, so with the chooser hidden there was no way to widen from `b * a` to `b * a + a`
without cancelling and re-dragging - and the same reasoning applies here.
Drops the flag from ExtractionRegion.Expressions and ExtractMethodPlan, and updates R2/R11
in the feature doc to match.
) * Workflow to remove Rovo Dev messages from Github PR descriptions * ADFA-5206: Also delete the Rovo Dev account-linking comment Atlassian nags in two places. Besides editing the PR description, atlassian[bot] posts a comment asking you to link your GitHub account to enable Rovo Dev code reviews. Delete it on issue_comment. The match is deliberately narrow - the bot as author plus the ad wording - because a genuine Rovo Dev review would come from the same bot and must survive. A comment event that is not the ad is filtered at the job level, so an ordinary comment never starts a runner. Adds a workflow_dispatch sweep that strips both nags across all open pull requests, for the ones opened before this lands. * ADFA-5206: Gate the comment path on pull requests, pin github-script Two review fixes. The issue_comment filter now requires github.event.issue.pull_request, so the job only runs for comments on pull requests. That matches the workflow_dispatch sweep, which walks pulls.list and never touched plain issues. actions/github-script moves from the mutable v7 tag to the commit it currently points at. This workflow runs on pull_request_target with pull-requests and issues write, so a moved tag would execute with those permissions.
* Update Indonesian translation
…e) (#1711) The Indonesian localization rendered the Android term "resource" as "sumber", which means source. The correct term is "sumber daya". Six user-facing strings affected -- five from #1703, plus the pre-existing new_xml_resource. Left "sumber" alone where it correctly means source: idepref_java_diagnosticsEnabled_summary (Java source files), title_open_source_licenses / summary_open_source_licenses (open source), and markdown-preview's view_source. Verified by round-tripping each changed value back to English through both Gemini and Google Cloud Translate; all six now return "resource".
…1677) * ADFA-5153: Decode Content rows against the shared Brotli dictionary WebServer now always decompresses brotli content server-side rather than ever passing compressed bytes through to the client -- sidesteps needing WebView-side dictionary support entirely, since the client never sees compressed bytes. It loads CompressionDictionary once at startup, and again on the debug-DB swap, and attaches it via brotli4j's attachDictionary before decoding -- falling back to plain decode if the table doesn't exist (a database that predates the dictionary migration). Confirmed cross-tool compatibility empirically: content compressed by OfflineDocumentationTools' brotli-CLI pipeline decodes byte-for-byte correctly via brotli4j's attachDictionary, and the same in-memory dictionary buffer is safe to reuse across many decode calls (WebServer holds one for its whole lifetime). BrotliDictionaryDecodeTest embeds those real cross-tool-produced fixtures as permanent regression coverage. Also adds testImplementation(libs.brotli4j.linux.x64): JVM unit tests exercising brotli4j's real native decoder had no native lib to load at all before this and would fail with UnsatisfiedLinkError -- a pre-existing gap, not introduced by this change, just never hit until now. docs/documentation-database.md updated for CompressionDictionary and WebServer's always-decompress behavior. * Apply spotlessApply formatting Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Narrow no-dictionary decode test to IOException CodeRabbit flagged this test as asserting an unsupported invariant, citing docs/documentation-database.md's claim that "wrong dictionary, or none" doesn't reliably fail loudly. Verified empirically that the two cases are actually distinct: a wrong dictionary decodes silently to incorrect bytes (its distances resolve into real, just wrong, bytes), but no dictionary at all reliably throws IOException, since distances into the dictionary region are out of bounds for any spec-compliant decoder. Narrowed the assertion from Exception to IOException and corrected the doc to describe both failure modes instead of conflating them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Address code-review findings on the dictionary compression PR Fixes 13 findings from a max-effort /code-review pass, most significant first: - Plugin-contributed Tier 3 docs (PluginDocumentationManager/BrotliCompressor) are plain brotli with no dictionary, but WebServer unconditionally attached the shared dictionary before decoding any brotli row -- every such page 500'd. Extracted decompressBrotli(): tries the dictionary first, falls back to a plain decode on IOException. Verified empirically that a dictionary attached to a stream compressed without one reliably throws rather than silently decoding wrong bytes, so this fallback never lets a real dictionary-compressed row slip through unnoticed. - loadCompressionDictionary() now wraps its whole body in one catch-all, matching DatabaseVersionResolver's existing pattern, instead of hand-anticipating individual failure cases. Fixes three related bugs this gap caused: a failed dictionary reload during the debug-DB swap left stale state with no retry; a dictionary-load failure at server startup aborted the entire server with no retry; a NULL dictionary blob threw an uncaught NPE. - Extracted switchToDatabase() so database/databaseTimestamp/ compressionDictionary/templateCache/bookshelfTemplateId are all swapped atomically in one place instead of duplicated across start() and the debug-swap block -- also fixes templateCache never being invalidated on a debug-DB swap, and a reopen-after-close ordering bug where a failed reopen left `database` referencing an already-closed handle. - Added test coverage for the previously-untested no-dictionary/plugin-content decode path. - Corrected docs/documentation-database.md's false "no dictionary-free content left" claim (contradicted by its own PluginDocumentationManager section) and the build.gradle.kts comment falsely claiming linux-x64 is the only platform this project's dev machines run JVM tests on. - Minor: deduped the byte[]->direct-ByteBuffer idiom, removed a stale Accept-Encoding comment on a header no longer read. Separately discovered (not caused by this PR, filed as ADFA-5168 instead of fixed here): :app:testV8DebugUnitTest is flaky (~50% of full-suite runs) due to Brotli4jLoader static state shared across one JVM test process between AssetsInstallationHelperTest's mockkStatic and BrotliDictionaryDecodeTest's real native load -- confirmed present on bfb3baa already, independent of any change in this commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Scope shared-dictionary claim to migrated brotli rows CodeRabbit caught a self-contradiction: line 34 already says non-Brotli content uses format-specific compression, but the prior wording said 'every row' is dictionary-compressed. Scoped to migrated Content rows with ContentTypes.compression = 'brotli'. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Add test proving the compression dictionary loads once Per ticket comment: verifies WebServer fetches CompressionDictionary only at startup and reuses the cached instance across every request, never re-querying it per-request. Drives 3 real HTTP requests over a socket against a mocked SQLiteDatabase and asserts the dictionary query fired exactly once while the Content query fired 3 times. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Reload compression dictionary per-request, not at swap time Moved loadCompressionDictionary() out of switchToDatabase() (called at startup and on the debug-DB swap) to right before the content fetch in handleClient(). A database swap can bring in a database with a different dictionary or none at all, so loading it right where it's consumed -- rather than caching it at swap time -- keeps it directly tied to whichever database is actually active when a request needs it. Updated the WebServerTest coverage added for the prior (now-reversed) "load once, cache for app lifetime" behavior: it now asserts zero dictionary queries before any request and one dictionary query per content fetch (3 requests -> 3 queries). Updated docs/comments to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Load compression dictionary lazily, once per database change Corrects the prior commit, which reloaded the dictionary on every single request instead of only when the database actually changes. Added compressionDictionaryStale, set by switchToDatabase() (startup and the debug-DB swap) instead of eagerly loading the dictionary there. The content-fetch site in handleClient() -- the one place the dictionary is actually consumed -- checks the flag and only loads when stale, clearing it once loaded. Net effect: loaded lazily (not merely from starting the server), but cached across every request against the same database, and reloaded exactly once when a swap brings in a database with a different dictionary (or none). Replaced the WebServerTest coverage accordingly: one test proves the dictionary loads on first use and stays cached across repeated requests against the same database; a second drives an actual debug-DB swap and proves it reloads exactly once for the new database, not on every subsequent request. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Run the brotli tests on any host, and cover the buffer helper Review of PR #1677 found three things worth fixing. The test native was pinned to linux-x64, so :app:testV8DebugUnitTest failed with UnsatisfiedLinkError in @BeforeClass for anyone on macOS, Windows, or linux-arm64 - a comment documented the breakage rather than fixing it. Dispatch on the host's OS/arch instead, reusing the pattern already proven in build-logic/plugins' build.gradle.kts. All six natives are already in the version catalog. BrotliDictionaryDecodeTest allocated its own direct buffer, a byte-for-byte copy of production's toDirectByteBuffer, leaving the only code that builds the runtime dictionary buffer untested. The two agree today, so this is a regression risk rather than a live bug: attachDictionary reads the buffer's capacity and ignores position/limit, so a later over-allocation there (pooling, rounding, padding) would break every doc page on device while the suite stayed green. The test now calls the production helper, and that helper's KDoc records the exact-capacity requirement. loadCompressionDictionary validated a missing table, an empty table, and a NULL data column, but not a zero-length blob. That yields a 0-capacity buffer, which attachDictionary rejects, so every row would fail its dictionary decode, fall through to a plain decode that also fails, and return HTTP 500 - with nothing above DEBUG to explain it. Added to the same ladder so it gets the same one-line warning. Left alone: peak heap on the chunked PDFs (always-decompress holds the accumulator, its copy, and the output live at once) and the debug-DB swap retrying every request after a failure. Both are pre-existing design questions rather than regressions from this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4t7fFLNPJq9EiXr9S9LxF * ADFA-5153: Cut peak heap on chunked rows, and stop retrying a bad debug DB Two findings from the PR #1677 review that were deferred as design questions. Peak heap on the largest bundled PDFs. Serving a >1 MB row concatenated its chunks into a ByteArrayOutputStream and then called toByteArray(), so the doubling buffer and its full copy were both live alongside the decompressed output - roughly 35 MB transient for AndroidNotesForProfessionals.pdf (8.8 MB over 9 chunks), a plausible OOM on a low-heap device. The chunks now stay a list: brotli rows decode from a SequenceInputStream over them, and non-brotli rows are joined once into an exactly-sized array. That drops the two largest transients, leaving the compressed chunks and the decompressed output. Fully streaming the response would remove the last one too, but that means giving up Content-Length, so it is left alone. A failed debug-database swap left databaseTimestamp unadvanced, and the swap is checked per request - so a corrupt or unreadable debug DB newer than the primary was reopened on every single request, logging an ERROR each time. The failing timestamp is now remembered and skipped; a newer copy has a different timestamp and is retried, which is the case that matters, since replacing the file is how a developer fixes it. joinChunks and chunksAsStream are internal top-level functions next to toDirectByteBuffer so the tests exercise the real code, with three new cases: a compressed stream decodes identically when split at uneven chunk boundaries, joinChunks concatenates in order at an exact size, and a lone chunk comes back without a copy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4t7fFLNPJq9EiXr9S9LxF * ADFA-5153: Address CodeRabbit findings on the dictionary tests - Assert the sqlite_master existence-check query count alongside the data query in both dictionary tests, not just the data query -- a regression that re-ran only the existence check every request would otherwise pass unnoticed. - Set socket.soTimeout before reading the response in sendRawGetRequestAndAwaitClose, so a server that fails to close the connection fails the test instead of hanging the JVM indefinitely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Address jatezzz's review on PR #1677 (3 of 5 findings) - loadCompressionDictionary no longer swallows exceptions into "no dictionary." It only returns null for a definitive absence (missing table, empty table, null/empty blob); an unexpected SQLiteException now propagates to the call site, which leaves compressionDictionaryStale set so the next request retries instead of permanently caching a transient failure as "no dictionary" for the rest of the database's lifetime. - brotli4jNativeForHost() in app/build.gradle.kts no longer throws on an unrecognized host. That ran at configuration time, so throwing failed every task in the build -- including :app:assembleV8Debug, which needs no desktop native at all -- not just the JVM unit-test tasks that consume it. Degrades to a logged warning and no test native instead. - Softened the chunked-content comment's memory-savings claim: the decompressed output still goes through a comparable accumulate-then-copy in decompressBrotli's own readBytes() call, so the saving from keeping compressed chunks as a list is real but doesn't eliminate that separate transient the way the prior wording implied. The two remaining findings (dictionary-first decode's theoretical silent-wrong-bytes risk, and the resulting double-decode cost for dictionary-free rows) need a design discussion, not a quick fix -- see the PR thread reply. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-5153: Warm the brotli loader before a test mocks it Order-dependent test failure between this PR's BrotliDictionaryDecodeTest and the pre-existing AssetsInstallationHelperTest: whichever runs first in a JVM decides whether the second one works. AssetsInstallationHelperTest does mockkStatic(Brotli4jLoader::class) and stubs ensureAvailability() to do nothing, since a unit test has no native library to load. brotli4j caches its availability in a static field, so a JVM whose first sight of that class is the mocked one keeps a "never loaded" state -- and BrotliDictionaryDecodeTest's @BeforeClass, which calls the real ensureAvailability(), then throws UnsatisfiedLinkError. unmockkAll() in teardown does not undo it: the damage is the cached state, not the mock. Loading it for real once, before anything mocks it, fixes it. runCatching because a host with no matching native is a legitimate configuration -- this PR's own brotli4jNativeForHost() degrades to a warning rather than failing the build -- so the warming is best-effort. CI is green on this PR because its test set happens to order favourably. The pair reproduces the failure deterministically: ./gradlew :app:testV8DebugUnitTest \ --tests "com.itsaky.androidide.assets.AssetsInstallationHelperTest" \ --tests "com.itsaky.androidide.localWebServer.BrotliDictionaryDecodeTest" Found while stacking ADFA-5176 and ADFA-5179 on this branch, where the added test class shifted the order enough to expose it. Landing the fix here keeps it with the test it protects, rather than leaving stage briefly broken after this merges. * ADFA-5153: Absorb only UnsatisfiedLinkError when warming the brotli loader Review was right that runCatching was too broad: it swallows every Throwable, so an unrelated failure in this setup would disappear silently. ensureAvailability() raises UnsatisfiedLinkError when there is no native for the host -- the one case the warming exists to tolerate -- so that is all it catches now, and it says so on stdout rather than passing in silence. * ADFA-5153: Gate the compression dictionary on the declared database version WebServer inferred the content format from whether a CompressionDictionary table happened to exist -- the heuristic ADFA-5220's version table exists to retire. It gets the answer wrong in both directions: a database carrying the table with unmigrated content makes every plain row pay a failed dictionary decode before its plain one, on every request, and a migrated database that lost the table fails quietly rather than loudly. Gate on DocumentationDatabaseVersion instead. At MAJOR >= 2 the dictionary is read and attached as before; below that, or with no version table at all, it is neither fetched nor used. The version read lives in DatabaseVersionResolver (common), so ADFA-5176's in-process transport can share the same gate rather than growing a second copy. It returns null for a definitively unversioned database and lets exceptions propagate, matching loadCompressionDictionary's existing contract: callers cache the answer per database, so a transient SQLiteException must stay distinguishable from a real absence or one hiccup would pin the database at unversioned until the next swap. The table is an append-only log, so the current version is the row inserted last, not MAX(major) -- rebuilding from an older content set is a downgrade and has to read as one. The CompressionDictionary probes stay, for a database that declares a new-enough version but has no usable dictionary row: without them the data query raises "no such table", which the caller correctly treats as transient and would then retry on every request. Tests: three new WebServer cases (major 1, no version table, major 3) asserting the dictionary queries are or are not issued -- with the dictionary cursors stubbed as available in every case, so they test the gate rather than a missing table -- and five DatabaseVersionResolver cases covering absent table, empty table, declared version, last-row-wins, and a downgrade. The two existing dictionary tests now declare a version; without that they would have kept passing while silently testing nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ADFA-5153: Load brotli4j's native library before decoding, not by luck Nothing in WebServer owned that load: it happened as a side effect of AssetsInstallationHelper's install or ToolsManager's tooling-jar update, neither of which runs on an ordinary cold start. A process that skipped both reached the first brotli row with the natives unregistered, and DecoderJNI.nativeCreate raised UnsatisfiedLinkError -- an Error, not an Exception, so it escaped handleClient's catch and killed the app from a coroutine worker instead of failing one request. Reproduced on device: force-stop, launch MainActivity directly (skipping SplashActivity, whose startup path happens to warm the loader), request a brotli row. The app died and restarted -- pid 10550 -> 10785, with FATAL EXCEPTION and UnsatisfiedLinkError in the log. Android restarting a killed process straight into the editor would take the same path. Referencing Brotli4jLoader triggers the static init that performs the load, so calling ensureAvailability() before the decode *is* the warm-up; afterwards it is a single static null-check on UNAVAILABILITY_CAUSE (verified against brotli4j 1.18.0's bytecode), cheap enough to leave on the per-decode path rather than tracking warmed state of our own. Its UnsatisfiedLinkError becomes an IOException so a genuinely broken environment costs one 500 rather than the process. After the fix, the same sequence returns the full 50,440-byte page, the pid is unchanged, and the log has no fatal or link-error lines. The version gate still behaves: a database declaring 1.0.0 serves 500 for a brotli row and 200 for a compression = 'none' row, without crashing. Also documents a trap that cost real debugging time: the debug-database swap compares modification times, and `adb push` preserves the source file's mtime, so pushing a database saved earlier than the one already on the device silently does not swap and the app keeps serving the old one with no error anywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* docs(ADFA-4510): design for code action tooltip fix
* docs(ADFA-4510): implementation plan for code action tooltip fix
* style(ADFA-4510): reformat files to tabs ahead of edits
Spotless ratchets whole files, so reformatting these four up front keeps the
following commits pure logic. ktlint normalisations only -- tabs, trailing
commas, expression bodies. No behaviour change; both modules compile.
* docs(ADFA-4510): correct Task 1 verification step
git diff -w can never be empty: ktlint normalises trailing commas, expression
bodies and blank lines, not just indentation. Replace with a hunk-by-hunk
review plus a compile of both modules.
* fix(ADFA-4510): resolve tooltip tags from either ActionItem member
retrieveTooltipTag() defaulted to "" while every LSP code action overrides the
tooltipTag property, so the code-actions renderer always read an empty tag.
Default the function to the property instead.
Add ActionMenu.findAction(itemId) so a submenu's renderer can reach children,
which are never registered with the ActionsRegistry.
* fix(ADFA-4510): pin java code action tooltip tags
VariableToStatementAction and FieldToBlockAction carried the fiximports tag by
copy-paste; neither touches imports. They were silent before this branch and
would have started showing wrong help. Drop both overrides.
Add JavaCodeActionTooltipTagTest, reading through retrieveTooltipTag() so it
exercises the member the renderer actually calls.
* fix(ADFA-4510): resolve code action tooltips at the bind site
Pass the parent ActionMenu to the submenu adapter so code actions resolve; the
registry only holds top-level actions.
Drop the contentDescription fallback. It read the action's label, which can
never match a tag, so it converted a missing tooltip into a silent DB miss.
Log a warning instead.
Use the action's own tooltip category rather than hardcoding 'ide', so
plugin-contributed code actions hit their plugin_<id> rows.
* fix(ADFA-4510): use the dialog tooltip tag in the override dialog
The method-selection dialog passed the menu item's tag, so it showed the menu
tooltip instead of its own. EDITOR_CODE_ACTIONS_OVERRIDE_SUPER_DIALOG was
declared but referenced nowhere.
* docs(ADFA-4510): add missing assets side-load step to Task 6
:app:assembleV8Debug does not bundle the large assets. Without building
:app:assembleV8Assets and pushing the payload to /sdcard/Download, a debug
install has no templates, no bootstrap, no SDK and no documentation.db, so
nothing about the fix can be verified on device.
* fix(ADFA-4510): keep the documentation fallback for untagged actions
Dropping the contentDescription fallback also dropped the ADFA-4754 popup.
That fallback made the tag non-empty for every action, so a long-press on an
untagged action reached showTooltip(), missed in the DB, and rendered "Sorry,
we don't have a tooltip for that. Explore the documentation." Returning early
on an empty tag turned that into a dead gesture for the eight untagged Java
actions and the two this branch un-tagged.
Still log the warning, but let the empty tag through so the miss renders the
fallback.
* test(ADFA-4510): cover the try/catch action and close the kotlin blind spot
Rebasing onto stage brought SurroundWithTryCatchAction into JavaCodeActionsMenu,
which the expected map did not list, so the suite failed on 23 actual vs 22
expected entries. Pin it to EDITOR_CODE_ACTIONS_TRY_CATCH.
Point the Kotlin twin at retrieveTooltipTag() too. Reading the property is the
exact hole that let ADFA-4510 through on the Java side while that suite stayed
green.
Add the GPL header both new test files were missing.
* test(ADFA-4510): drop Robolectric, assert through Truth
ActionTooltipResolutionTest exercises findAction(Int) and retrieveTooltipTag()
-- an id.hashCode() lookup and a String property. Its only Android type is a
Drawable? assigned null and never called, so every class in it bootstrapped an
SDK sandbox for nothing.
JavaCodeActionTooltipTagTest used raw JUnit asserts. ARCHITECTURE.md prefers
Truth, and containsExactlyEntriesIn names the offending key instead of dumping
both maps -- which is what the missing try/catch entry cost to read.
* docs(ADFA-4510): derive the repo root, validate the Firebase donor path
The plan hardcoded /Users/eisen/src/cogo/ADFA-4510, so the commands only ran on
one machine. Derive it with git rev-parse --show-toplevel.
The google-services.json fallback was worse: it copied from a hardcoded sibling
checkout with no check that the path existed or belonged to this project.
Require the donor as GOOGLE_SERVICES_SRC and verify it is a file first.
* docs(ADFA-4510): mark the try/catch tag as reserved ahead of content
The suite grouped surroundWithTryCatch with the tags that have authored
tooltips, but documentation.db has no editor.codeactions.trycatch row, so
long-press renders the documentation fallback. Its Kotlin twin,
editor.codeactions.kotlin.trycatch, is authored - this is an authoring gap,
not a wiring one.
Verified against the current documentation.db (46,105 tooltips, wholedb
2026-08-20), not the stale local asset copy.
Comment-only. The tag stays pinned: dropping it would change production
behavior, and the tag is correctly wired.
* fix(ADFA-4510): give the Kotlin import chooser a tooltip tag
AddImportAction opens a chooser dialog when a reference resolves to more
than one importable classifier, but wired no tooltip tag, so long-pressing
anywhere in that dialog did nothing. Same defect this branch already fixed
on the Java side for the override-superclass dialog.
Follows that precedent: applyLongPressRecursively bails out of ListView
subtrees, so the rows get their own OnItemLongClickListener and the dialog
chrome is wired in setOnShowListener.
The chooser construction moves into showImportChooser() because the
listener needs the created dialog, not the builder.
New tag editor.codeactions.kotlin.importclass.dialog has no row in
documentation.db yet, so long-press renders the ADFA-4754 documentation
fallback until content is authored - a live link, not a dead press.
471 tests across actions, idetooltips, lsp/java, lsp/kotlin: 0 failures.
* style(ADFA-4510): reindent Java AddImportAction to tabs
Space-indented, so the file-level Spotless ratchet reformats it whole the
moment it is touched. Isolating that churn here keeps the tooltip fix that
follows reviewable.
Whitespace plus the usual ktlint normalisations, verified with git diff -w:
two blank lines removed after a declaration opens, one trailing comma added,
and postExec's parameter list exploded one-per-line. No identifier, literal,
condition, or call argument changed.
* fix(ADFA-4510): give the Java import chooser a tooltip tag
Java's AddImportAction has the same gap just fixed on the Kotlin side: the
chooser shown when a simple name resolves to several importable types wired
no tooltip tag, so long-pressing it did nothing.
Same shape as the Kotlin fix and the override-superclass dialog already on
this branch: build, create(), wire the rows via OnItemLongClickListener and
the chrome via setOnShowListener, then show. applyLongPressRecursively bails
out of ListView subtrees, which is why both are needed.
New tag editor.codeactions.fiximports.dialog has no row in documentation.db
yet, so long-press renders the ADFA-4754 documentation fallback until content
is authored.
471 tests across actions, idetooltips, lsp/java, lsp/kotlin: 0 failures.
* fix(ADFA-4510): give the Kotlin null-safety chooser a tooltip tag
NullSafetyAction offers three fixes for an UNSAFE_CALL - assert non-null,
safe call, Elvis fallback - in a chooser dialog that wired no tooltip tag,
so long-pressing it did nothing. The action tag itself is authored, making
the dialog the only dead surface on this path.
Same pattern as the two import choosers: create(), rows via
OnItemLongClickListener, chrome via setOnShowListener.
New tag editor.codeactions.kotlin.nullsafetyfix.dialog has no row in
documentation.db yet, so long-press renders the ADFA-4754 documentation
fallback until content is authored.
* style(ADFA-4510): reindent AutoFixImportsAction to tabs
Space-indented, so the file-level Spotless ratchet reformats it whole on the
first touch. Isolating that churn keeps the tooltip fix that follows small.
Whitespace plus the usual ktlint normalisations, verified with git diff -w:
two blank lines removed after a declaration opens, five parameter lists
exploded one-per-line, getFileImports collapsed to an expression body, the
dialog builder chain rewrapped, and a redundant "${klass}" reduced to
"$klass". No identifier, condition, or call argument changed.
* fix(ADFA-4510): give the Java class chooser a tooltip tag
AutoFixImportsAction asks which class to import when a simple name is
ambiguous, one dialog per name. It wired no tooltip tag, so long-pressing
it did nothing. Last of the four unwired code-action dialogs.
Reuses editor.codeactions.fiximports.dialog rather than minting a new tag:
same question asked of the user as AddImportAction's chooser, and the two
actions already share an action tag.
Note this dialog is built through DialogUtils.newMaterialDialogBuilder
directly, not the newDialogBuilder helper the other three use - which is why
it did not turn up in the first sweep for unwired dialogs.
The nullable `e` is captured into a local `entry` so the listener body does
not smart-cast a var across a lambda boundary.
481 tests across actions, editor, idetooltips, lsp/java, lsp/kotlin:
0 failures.
testing/resources/test-project/.cg/gradle-sync/{project.pb,sync.pb,sync.lock}
were tracked, and a test run rewrites them with the local machine's absolute
paths. So they turn up modified in everyone's working tree, and a `git add -A`
buries them in an unrelated commit -- including a 13 MB binary. That is how they
last changed: the most recent commit touching them is an AI-plugin extraction
refactor that had no reason to.
They are a cache, not a fixture. ProjectSyncHelper writes them, and the only
test that mentions the cache is "WHEN sync files are unreadable THEN sync
anyway" -- it exercises their absence. Nothing reads a committed copy.
A lock file was tracked too.
Verified: with them untracked and ignored, running :lsp:java:testV8DebugUnitTest
-- the suite that used to rewrite them -- leaves a clean working tree.
Note for whoever picks up ADFA-5068: :subprojects:tooling-api-impl is a
java-library, so its tests never ran in CI (the aggregate depends only on
testV8DebugUnitTest), and ToolingApiServerImplTest does not currently compile on
stage -- "No value passed for parameter 'buildId'". Once that gap is closed, that
failure becomes visible.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
) The task copied tooling-api-model.jar to <root>/tests/test-home/.cg/init/model.jar. Nothing reads that file, and nothing reads that directory. The only consumer of a "test home" is gradle-plugin's test helper, which resolves FileProvider.testHomeDir() -- testing/resources/test-home, a different directory -- and then *writes its own* init script there, with a classpath from Gradle's PluginUnderTestMetadataReading. It never asks for a model jar. A grep for model.jar across the repo returned only the task that produced it. The destination had drifted before: 2a84174 (Feb 2023) is "fix: invalid path specified in copyToTestDir", and #1161 renamed .androidide to .cg inside it. Removing it takes three problems with it: - into(rootProject.mkdir(...)) ran at configuration time, so merely realizing the task created directories in the source tree -- on --dry-run, and again after every clean. That is why tests/test-home kept reappearing. - outputs.upToDateWhen { false } on both the copy and jar meant any build touching this module re-jarred and re-copied unconditionally. - Its output being a directory inside the source tree is what tripped Gradle's implicit-dependency validation against Spotless (ADFA-5244). That was worked around at the consumer by excluding the directory from the Spotless walk. Verified: the jar still builds; tests/ is no longer created at configuration time; :app:assembleV8Debug succeeds with the task absent from the graph; and `:common:compileV8DebugKotlin spotlessCheck` -- the exact invocation ADFA-5244 was filed for -- now passes on this branch, which carries no Spotless exclude at all. The two .gitignore entries that existed only for this task's output go too. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* ADFA-4928: Wire up Jetpack Compose in the app module Adds the Compose plugin/buildFeatures/dependencies to app/build.gradle.kts, mirroring the floating-window/profiler modules' setup, plus a shared ManagerTheme composable that resolves Theme.AndroidIDE's Material3 attrs (same technique as FloatingTheme). This is the first commit of the Plugin Manager + Template Manager merge (ADR 0009 requires new screens to be Compose); the theme/build wiring lands separately from any screen code so it's independently reviewable and buildable. * ADFA-4928: Port Plugin Manager screen to Compose Rebuilds PluginManagerActivity's screen in Jetpack Compose (ADR 0009), preserving every capability of the old RecyclerView/dialogs UI: install via SAF picker, enable/disable/uninstall, overwrite and signature-mismatch conflict handling, restart prompt, and the discover-plugins action. PluginManagerViewModel/PluginRepository are reused unchanged. The six long-press tooltip anchor points collapse to two (list items, and the screen's background/empty state) since they all showed the same TooltipTag.PLUGIN_MANAGER content anyway - verified on-device that the long-press still correctly reaches TooltipManager. Also moves two dialogs' hardcoded English strings (uninstall confirmation, plugin details labels) into string resources. Note: taken together with the prior commit, this is the buildable/ tested state; the prior commit's PluginListAdapter.kt deletion was accidentally bundled with the build-wiring commit rather than this one, so that earlier commit alone doesn't compile in isolation - only the combined history does (verified via :app:assembleV8Debug and a manual on-device pass). * ADFA-4928: Add Templates data layer Ports the parsing/model layer from appdevforall/TemplateManagerPlugin (CgtTemplateReader, TemplateMetadata/CgtFileItem, plus their unit tests) into the app module as the basis for the new Templates tab. Adds TemplateRepository/TemplateRepositoryImpl, which reimplement the plugin's install/uninstall/delete semantics as direct file operations on Environment.TEMPLATES_DIR + the Downloads folder, since the host app doesn't need IdeTemplateService's plugin-facing permission gate. Provenance (bundled/plugin/user) is inferred from the same filename convention IdeTemplateServiceImpl/PluginProjectManager already use. Adds TemplateManagerViewModel (UDF shape matching PluginManagerViewModel) and a Koin di/TemplateModule, registered in IDEApplication alongside pluginModule. No UI yet - this commit is data-layer only. CgtTemplateReaderTest needs @RunWith(RobolectricTestRunner::class): org.json.JSONObject throws "not mocked" under a plain JVM unit test, same as other app-module tests that touch real android.jar classes. * ADFA-4928: Add Templates tab Compose UI Adds the Compose UI for the Templates tab, backed by the data layer from the previous commit: TemplateListItem (card - tapping only opens the multi-template sub-list, matching the reference plugin's design), TemplateManagerDialogs (delete confirmation, file-level details, per-template details, multi-template sub-list), and TemplateManagerScreen (content composable wiring the ViewModel's uiState/uiEffect, same long-press pointerInput tooltip shim as the Plugins tab, new TooltipTag.TEMPLATE_MANAGER). TemplateManagerScreen is content-only (no Scaffold/TopAppBar/FAB) - unlike the Plugins tab there's no install-flow FAB, matching the ported plugin's passive Downloads-folder scanning. It's meant to be composed as one tab's body inside the shared manager screen; wiring the two tabs together is the next commit. * ADFA-4928: Wire Plugins and Templates tabs together New ManagerScreen composable owns the shared Scaffold/TopAppBar/TabRow + HorizontalPager, hosting Plugins and Templates as pages (Plugins default). The FAB and discover-plugins action only render on the Plugins tab, since Templates is a passive Downloads-folder scan with no equivalent action. Refactors the old PluginManagerScreen into PluginManagerContent - a Scaffold-free content composable, matching TemplateManagerScreen's shape - so both tabs plug into ManagerScreen's single Scaffold instead of nesting their own. PluginManagerActivity now resolves both PluginManagerViewModel and TemplateManagerViewModel and renders ManagerScreen; its class name and entry points (Settings, the crash-recovery dialog) are unchanged. Updates ARCHITECTURE.md: this is the first production Compose screen in app (ADR 0009), and templates/manager is a new data-layer package. Verified end-to-end on a physical device: assembleV8Debug, installed APK, exercised both tabs from Settings -> Plugin Manager. Templates tab correctly scanned Environment.TEMPLATES_DIR + Downloads (found real pre-existing .cgt fixtures on the test device), and a full install/uninstall round-trip moved files between Downloads and TEMPLATES_DIR and refreshed the list correctly. No crashes. * ADFA-4928: Complete tab wiring (PluginManagerContent refactor + activity + docs) Finishes the previous commit: a staging mistake (a `git add` call hit a stale pathspec and aborted before reaching these files) left `81e3797ab` with only the new `ManagerScreen.kt` and a content-less file rename, referencing a `PluginManagerContent` composable that didn't exist yet in that commit alone - not independently buildable. This commit adds what was missed: the actual `PluginManagerContent.kt` refactor (Scaffold/TopAppBar/FAB stripped out, now content-only), `PluginManagerActivity.kt` wired to render `ManagerScreen` with both view models, the `ARCHITECTURE.md` updates, and the `title_manager` string. Combined history through this commit compiles (:app:compileV8DebugKotlin) and matches what was already verified end-to-end on-device in the previous message. * ADFA-4928: Rename the preferences entry to Extensions Manager The Settings entry that opens the merged Plugins/Templates screen was still titled "Plugin Manager" with a summary mentioning "extensions" (the old plugin-only wording). Renamed to "Extensions Manager" with a summary reflecting both tabs it now opens: "Manage IDE plugins and templates". Verified on-device: preferences list and the opened screen both render correctly. * ADFA-4928: Warm Application.filesDir off the main thread PluginModule's Koin factories called Context.filesDir directly, which does a real File.exists() check on every call, not just the first. That trips StrictMode's DiskReadViolation the first time the Extensions Manager screen resolves PluginRepository/PluginManagerViewModel on the main thread. Cache the resolved File once, off-main, during app startup (IDEApplication.cachedFilesDir), and have PluginModule read that instead - later reads are then a plain field access rather than a syscall. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4928: Narrow the plugin install file picker to .cgp-like files The SAF picker launched with "*/*", showing every file regardless of type. SAF filters by MIME, not extension, and .cgp has no registered MIME type, so the closest working filter is "application/octet-stream" - what document providers report for files with an unrecognized extension. This hides files with a known type (zips, jars, images, ...) while leaving .cgp files selectable. isSupportedPluginFile() still validates the actual pick, since this is an approximation, not an exact extension filter (SAF has no such thing). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4928: Harden TemplateRepositoryImpl file operations Address CodeRabbit review feedback on PR #1627: - Use SLF4J logging instead of android.util.Log - Narrow runCatching to expected I/O/parsing exceptions, rethrowing CancellationException instead of swallowing it - Refuse to install/uninstall over an existing same-name destination file instead of silently overwriting it - Treat a failed source-file delete as an install/uninstall failure and roll back the copied destination file * ADFA-4928: Keep plugin picker/install work off the main thread Address CodeRabbit review feedback on PR #1627: - Warm IDEApplication.cachedFilesDir on an IO thread before Koin starts, eliminating the race where pluginModule/templateModule could resolve it on the main thread first - Bound FileImage's bitmap decode with inSampleSize and move the file-existence check inside the IO dispatcher; narrow its catch to recoverable failures and let CancellationException propagate - Move the picked plugin file's name/extension validation (a ContentResolver IPC call for content:// URIs) off the picker callback and into PluginManagerViewModel on a background dispatcher, routed back through a new ShowInstallConfirmation effect - Replace android.util.Log with SLF4J logging in PluginManagerContent - Narrow the file-picker launch catch to ActivityNotFoundException and log it instead of silently swallowing any Exception * ADFA-4928: Fix manager UI correctness/accessibility issues Address CodeRabbit review feedback on PR #1627: - Avoid double system-bar insets by zeroing ManagerScreen's Scaffold contentWindowInsets, since the activity's root already applies them - Fix the back button's TalkBack announcement (was "Cancel") with a dedicated cd_navigate_back string - Wire long-press tooltips to the discover-plugins action and install FAB - Always show Uninstall for a listed plugin, even when it failed to load, so a broken plugin has a recovery action - Move the detail-row "label: value" format into a string resource so translators control ordering/punctuation - Only treat a template card as clickable when it bundles more than one template, instead of always exposing tap/press semantics - Use an Android plurals resource for the template count string instead of a fixed "templates" string - Buffer TemplateManagerViewModel's uiEffect channel and use send() instead of trySend() so effects aren't dropped before a collector is ready * ADFA-4928: Add KDoc for TemplateMetadata/CgtFileItem Address CodeRabbit review feedback on PR #1627: document the model contracts, including the meaning of installed/provenance and the one-archive-to-many-templates relationship. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4928: Enable JUnit Jupiter for app unit tests Address CodeRabbit review feedback on PR #1627 (matches the JUnit Jupiter + Truth strategy ARCHITECTURE.md already documents for unit tests, which the app module hadn't wired up yet): - Run app unit tests on the JUnit Platform, with the vintage engine so existing JUnit 4/Robolectric tests keep running unchanged - Migrate CgtFileItemTest (no Robolectric dependency) to org.junit.jupiter.api.Test with Truth assertions - Keep CgtTemplateReaderTest on JUnit 4/RobolectricTestRunner (no built-in Jupiter integration) but switch its assertions to Truth Verified all 22 app unit test classes still run under :app:testV8DebugUnitTest with 0 failures. * ADFA-4928: Fix regressions CodeRabbit's re-review found in prior fixes Address CodeRabbit follow-up review feedback on PR #1627: - Only run the cachedFilesDir warmup eagerly in onCreate() when credential-protected storage is already unlocked - the default Context.getFilesDir() throws during Direct Boot. When locked, warm it instead from CredentialProtectedApplicationLoader.load(), which only proceeds once that storage is confirmed accessible. - Base FileImage's inSampleSize loop on the larger image dimension instead of requiring both dimensions to exceed the target, so a wide-but-short (or tall-but-narrow) image still gets downsampled - Log FileImage's swallowed SecurityException/OutOfMemoryError icon-load failures via a throttled SLF4J warning, without logging the file path - Buffer PluginManagerViewModel's uiEffect channel and use send() instead of trySend(), same fix already applied to TemplateManagerViewModel, so effects (e.g. the new ShowInstallConfirmation) aren't dropped - Narrow UriExtensions.getFileName's second catch to SecurityException/ IllegalArgumentException instead of blanket Exception, so unexpected ContentResolver failures surface instead of being silently mislabeled as "Unknown File" (and then downstream as an unsupported plugin file); switch its logging to a class-scoped SLF4J logger * ADFA-4928: Address human review comments from hal-eisen-adfa - Disable the install FAB while a plugin install is in flight, so a second tap can't start a concurrent installPlugin() coroutine. The Compose ManagerScreen replaced the old Activity, which disabled the FAB via binding.fabInstallPlugin.isEnabled = !state.isInstalling; nothing carried that behavior over. - Fix PLUGIN_AUTHORING.md pointers left dangling by the PluginListAdapter.kt -> PluginListItem.kt/FileImage.kt migration. The delete-failure-handling and cachedFilesDir warmup comments from the same review were already addressed by prior commits on this branch; verified against current HEAD, no further changes needed. * ADFA-4928: Fix double system-bar insets and non-lifecycle-scoped effect collection - ManagerScreen's TopAppBar still used its default status-bar insets on top of PluginManagerActivity.onApplySystemBarInsets, which already pads the root view by the full system-bar insets (that padding doesn't consume the insets, so Compose saw them a second time). Zero out TopAppBar's windowInsets to match the Scaffold's contentWindowInsets, which was already zeroed. - PluginManagerContent and TemplateManagerScreen collected viewModel.uiEffect in a bare LaunchedEffect, so it kept collecting while the activity was stopped. A plugin install finishing while the app is backgrounded could then run DialogUtils.showRestartPrompt or a flashbar builder against a stopped activity. Wrap both collectors in repeatOnLifecycle(STARTED), matching the old repeatOnLifecycle(STARTED) pattern used elsewhere in the app. * ADFA-4928: Address second review pass from hal-eisen-adfa - Long-press tooltips on the FAB and Discover-plugins IconButton never fired: pointerInput(detectTapGestures) placed on the caller-side modifier loses the down event to the button's own internal clickable, which runs first on the Main pointer pass. Drive the tooltip off the button's own MutableInteractionSource instead (press duration vs LocalViewConfiguration's longPressTimeoutMillis), which observes the same press stream the button already dispatches rather than racing it for the raw pointer event. - CgtTemplateReader.readTemplates read a zip entry's bytes unbounded, so a corrupt/hostile .cgt sitting in the public Downloads folder could OOM the app; bound the read and throw IOException past 1 MiB. parseCgtFile also didn't catch IllegalArgumentException, which ZipInputStream.nextEntry throws for a non-UTF-8 entry name - that propagated out of the bare viewModelScope.launch in TemplateManagerViewModel.loadTemplates (no CoroutineExceptionHandler) and crashed the app. Both are now handled per-file, so one bad archive is skipped instead of failing the whole scan. - UriExtensions.getFileName's catch was narrowed to SecurityException/IllegalArgumentException, but a misbehaving content provider can throw other RuntimeExceptions from query()/getString() (CursorWindowAllocationException, a wrapped DeadObjectException, ...). Broadened back to Exception, since this is a best-effort display-name lookup, not a path that should ever crash the caller. - TemplateManagerDialogs' DetailRow still built "$label: $value" with string concatenation instead of the R.string.label_value fix that landed in the plugin dialog, and the optional-tags list hardcoded a non-ASCII "*" bullet in code (CLAUDE.md's ASCII rule). Added R.string.template_optional_tag and reused R.string.label_value. TemplateListItem's status/provenance row had the same hardcoded-separator shape; extracted it to R.string.label_separator. - PluginManagerActivity's try/catch around setContent no longer caught anything: setContent only registers the composable, and its lambda (where both ViewModels first resolve via Koin) runs at first layout, after onCreate has already returned past the catch. Force-resolve both `by viewModel()` delegates inside the try, before setContent. - PluginListItem.pluginVersionLabel duplicated CgtFileItem.versionLabel and disagreed with it on blank input (a stray "v" chip vs the tested ""). Deleted the duplicate and reused the tested helper. Added a CgtTemplateReaderTest regression case covering the bounded-read cap. Verified via :app:testV8DebugUnitTest (all passing) and spotlessCheck. * ADFA-4928: Guard FileImage's downsampling loop against a non-positive bound CodeRabbit flagged (2026-08-05 review, still unresolved) that decodeBounded()'s inSampleSize loop assumes maxDimensionPx > 0. If it's ever <= 0 - e.g. the 40.dp default rounding to a sub-pixel size at an unusual density - the loop condition (a non-negative quotient >= a non-positive bound) is permanently true, hanging on an unbounded doubling of inSampleSize instead of throwing. Skip the downsampling loop entirely in that case and decode at inSampleSize = 1. * ADFA-4928: Address third review pass from hal-eisen-adfa - Wrap the cachedFilesDir warm-up in runCatching in both IDEApplication.onCreate and CredentialProtectedApplicationLoader.load, so a filesDir failure unrelated to Direct Boot lock state (ADFA-2358) degrades to a disk read on first use instead of crashing the process. - Replace the Discover-plugins IconButton with a Box+combinedClickable so a single gesture detector owns long-press and click, and make the FAB consume a one-shot suppression flag set at the long-press timeout, so long-pressing either control shows the tooltip without also firing its click action. Verified on-device: long-press shows the tooltip and leaves the file picker/browser closed; a normal tap still opens each. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4928: Address fourth review pass from hal-eisen-adfa - Fix the FAB long-press suppression flag latching and eating a later, unrelated tap: reset it at press start instead of relying on the click to clear it, so a long press that never ends in a tap-up on the FAB (slide-off, or the tooltip popup stealing the gesture) can no longer swallow the next real Install tap. Blocking. - Restore accessibility parity the IconButton -> Box swap dropped for the Discover-plugins control: role = Role.Button and onLongClickLabel so TalkBack again announces it as a button and names the long-press action. - Broaden the plugin file-picker SAF filter to application/octet-stream, application/zip, */* - some providers report a .cgp as application/zip rather than octet-stream, which the single-type filter hid with no way to reach them. - Delete the dead TemplateOperation sealed class (zero references). - Wire TemplateManagerUiState.isLoading through installTemplate/ uninstallTemplate/confirmDeleteDownloadFile (previously only loadTemplates set it) and render it as a top-aligned LinearProgressIndicator, so install/ uninstall/delete get visible feedback between the tap and the flashbar. - Make the Templates and Plugins tab dialog state rememberSaveable, so rotating or switching tabs (HorizontalPager disposes the off-screen page's state) no longer silently dismisses an open confirmation dialog. Neither CgtFileItem nor PluginInfo is Parcelable, so state is keyed on the file path / plugin id and the live item is resolved from uiState at the point of use - this avoids adding Parcelable to plugin-api's public API surface. - Add TemplateRepositoryImpl tests pinning the two riskiest branches: a name collision must fail without touching either copy, and a failed delete after a successful copy must roll back to leave exactly one copy behind (both for installTemplate and uninstallTemplate). Add TemplateManagerViewModel tests covering the init load, install success/failure, and the effect buffering Channel.BUFFERED was chosen to protect. Not changed: the two threads Hal already marked resolved (cachedFilesDir runCatching, FAB/Discover long-press-vs-click) needed no further action. The template.manager/plugin.manager tooltip-body question is answered in a PR reply - the bundled documentation.db has neither tag, pre-existing and not fixable from this repo (the asset is fetched from an external URL, not seeded here). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ADFA-4928: Offer the add-extension button on both manager tabs The FAB and the discover-plugins action were both gated on the Plugins tab, justified by the Templates tab being "a passive scan of the Downloads folder with no equivalent action". Merging stage falsified that: ADFA-4934 brought in TemplateCollectionRepository and a tested .cgt install flow (confirm -> name conflict -> overwrite/rename), reachable until now only by opening a file from outside the app. The screen is the Extensions Manager, so one "+" means "add an extension" on either tab. The picked file is routed by extension and the owning tab is brought forward, so the result is visible where it landed. Discover stays Plugins-only - it opens a plugin catalog, which has no meaning on the Templates tab. - Move the SAF launcher from PluginManagerContent up to ManagerScreen. HorizontalPager disposes the off-screen page, so a launcher owned by the Plugins page would not exist while Templates is showing. Routing also brings the target tab forward *before* dispatching, because uiEffect is a receiveAsFlow() Channel with a single consumer that lives on that page. - Extract ExternalFileInstallDialogs from ExternalFileInstallScreen, so the manager reuses ADFA-4934's confirm/conflict/rename flow rather than growing a second one. The activity keeps its own finish behaviour via onFinish. - Keep .cgp on the existing ContentUri path instead of routing it through onReceived. That path hands the ViewModel a LocalFile temp copy, for which the install dialog's "delete installation file after install" checkbox is meaningless - picked plugins would have silently lost that option. - Drop the now-dead OpenFilePicker event, effect and openFilePicker(). - Add msg_unsupported_extension_file for a pick that is neither type. Verified on the emulator: the "+" is present on both tabs and Discover is not; picking a .cgt stays on Templates and opens the collection-install dialog naming all nine templates in the archive; picking a .cgp brings the Plugins tab forward with the plugin dialog and its delete-source checkbox; picking a .exp shows the new message. * ADFA-4928: Quote the archive name with backticks in the install message msg_template_installed was written as "%1$s" installed successfully, but Android strips unescaped double quotes from a string value, so the quotes never reached the screen - it rendered as `qa-hello installed successfully` with the name unquoted. Backticks are not stripped, and strings.xml is already inconsistent about quoting %1$s, so this avoids adding another escaped-quote variant. Verified on the emulator: the flashbar now reads `qa-hello` installed successfully. * ADFA-4928: Show one card per template archive, not one per file on disk scanTemplates() concatenated the two directory scans with no de-duplication, so a .cgt present in both the template store and Downloads produced two cards. They render identically apart from the status line - same title (which is the first bundled template's name, not the archive's), same filename, and neither shows a location - and the Downloads twin is a dead end, since installTemplate refuses to overwrite and its Install can only ever fail. Let the installed copy win. Names are compared case-insensitively to match the stricter of the two install paths (TemplateCollectionRepository .findExistingCollision), so any row still listed as "not installed" is one the user can actually install. Filtering happens before parsing, so a shadowed archive is not unzipped just to be discarded. Tests move to Robolectric: the new cases build real .cgt archives, and parsing one reaches org.json.JSONObject, a "not mocked" stub under plain android.jar - the same reason CgtTemplateReaderTest already uses it. Added cases cover the twin being hidden, case-insensitive matching, and - the one that catches a sloppy filter - downloads that are not twins surviving. Known interaction: with the twin hidden, uninstallTemplate still refuses while a same-named file sits in Downloads, and that file no longer has a row. The failure names it exactly ("A download named 'x.cgt' already exists in /storage/emulated/0/Download"), so it is recoverable; changing uninstall's semantics is deliberately left out of this change. Verified on the emulator by reproducing the reported state - qa-hello.cgt installed and a second copy pushed to Downloads - which previously showed two identical cards and now shows one, marked Installed. --------- Co-authored-by: yaturner <thursdaynext@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: jimturner-adfa <jim.turner@appdevforall.org>
Brings the branch up to date with stage (38 commits) to clear the merge conflict on the PR. One conflict, in app/build.gradle.kts: both sides added imports at the same point in the block. Kept both sets. The branch needs GRADLE_API_NAME_JAR_BR / GRADLE_API_NAME_JAR_ZIP / GRADLE_DISTRIBUTION_ARCHIVE_NAME for the toolchain version constants, and stage needs DefaultNativePlatform; all four are still referenced after the merge. constants.kt and gradle/libs.versions.toml auto-merged and were checked by hand: the on-device toolchain stays at AGP 9.3.1 / Gradle 9.6.1 / Kotlin 2.3.21, and agp-tooling stays 9.3.1. Verified with `./gradlew :app:help --offline` on the merged tree. Claude-Session: https://claude.ai/code/session_017HGpMsUzZ5wxCfMtDZ2HGP
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
10 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.
Moves the on-device toolchain to Gradle 9.6.1 + AGP 9.3.1 + Kotlin 2.3.21, which collapses the duplicate Kotlin compilers the device was shipping. Gradle 9.6.1 embeds Kotlin 2.3.21 and AGP 9's built-in Kotlin resolves the same 2.3.21 compiler, so the build-script compiler and the app compiler become one artifact.
Why these exact versions
The window is constrained from both ends:
InternalProblems, removed there)This is a one-way move: at 9.6.1 there is no falling back to AGP 8 without also moving Gradle back.
Why the model migration is in the same commit
Bumping
agp-toolingto match the device breaksbuilder-model-impl, and the migration can't land first either — overriding members that don't exist in AGP 8.13.1 won't compile. Splitting them would leave a red commit in the stack.AGP 9 removed
PrivacySandboxSdkInfoandAndroidProject.PROPERTY_ANDROID_SUPPORT_VERSION(value inlined asandroid.injected.studio.version), and addedmappingR8TextFile,mappingR8PartitionFile,keepRulesDirectories,aarKeepRulesDirectories.