From d279292e4887123ac27471ef5a1213ce4dfd4ccb Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Thu, 18 Jun 2026 16:02:10 -0400 Subject: [PATCH 01/12] ADFA-4357 Add agent & contributor documentation set Add a coordinated set of Markdown docs to onboard both human and AI contributors and to capture the project's architectural decisions. - CLAUDE.md: operational guide for Claude Code (build/test commands, ABI flavors, project constraints); points to ARCHITECTURE.md for architecture rather than duplicating it. - AGENTS.md: operational rules for agents (CI-vs-local, Jira CLI, SonarQube MCP, git message handling); persistence rule now points to ARCHITECTURE.md. - ARCHITECTURE.md: single source of truth for module layout, layering & data flow (UDF), dependency rules, tech stack, state management, and the testing strategy. - REVIEW.md: code-review coaching (exception handling vs the Sentry crash wrapper, LeakCanary leaks, StrictMode, OWASP, tests/coverage, analytics, duplication, docstrings, strings.xml). - SECURITY.md: how to avoid introducing new SonarQube/Snyk/Semgrep blocker findings; vulnerability classes for an Android/Kotlin IDE. - docs/adr/: 8 Architecture Decision Records (MADR/Nygard) plus an index covering persistence-without-Room, on-device builds via the Gradle Tooling API, the vendored toolchain, embedded Termux, per-ABI flavors, Koin DI, the StrictMode whitelist engine, and retaining the com.itsaky.androidide namespace. --- AGENTS.md | 33 ++++ ARCHITECTURE.md | 180 ++++++++++++++++++ CLAUDE.md | 43 +++++ REVIEW.md | 142 ++++++++++++++ SECURITY.md | 100 ++++++++++ docs/adr/0001-persistence-without-room.md | 42 ++++ ...on-device-builds-via-gradle-tooling-api.md | 44 +++++ .../0003-vendored-forked-desktop-toolchain.md | 39 ++++ docs/adr/0004-embedded-termux-runtime.md | 42 ++++ docs/adr/0005-per-abi-product-flavors.md | 38 ++++ docs/adr/0006-koin-dependency-injection.md | 36 ++++ docs/adr/0007-strictmode-whitelist-engine.md | 40 ++++ docs/adr/0008-retain-androidide-namespace.md | 35 ++++ docs/adr/README.md | 24 +++ 14 files changed, 838 insertions(+) create mode 100644 AGENTS.md create mode 100644 ARCHITECTURE.md create mode 100644 CLAUDE.md create mode 100644 REVIEW.md create mode 100644 SECURITY.md create mode 100644 docs/adr/0001-persistence-without-room.md create mode 100644 docs/adr/0002-on-device-builds-via-gradle-tooling-api.md create mode 100644 docs/adr/0003-vendored-forked-desktop-toolchain.md create mode 100644 docs/adr/0004-embedded-termux-runtime.md create mode 100644 docs/adr/0005-per-abi-product-flavors.md create mode 100644 docs/adr/0006-koin-dependency-injection.md create mode 100644 docs/adr/0007-strictmode-whitelist-engine.md create mode 100644 docs/adr/0008-retain-androidide-namespace.md create mode 100644 docs/adr/README.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000..296bc1cc5c --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,33 @@ +Code On The Go is an Android app which allows the user to edit, build and deploy their own new Android apps. It is an Integrated Development Environment similar to Eclipse or VSCode. + +There is at least one Android emulator available. Use `adb devices -l | grep -v offline` to find it. Then use the ANDROID_SERIAL environment variable. + +For new persistence, use SQLite or the filesystem — never add Room. (See ARCHITECTURE.md for the full persistence model and the one legacy Room exception.) + +Avoid adding dependencies - we probably already have everything loaded that you need. Look in build.gradle.kts for details. + +Always plan before building. As part of the plan, estimate the projected size of the change, both number of files and lines of code. When that estimate is greater than 500 lines of code or more than 10 files, you must organize the work into 2 or more smaller change sets, so that the user can help you make 2 or more PRs (Pull Requests). + +Always protect the two critical Android interaction locations: (1) the top bar containing the time, notification icons, and status icons, and (2) the bottom bar containing the home button, back button, and app selector. Never put UI elements of our app on top of those Android reserved areas. + +## Official/Public Actions Run in CI, Not Locally + +Anything official or public-facing must happen through version-controlled GitHub Actions (`.github/workflows/*.yml`) — never from the user's laptop or a local machine. This includes uploading SonarQube/SonarCloud analyses, publishing releases or artifacts, deploying, and pushing to external services. The repo holds tokens like `SONAR_TOKEN` as GitHub secrets scoped to those workflows by design; do not go hunting for them locally. When asked to run something like the sonar task on the local machine, treat it as **local verification only** (run the tests/build to confirm a fix works) and let the official analysis happen in CI. + +## Reading Jira Tickets + +Prefer the local authenticated `jira` CLI for reading tickets — it is configured via the `JIRA_API_TOKEN`, `JIRA_HOST`, and `JIRA_USER` environment variables. Example: `jira issue view ADFA-1234`. Do NOT start the Atlassian MCP OAuth flow for ticket reads; that heavyweight authentication is unnecessary when the CLI is available. + +## SonarQube MCP Server + +The sonarqube MCP server runs in a Docker container, so Docker must be running before launching Claude Code. Its first launch pulls a ~225MB image (`mcp/sonarqube:latest`), which exceeds Claude Code's 30s MCP handshake timeout — so the initial connect reports a timeout even though nothing is broken. Pre-pull the image (or let one launch finish) so subsequent `/mcp` reconnects succeed. A `docker system prune` removes the image and reintroduces the slow first launch. + +## Resolving CI Job Names + +When the user references a CI/CD job by name (e.g., "the sonar job", "the analyze workflow"), read `.github/workflows/*.yml` first. The workflow YAML is the authoritative source for the exact gradle/shell invocation — do not introspect gradle tasks or grep build files to reverse-engineer the command. + +## Multi-line git/gh Messages + +**Default to writing the body to a tempfile** via the Write tool, then `git commit -F /tmp/msg.txt` or `gh pr create --body-file /tmp/body.md`. Treat heredoc/`--body "$(cat < Audience: engineers working in this repository. This describes how the code is *actually* organized today, including the places where the codebase is mid-migration. Where a pattern is the agreed target for new code, it is called out explicitly. + +## Overview + +Code On The Go (CoGo) is a full Android IDE that runs **on the device** — it edits, builds, and deploys real Android apps offline, embedding a Termux toolchain and running an actual Gradle build in a separate process via the `tooling-api`. It is the maintained successor to AndroidIDE, so the codebase namespace is still `com.itsaky.androidide`. + +There is **no single architectural philosophy** across the whole app. It is a large, layered, View-based application where newer feature surfaces (plugin manager, AI agent, git, project list) follow a deliberate **Unidirectional Data Flow (UDF)** with Koin DI, `ViewModel` + `StateFlow`, sealed UI-state/effect types, and repositories — while older surfaces still use `LiveData` and talk to GreenRobot EventBus directly. New work should follow the UDF pattern documented below. + +## Core Architecture & Data Flow + +The intended layering for feature code is **UI → ViewModel → Repository → data source**, with state flowing up and events/intents flowing down. Dependencies are provided by Koin (`coreModule`, `pluginModule`) and constructor-injected into ViewModels. + +- **Data sources** — Room (`RecentProjectRoomDatabase` + DAO, `suspend` functions), raw SQLite (`SQLiteOpenHelper`, e.g. `localWebServer/WebServer`), the filesystem/preferences, the embedded `tooling-api` (on-device Gradle), and external clients (Gemini via the Google GenAI SDK, on-device llama.cpp, JGit). Most are exposed through `suspend` functions. +- **Repositories** — e.g. `agent/repository/GeminiRepository`, `repositories/PluginRepository`, `repositories/BreakpointRepository`. They wrap data sources and hide threading/IO from the ViewModel. +- **ViewModels** — run work in `viewModelScope` on `Dispatchers.IO`, hold a private `MutableStateFlow`/`MutableSharedFlow`, and expose read-only `StateFlow`/`SharedFlow`. One-shot effects (toasts, navigation, dialogs) go through a separate `SharedFlow` of a sealed `*UiEffect` type. +- **UI (Fragments / Activities / Views)** — collect state in a lifecycle-aware coroutine and render it; user actions are sent back to the ViewModel as method calls or sealed `*UiEvent` intents. The UI is **Android Views + Fragments + RecyclerView adapters**, not Jetpack Compose (`compose-preview` is a tool for previewing the *user's* Compose code, not CoGo's own UI). + +``` + ┌─────────────────────────────────────────────┐ + intents ↓ │ UI LAYER │ ↑ state + (UiEvent / │ Activity / Fragment / RecyclerView.Adapter │ (StateFlow) + method call) │ collects StateFlow, renders; emits intents │ ↑ effects + └───────────────┬─────────────────────────────┘ (UiEffect SharedFlow) + │ + ┌───────────────▼─────────────────────────────┐ + │ VIEWMODEL LAYER │ + │ viewModelScope + Dispatchers.IO │ + │ private MutableStateFlow ──► StateFlow │ + │ private MutableSharedFlow ─► UiEffect flow │ + │ sealed UiState / UiEvent / UiEffect / State │ + └───────────────┬─────────────────────────────┘ + │ suspend calls + ┌───────────────▼─────────────────────────────┐ + │ REPOSITORY LAYER │ + │ GeminiRepository / PluginRepository / ... │ + └───────────────┬─────────────────────────────┘ + │ + ┌───────────────────────┼────────────────────────────────────┐ + ▼ ▼ ▼ + ┌───────────┐ ┌───────────────┐ ┌──────────────────┐ + │ Room / │ │ tooling-api │ │ External clients │ + │ SQLite / │ │ (on-device │ │ Gemini · llama │ + │ FS / prefs│ │ Gradle build)│ │ · JGit │ + └───────────┘ └───────────────┘ └──────────────────┘ + + Cross-cutting: GreenRobot EventBus carries decoupled, app-wide events + (build progress, install results, editor signals) outside the UDF spine. +``` + +**EventBus is a deliberate side-channel.** Long-running, cross-module signals (build/install lifecycle, editor events) are broadcast via GreenRobot EventBus (`@Subscribe(threadMode = ThreadMode.MAIN)`) and the `eventbus-events` module's shared event types. Treat it as the integration bus *between* subsystems; do not use it to replace a ViewModel's own state inside a single screen. + +## Module Structure + +Strategy: **layer-and-subsystem based**, not feature-by-feature. The Gradle build has ~80 modules (`settings.gradle.kts`) plus three included composite builds. `app` is the integration point; the rest are libraries it composes. + +| Group | Modules | Responsibility | +|---|---|---| +| Application | `app` | The IDE itself — activities, fragments, services, DI, agent, web server. Wires everything together. | +| Build engine | `subprojects:tooling-api*`, `gradle-plugin*`, `subprojects:projects`, `subprojects:builder-model-impl` | Runs a real Gradle build of the user's project out-of-process and streams events back. | +| Language tooling | `lsp:{api,java,kotlin,xml,indexing,…}`, `lexers`, `editor*`, `editor-treesitter` | Language servers, indexing, the Sora-based editor and highlighting. | +| UI design tooling | `layouteditor`, `uidesigner`, `xml-inflater`, `vectormaster`, `compose-preview` | Visual/XML design surfaces for the *user's* app. | +| Shell | `termux:{termux-app,termux-shared,termux-view,termux-emulator}` | Embedded Termux shell and terminal. | +| Plugin system | `plugin-api`, `plugin-api:plugin-builder`, `plugin-manager` | In-app plugin SDK + manager (`plugin.json` manifest, permissions, extensions). | +| On-device AI | `llama-api`, `llama-impl` | llama.cpp integration, shipped as a per-flavor native AAR. | +| Cross-cutting | `eventbus`, `eventbus-android`, `eventbus-events`, `common`, `common-ui`, `logger`, `resources`, `preferences`, `shared` | Shared infra and the event bus. | +| Testing | `testing:{android,unit,lsp,tooling,common}` | Shared test harnesses, split by what's under test. | + +**Dependency rules (enforced):** +- **`app` depends inward; libraries never depend on `app`.** Subsystems are consumed by `app`, not vice versa. +- **Vendored forks are substituted, not imported ad hoc.** `composite-builds/build-deps` and `build-deps-common` provide forked `javac`/`jdt`/`layoutlib`/etc.; `settings.gradle.kts` substitutes them in for `com.itsaky.androidide.build:*`. Don't add a Maven coordinate for something already substituted. +- **All module config flows through `composite-builds/build-logic`.** Every Android module gets the `v7`/`v8` ABI flavors centrally (`AndroidModuleConf.kt`) — there is no flavorless `assembleDebug`. `:plugin-api` is intentionally excluded from flavors. +- **`RepositoriesMode.FAIL_ON_PROJECT_REPOS`** — repositories are declared once in `settings.gradle.kts`; modules must not declare their own. +- **Avoid new dependencies.** Check `gradle/libs.versions.toml` first; the build almost certainly already has it. + +## Build & Module Configuration + +These structural facts shape every module. The day-to-day build *commands* live in `CLAUDE.md`; the rules that produce those commands live here. + +- **Centralized convention logic.** All Android module setup flows through `composite-builds/build-logic` (`conf/AndroidModuleConf.kt`). Modules stay thin; configuration is shared, so understanding any module's setup starts here. +- **ABI product flavors.** Every Android module *except* `:plugin-api` gets two flavors on the `abi` dimension — `v7` (`armeabi-v7a`) and `v8` (`arm64-v8a`) — defined centrally. There is no flavorless variant; tasks are `assembleV8Debug`, `assembleV7Release`, etc. +- **SDK levels** (`build-logic/.../build/config/BuildConfig.kt`): `COMPILE_SDK=36`, `MIN_SDK=28`, `TARGET_SDK=28`. `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16` is the floor for the apps a *user* builds with CoGo — distinct from CoGo's own `MIN_SDK`. +- **Native asset bundling.** The on-device LLM (`llama-impl`) ships as a per-flavor native AAR, wired through the root `build.gradle.kts` (`bundleLlamaV8Assets` / `assembleV8Assets`, …); prebuilt per-flavor assets live under `assets/release/v7/` and `assets/release/v8/`. +- **`app` package layout is by concern, not feature:** `activities`, `fragments`, `services`, `di`, `agent`, `viewmodel(s)`, `repositories`, `roomData`, `localWebServer`, `preferences`, `ui`, `utils`, …. + +## Technology Stack + +| Concern | Library / Approach | +|---|---| +| UI | Android Views + Fragments + `RecyclerView` (Material Components). **Not Compose** for the IDE's own UI. | +| Dependency Injection | **Koin** (`org.koin`) — `coreModule`/`pluginModule`, `startKoin` in `IDEApplication`, plus a `ServiceLocator : KoinComponent` for lazy post-startup access. No Hilt/Dagger. | +| Asynchronous work | **Kotlin Coroutines + Flow** (`StateFlow`/`SharedFlow`, `viewModelScope`, app-scoped `CoroutineScope(SupervisorJob() + Dispatchers.IO)`); **GreenRobot EventBus** for cross-subsystem events. | +| Networking | Offline-first; no general REST layer. External I/O is **Google GenAI SDK** (Gemini), **on-device llama.cpp**, and **JGit** (git). Retrofit is in the catalog but effectively unused in app code. | +| Database / Persistence | **Raw SQLite** (`SQLiteOpenHelper` / `SQLiteDatabase`) and **filesystem + preferences** for almost everything. **Room** survives in exactly one place — the Recent Projects feature (see policy below). | +| Serialization | `kotlinx.serialization` and Gson. | +| AI agent | Google GenAI (cloud) + llama (local), behind `GeminiRepository` / `SwitchableGeminiRepository`, with planner/critic/executor agents in `agent/repository`. | + +> **Persistence policy (authoritative):** new persistence must use **raw SQLite or the filesystem/preferences — not Room.** Do not add Room entities or extend it to new tables. +> +> Room is used in exactly **one** place: the **Recent Projects** feature in the `app` module — `app/src/main/java/com/itsaky/androidide/roomData/recentproject/` (`RecentProjectRoomDatabase`, `@Database version = 4` with migrations 1→4; `RecentProjectDao`; the `RecentProject` `@Entity` → table `recent_project_table`). It's provided via Koin in `di/AppModule.kt` and consumed by `MainViewModel`, `RecentProjectsViewModel`, `MainActivity`, `ProjectInfoBottomSheet`, and `ProjectCreationManager`. +> +> Two things look like Room but aren't: `idetooltips` declares the Room Gradle deps but doesn't use them (its tooltip store is raw `SQLiteDatabase` in `ToolTipManager`), and the Room coordinates in `editor`'s `GroovyAutoComplete` are autocomplete suggestions for the *user's* code, not CoGo's own usage. + +## State Management + +- **UI state is a single immutable `data class`** exposed as a `StateFlow<…UiState>`; the ViewModel mutates a private `MutableStateFlow` via `update { it.copy(...) }`. Derived booleans live as computed properties on the state class (so the UI stays dumb). +- **One-shot effects** (errors, navigation, dialogs, restart prompts) are modeled as a **sealed `…UiEffect`** emitted through a separate `SharedFlow` — never folded into the persistent state, so they don't replay on rotation. +- **Inbound intents** are a sealed `…UiEvent` (or direct ViewModel method calls on older screens). +- **Process/long-task state** uses dedicated sealed hierarchies — e.g. `BuildState`, `TaskState`, `InstallationState`, `ApkInstallationViewModel.SessionState`, `agent/AgentState`. +- **Legacy screens** still expose `LiveData` (~8 ViewModels) instead of `StateFlow` (~20). When touching one substantially, prefer migrating it to `StateFlow`. + +Real example from `ui/models/PluginManagerUiState.kt` — the pattern to copy for new screens: + +```kotlin +// Persistent, immutable UI state — exposed as StateFlow +data class PluginManagerUiState( + val isLoading: Boolean = false, + val plugins: List = emptyList(), + val isPluginManagerAvailable: Boolean = false, + val isInstalling: Boolean = false, +) { + val isEmpty: Boolean get() = plugins.isEmpty() && !isLoading // derived in state + val showEmptyState: Boolean get() = isEmpty && isPluginManagerAvailable +} + +// Inbound intents from the UI +sealed class PluginManagerUiEvent { + object LoadPlugins : PluginManagerUiEvent() + data class EnablePlugin(val pluginId: String) : PluginManagerUiEvent() + data class InstallPlugin(val uri: Uri, val deleteSourceAfterInstall: Boolean) : PluginManagerUiEvent() + // ... +} + +// One-shot effects — emitted via a SharedFlow, not stored in UiState +sealed class PluginManagerUiEffect { + data class ShowError(@StringRes val messageResId: Int, val formatArgs: List = emptyList()) : PluginManagerUiEffect() + object ShowRestartPrompt : PluginManagerUiEffect() + // ... +} +``` + +Typical ViewModel wiring: + +```kotlin +private val _uiState = MutableStateFlow(PluginManagerUiState()) +val uiState: StateFlow = _uiState.asStateFlow() + +private val _effects = MutableSharedFlow() +val effects: SharedFlow = _effects.asSharedFlow() + +fun onEvent(event: PluginManagerUiEvent) = viewModelScope.launch(Dispatchers.IO) { + when (event) { + PluginManagerUiEvent.LoadPlugins -> { + _uiState.update { it.copy(isLoading = true) } + val plugins = repository.loadPlugins() + _uiState.update { it.copy(isLoading = false, plugins = plugins) } + } + // ... + } +} +``` + +## Testing Guidelines + +Test code lives both alongside each module and in the shared `testing:{unit,android,lsp,tooling,common}` harnesses. Run with the flox wrapper, e.g. +`flox activate -d flox/local -- ./gradlew :testing:unit:test` or a module's `:module:test --tests "…"`. + +| Layer | Runner / Tools | What to test | +|---|---|---| +| Unit (pure JVM) | **JUnit Jupiter (5)**, some legacy **JUnit 4**; assertions via **Google Truth**; mocking via **MockK** (primary) and **Mockito-Kotlin** (legacy) | ViewModels (state transitions over a fake repository), repositories, parsers, builder/tooling logic. Keep these off the device. | +| JVM + Android framework | **Robolectric** | Code needing `Context`/resources/`SQLiteOpenHelper` without an emulator. | +| Instrumented / UI | **Espresso** + **AndroidX Test** + **UiAutomator**, run under **Test Orchestrator**; `mockk-android` for on-device mocks | End-to-end IDE flows (create/build/deploy, editor, terminal). | + +Preferences and conventions: +- **Assertions: Google Truth** (`assertThat(x).isEqualTo(...)`) over raw JUnit asserts. +- **Mocking: MockK** for new code; relax it deliberately rather than over-stubbing. +- For UDF ViewModels, drive `onEvent(...)`/method calls against a fake or mocked repository and assert the emitted `UiState` sequence (collect the `StateFlow`); assert effects by collecting the effect `SharedFlow`. +- On the M2 emulator, prefer short `flakySafely` timeouts (2–3s) and `AccessibilityNodeInfo.ACTION_CLICK` over raw coordinate taps — the bottom system-bar region swallows coordinate clicks, and the two system bars (top status, bottom nav) must never be obstructed by tests. +``` diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000000..2a6c013e5c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,43 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +Code On The Go (CoGo) is an Android IDE that runs *on an Android device* — it lets users edit, build, and deploy real Android apps from a phone, offline. It is the actively-maintained successor to AndroidIDE, so the Gradle/AGP namespace is still `com.itsaky.androidide` throughout the codebase even though the product is "Code On The Go". A bundled Termux provides the shell/toolchain, and a `tooling-api` runs a real Gradle build inside the app. + +`AGENTS.md` holds operational rules (branch naming, Jira CLI, SonarQube MCP, session/push protocol). Read it — this file does not duplicate it. + +## Build & test + +All Gradle invocations must be wrapped in `flox` for the correct toolchain: + +```bash +flox activate -d flox/local -- ./gradlew +``` + +Common tasks: +- **Debug APK (arm64, the usual target):** `flox activate -d flox/local -- ./gradlew :app:assembleV8Debug --parallel --max-workers=6` +- **Single unit test:** `flox activate -d flox/local -- ./gradlew :module:test --tests "com.itsaky.androidide.SomeTest"` +- **Module unit tests:** `flox activate -d flox/local -- ./gradlew :testing:unit:test` + +When a CI/CD job is referenced by name ("the sonar job", "the analyze workflow"), `.github/workflows/*.yml` is the authoritative source for the exact command — don't reverse-engineer it from Gradle tasks. Official/public actions (Sonar upload, releases, deploys) run only in CI, never locally; local runs are verification only. + +### ABI product flavors (v7 / v8) + +Every Android module carries `v7` (`armeabi-v7a`) and `v8` (`arm64-v8a`) flavors, so build tasks are `assembleV8Debug`, `assembleV7Release`, etc. — there is **no** flavorless `assembleDebug`. See [ARCHITECTURE.md](ARCHITECTURE.md) (Build & Module Configuration) for how flavors, native asset bundling, and SDK levels are configured. + +## Architecture + +See **[ARCHITECTURE.md](ARCHITECTURE.md)** — the single source of truth for the module map, layering and data flow, dependency rules, the technology stack (DI, async, persistence, networking), state management, and the testing strategy. Don't re-document those here; update ARCHITECTURE.md instead. + +## Project-specific constraints + +- **Avoid new dependencies** — the build almost certainly already has what's needed. Check `gradle/libs.versions.toml` and `build.gradle.kts` first. +- **Protect the two Android system bars** in any UI work: the top status bar (clock, notifications, status icons) and the bottom navigation bar (home, back, recents). Don't draw over or intercept them. +- **Plan and size before building.** If a change is estimated at >500 LOC or >10 files, split it into 2+ smaller PRs. +- `.androidide_root` is a sentinel file used by tests to locate the project root — do not delete it. + +## Code style + +2-space indents everywhere. Java uses Google style (`google-java-format`); Kotlin uses `ktfmt` Google-internal style; XML uses the Android Studio formatter. Branch names must match `.../ADFA-#####` (3–5 digits) — see CONTRIBUTING.md; a pre-commit hook enforces it (`sh ./scripts/install-git-hooks.sh`). diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 0000000000..7d3e131994 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,142 @@ +# Code Review Guide + +How to give a good review on Code On The Go. This is a coaching doc, not a gate — use judgment, explain the *why*, and prefer a concrete suggestion (or a diff) over "this is wrong." It complements, and does not replace, [ARCHITECTURE.md](ARCHITECTURE.md) (the source of truth for patterns) and the operational rules in `AGENTS.md` / `CLAUDE.md`. + +## How to use this + +- **Author:** self-review against this list *before* requesting review. Most of it you can check in five minutes. +- **Reviewer:** you own correctness, leaks, security, and tests. Don't rubber-stamp; don't bikeshed style the formatter already enforces (`ktfmt` / `google-java-format`, 2-space indents). +- Tie every blocking comment to a concrete risk (a crash, a leak, a CVE class, an untested branch). Tag non-blocking polish as **nit:** so the author can triage. + +## The 60-second checklist + +- [ ] **Exceptions** are handled locally — nothing unexpected reaches the global Sentry crash handler. +- [ ] **No leaks** LeakCanary would catch later: every register/open/subscribe has a matching unregister/close in the right lifecycle callback. +- [ ] **No main-thread disk/network I/O** — no new StrictMode violations, and no whitelisting of *our own* code. +- [ ] **Security:** untrusted input (zip entries, URLs, file paths, web-server requests) is validated; no secrets in code, logs, or analytics. +- [ ] **Tests:** non-UI logic has unit coverage. Where coverage is thin, there's logging to diagnose it in the field. +- [ ] **No duplication:** the change reuses existing helpers instead of copy-pasting. +- [ ] **Docs:** public classes/functions have KDoc/Javadoc explaining *why*, not *what*. +- [ ] **Strings** are in `strings.xml`, not inline literals. +- [ ] **Analytics:** meaningful user/build actions emit an event (see below). +- [ ] **Scope/size:** PR is focused and within the ~500 LOC / 10-file guideline (`AGENTS.md`). + +--- + +## 1. Exception handling — stay out of the Sentry crash wrapper + +`IDEApplication` installs a global uncaught-exception handler (`handleUncaughtException`) that reports to **Sentry** and then runs the device/credential-protected loaders' handlers. An exception that escapes your code lands there and is recorded as a **crash**. That handler is a safety net, not a control-flow tool. + +- **Catch where you can recover.** Wrap I/O, parsing, IPC to the `tooling-api`, git, and plugin calls. Convert failures into a sealed error state (`…UiEffect.ShowError`, `Result`, `BuildState.Failed`) the UI can render. +- **Never swallow silently.** A bare `catch (e: Exception) {}` hides bugs. At minimum log it; if it's notable-but-handled, report it explicitly with the established idiom: + ```kotlin + } catch (e: IOException) { + log.error("Failed to install APK for {}", projectName, e) + Sentry.captureException(e) // handled, but we want visibility + _uiState.update { it.copy(error = e.toUserMessage()) } + } + ``` +- **Catch narrowly.** Prefer specific types over `Throwable`/`Exception`. Don't catch `CancellationException` in coroutines — rethrow it so structured cancellation works. +- **Coroutines:** an uncaught exception in a `launch` propagates to the scope's handler. Handle it inside the coroutine; don't rely on the crash wrapper to mop up. + +## 2. Memory & resource leaks — fix them before LeakCanary does + +LeakCanary runs in debug builds (`debugImplementation`), so leaks *will* surface — catch them in review first. Common offenders here: + +- **EventBus:** every `EventBus.getDefault().register(this)` needs an `unregister(this)` in the symmetric lifecycle callback (`onStart`/`onStop`, `onResume`/`onPause`). Missing unregister = leak **and** stale-event bugs. +- **Context:** don't hold an `Activity`/`Fragment`/`View` context in a `single` (Koin), a long-lived object, or a companion. Use `applicationContext` for anything outliving a screen. +- **Listeners/receivers/observers:** `BroadcastReceiver`, `ServiceConnection`, `ContentObserver`, editor/terminal callbacks — unregister them. We have real examples (`AppLogsCoordinator`, the log service connection); follow that lifecycle. +- **Closeables:** files, cursors, streams, the `tooling-api` connection — use `use {}` or close in `finally`. +- **Coroutines:** launch in `viewModelScope` / `lifecycleScope`, never `GlobalScope`. A scope tied to a destroyed component must be cancelled. +- **Bitmaps/large buffers** (APK viewer, image-to-XML, previews): release them; don't cache unbounded. + +## 3. Threading & StrictMode — don't whitelist our own sins + +The app runs a real StrictMode policy via `StrictModeManager` with a **whitelist engine**. Project rule (see learnings/memory): **the whitelist is only for vendor/framework code we can't change — never for app-owned violations.** If your code trips StrictMode, fix the code. + +- Move disk and network I/O off the main thread (`Dispatchers.IO`, `withContext`). No DB reads, file reads, or `SharedPreferences` first-access on the UI thread. +- Don't reach for `allowThreadDiskReads()` / `permitAll()` to silence a violation in our code. +- Touch UI only on the main thread; post results back via `StateFlow`/`withContext(Dispatchers.Main)`. + +## 4. Security — OWASP Top Ten for a mobile IDE + +This app extracts archives, runs a local web server, stores git credentials and signing keys, and executes builds — so the attack surface is real. Review accordingly: + +- **Injection / path traversal (Zip Slip):** template/project extraction (`ZipRecipeExecutor`) and any unzip must reject entries that resolve outside the target dir (`canonicalPath.startsWith(targetDir)`). Validate file paths built from user/project input. +- **SQL:** use parameterized queries (`rawQuery(sql, args)` with `?` placeholders), never string-concatenated SQL. (Existing `WebServer`/tooltip queries already do this — match them.) +- **Secrets & credential storage:** git tokens, keystore/signing passwords → `EncryptedSharedPreferences` / the Android Keystore, never plaintext files, never committed, **never logged or sent to analytics/Sentry**. Scrub secrets from breadcrumbs and exception messages. +- **Local web server (`WebServer`):** bind to loopback, scope what it serves, and don't reflect unsanitized input into responses. Treat every request as untrusted. +- **Network:** HTTPS only; no disabled TLS/hostname verification; verify git remotes. +- **Untrusted code/plugins:** respect the `plugin.json` permission model; don't widen plugin capabilities or load classes from untrusted sources without the manager's checks. +- **Deserialization & intents:** validate parsed JSON (Gson/kotlinx) and incoming `Intent`/deep-link extras; don't trust their shape. +- **Logging leaks:** PII, file contents, tokens, and full request bodies don't belong in logs. + +## 5. Tests & coverage + +**If the code is not purely UI, expect unit tests in the same PR.** ViewModels, repositories, parsers, builder/tooling logic, and security-sensitive helpers are all testable off-device. + +- Test the *behavior*: drive `onEvent(...)` / methods against a fake or MockK'd repository and assert the emitted `UiState` sequence (and effects via the effect `SharedFlow`). See ARCHITECTURE.md → Testing. +- Tools: **JUnit (Jupiter)**, **Truth** assertions, **MockK** (new) / Mockito-Kotlin (legacy), **Robolectric** for framework-dependent JVM tests. Live in the module or the shared `testing:*` harnesses. +- Cover the **error and edge paths**, not just the happy path — those are exactly what the crash wrapper would otherwise catch in production. +- Pure-UI changes (layout, view wiring) may legitimately have no unit test; say so in the PR description so the reviewer isn't guessing. + +### When coverage is unavoidably low, add logging +Some code is genuinely hard to unit-test (IPC with the `tooling-api`, native/llama boundaries, terminal/PTY, real device I/O). When you can't test a non-trivial branch, **instrument it** so it's diagnosable in the field — log decision points and failure modes via SLF4J: +```kotlin +private val log = LoggerFactory.getLogger(MyClass::class.java) +... +log.info("Selected build strategy {} for project {}", strategy, projectName) +log.warn("Falling back to {} after {}", fallback, reason) +``` +Logging is a **stopgap that buys observability, not a substitute for a test.** Prefer a test where one is feasible. + +## 6. Observability — logging & analytics + +**Logging:** use SLF4J (`LoggerFactory.getLogger(Class::class.java)`), not `android.util.Log`. Right level (`debug` for flow, `info` for milestones, `warn`/`error` for problems), structured `{}` placeholders, and pass the throwable as the last arg (don't `"$e"`). No secrets/PII (see §4). + +**Firebase Analytics:** events go through the injected `IAnalyticsManager` (Koin `single`), not `FirebaseAnalytics` directly. When a PR adds a meaningful user or build action, ask whether it deserves an event. Good candidate locations: + +| Where | Call | +|---|---| +| Screen/fragment shown | `trackScreenView(name)` | +| Project created / opened / cloned | `trackProjectOpened(path)` | +| A feature is actually used (editor action, layout designer, plugin install, AI agent run) | `trackFeatureUsed(name)` | +| Build started / completed / strategy chosen | `trackBuildRun(...)`, `trackBuildCompleted(...)`, `trackGradleStrategySelected(...)` | +| Recoverable, notable error | `trackError(type, message)` | + +Keep event names/params stable and low-cardinality; **no PII, file paths with usernames, or secrets** in event payloads. + +## 7. Code quality + +- **No duplication.** If you copy-pasted a block, extract a function/extension into the right `common`/`utils` module. Before adding a helper, grep — we likely already have it. Repeated literals/magic numbers → named constants. +- **Docstrings.** Public classes, functions, and non-obvious logic get KDoc/Javadoc. Document the *contract and the why* (threading expectations, nullability, side effects, units), not a restatement of the signature. +- **Strings in `strings.xml`.** User-facing text must be a string resource, never an inline literal — lint flags `HardcodedText`, and externalized strings feed our Crowdin translation flow. Use plurals/`getQuantityString` and positional args for formatting. Log messages and analytics keys are *not* user-facing and stay in code. +- **Dependencies:** don't add one without checking `gradle/libs.versions.toml` first — we probably already have it (`AGENTS.md`). + +## 8. Architecture alignment + +Hold the change to the patterns in [ARCHITECTURE.md](ARCHITECTURE.md): +- New screens follow **UDF**: `ViewModel` + `StateFlow`, sealed `UiEvent`/`UiEffect`, repository for data. Don't put I/O or business logic in Fragments/Activities. +- DI via **Koin** (constructor injection); register new singletons/viewModels in the module. +- **Persistence:** raw SQLite or filesystem — **not Room** (Recent Projects is the lone legacy exception). +- **UI safety:** never place our UI over the two Android system bars — the top status bar and the bottom navigation bar (`AGENTS.md`). + +## 9. PR hygiene + +- Focused and reviewable: aim for the **~500 LOC / 10-file** ceiling; split larger work into stacked PRs. +- Title/branch follow `ADFA-####`; description says *what changed, why, how it was verified*, and flags anything intentionally out of scope (e.g. "UI-only, no unit test"). +- No stray debug logs, commented-out code, `dummy.apk`-style artifacts, or unrelated reformatting that buries the real diff. + +--- + +## Open for discussion (proposed additions) + +These aren't established rules yet — flagging them as candidates for the team: + +- **Accessibility:** require `contentDescription` on actionable views and adequate touch targets. (We already lean on accessibility actions in UI tests — `ACTION_CLICK` — so this is low-cost and improves both a11y and test stability.) +- **Backward compatibility:** `MIN_SDK=28` — review new APIs for guard/desugaring; remember user-built apps target `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16`. +- **Performance budget for startup & editor:** watch added work in `Application.onCreate`, `tooling-api` startup, and per-keystroke editor paths; flag synchronous heavy work there. +- **Offline-first:** CoGo is meant to work without a network. New features should degrade gracefully offline; analytics/Sentry/Gemini calls must be non-blocking and failure-tolerant. +- **Feature flags / kill switches** for risky surfaces (AI agent, plugins, web server) so we can disable in the field without a release. + +Add to or push back on any of these — this doc is meant to evolve with the team. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000000..09479ff4d2 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,100 @@ +# Security Guide + +This is a developer guide for **not introducing new security findings** that our scanners — **SonarQube/SonarCloud**, **Snyk**, and **Semgrep** — flag. It explains the vulnerability classes those tools catch in a Kotlin/Java Android app like ours, with concrete do/don't guidance. For the per-PR checklist, see [REVIEW.md](REVIEW.md) §4; for vulnerability disclosure, see [Reporting](#reporting-a-vulnerability) at the bottom. + +## The policy: the baseline is frozen, don't grow it + +We currently carry a large baseline of **blocker-severity** findings that have been suppressed / marked "won't fix" so the build is workable. That baseline is **accepted technical debt — it is closed.** + +- **Your change must not add a new blocker (or high) finding in SonarCloud, Snyk, or Semgrep.** "New code" is what's measured; clean new code is the bar. +- **Do not extend the whitelist/suppression to cover your own code.** Suppression is reserved for **vendored / third-party code we don't own** (the same philosophy as our StrictMode whitelist: fix what we own, suppress only what we can't change). A suppression on app-owned code in a PR is a red flag, not a fix. +- If a tool flags something you believe is a false positive, get explicit reviewer sign-off and annotate *why* — don't silently suppress. + +Most of our existing blockers live in vendored subtrees (the `javac`/`jdt` forks, `termux`, native libs). The goal of this doc is to keep **our** code — `app`, plugins, feature modules — clean. + +--- + +## What the three tools catch (and where they differ) + +All three overlap heavily on code-level vulnerabilities; each has a distinct strength: + +| Tool | Strongest at | In our repo | +|---|---|---| +| **SonarQube / SonarCloud** | Broad Java/Kotlin **quality + security**, Android-aware rules, security hotspots. Most of our blocker count. | Runs in CI (`analyze.yml`) against SonarCloud project `appdevforall_CodeOnTheGo`. | +| **Snyk** | **Vulnerable dependencies (SCA)** — known CVEs in transitive libraries — plus Snyk Code SAST. | Watches our Gradle dependency graph; this is the one that fires on a bad library version. | +| **Semgrep** | **Pattern + taint-flow SAST** — tracks untrusted data from source to dangerous sink; highly customizable rules. | OWASP / Android / secrets rulesets. | + +If you only remember one distinction: **Snyk is mostly about the libraries you pull in; Sonar and Semgrep are mostly about the code you write.** + +--- + +## The vulnerability classes to avoid + +These are the categories that produce blocker/high findings in apps like ours. Representative SonarQube rule IDs are given where well-known (IDs and severities can shift between versions — treat them as pointers). + +### 1. Hardcoded secrets & credentials +*(Sonar S2068/S6418; Snyk Code; Semgrep `secrets`)* +- **Don't** put API keys, tokens, passwords, signing-key passwords, or `google-services.json` values in source, resources, or committed config. All three tools scan for high-entropy strings and known key formats. +- **Do** read secrets from `BuildConfig` fields injected at build time (CI secrets), the Android Keystore, or `EncryptedSharedPreferences`. Git tokens and keystore passwords (`GitCredentialsManager`, the keystore generator) must never be plaintext or logged. + +### 2. Injection +- **SQL injection** *(Sonar S3649)* — never concatenate user/project input into SQL. Use parameterized queries: `db.rawQuery(sql, arrayOf(arg))` with `?` placeholders (our `WebServer`/tooltip queries already do this — match them). +- **OS command injection** *(Sonar S2076; Semgrep taint)* — building shell strings from untrusted input is the classic finding. We run a real shell (Termux) and Gradle; pass arguments as a list/array, never an interpolated command string, and validate anything derived from a project path or filename. +- **Path traversal / Zip Slip** *(Sonar S2083 / S6096)* — extracting archives is a top hit for us. Every zip/tar entry (templates via `ZipRecipeExecutor`, imported projects, plugin packages) must be resolved and checked to stay inside the target dir before writing: + ```kotlin + val outFile = File(targetDir, entry.name) + if (!outFile.canonicalPath.startsWith(targetDir.canonicalPath + File.separator)) { + throw SecurityException("Zip Slip blocked: ${entry.name}") + } + ``` + Same idea for any `File(base, userControlledName)`. + +### 3. Weak cryptography & randomness +- **Weak hashes** *(Sonar S4790)* — MD5/SHA-1 for anything security-relevant. Use SHA-256+. (MD5 for a non-security cache key is fine but will still get flagged — prefer a non-crypto hash there.) +- **Weak ciphers / modes** *(Sonar S5542/S5547)* — no DES/3DES/RC4, no ECB mode, no hardcoded IVs *(S3329)*. Use AES-GCM with a random IV. +- **Insecure randomness** *(Sonar S2245)* — `java.util.Random` / `Math.random()` for tokens, keys, nonces, or filenames-as-secrets. Use `java.security.SecureRandom`. +- Prefer Jetpack Security / Keystore primitives over rolling your own crypto. + +### 4. Insecure network & transport +- **Cleartext traffic** *(Sonar S5332)* — no `http://`, `ftp://`, `ws://` for anything but localhost. Use HTTPS/WSS. +- **Disabled TLS validation** *(Sonar S4830 / S5527)* — never install a trust-all `TrustManager`, an `ALLOW_ALL` `HostnameVerifier`, or override `onReceivedSslError` to proceed. These are guaranteed blockers and a real MITM risk for git/clone and update flows. +- Don't widen `network_security_config` to permit cleartext or user CAs broadly. + +### 5. WebView & the local web server +We render content in WebViews (tooltips, markdown preview, APK viewer) and run a local `WebServer` — both are scanner magnets. +- *(Sonar S6362)* `setJavaScriptEnabled(true)` only when required, and never on a WebView loading **remote/untrusted** content. +- **`addJavascriptInterface`** with untrusted content is a remote-code-execution class finding — avoid it; if unavoidable, gate to `@JavascriptInterface` methods and trusted local content only. +- *(Sonar S6363)* don't enable `setAllowFileAccess` / `setAllowUniversalAccessFromFileURLs` unless strictly needed. +- The local `WebServer`: bind to **loopback only**, serve a scoped directory, don't reflect unsanitized request input into responses (XSS/SSRF), and treat every request as untrusted. + +### 6. Android component & data exposure +- **Exported components** — `activity`/`service`/`receiver`/`provider` with `android:exported="true"` and no permission is a standard finding. Export only what must be, and protect it with a signature-level permission. Validate all incoming `Intent` extras (intent-redirection / spoofing). +- **PendingIntent** — must be `FLAG_IMMUTABLE` unless mutability is genuinely required. +- **Insecure storage** — no `MODE_WORLD_READABLE/WRITEABLE`; don't put sensitive data on external/shared storage; don't log file contents, tokens, or PII (scanners flag `Log`/print of tainted data, and it also leaks into Sentry/analytics). +- **Manifest hygiene** — `android:allowBackup` and `android:debuggable` are flagged for sensitive apps; set deliberately. +- Request the minimum permissions; over-requesting is flagged. + +### 7. Insecure deserialization & XML +- **XXE** *(Sonar S2755)* — when parsing XML (layouts, manifests, project files) disable external entities/DTDs on the factory (`setFeature("http://apache.org/xml/features/disallow-doctype-decl", true)`). +- **Untrusted deserialization** — validate JSON (Gson/kotlinx) shape before use; never deserialize untrusted data into arbitrary types. + +### 8. Vulnerable & risky dependencies — Snyk's specialty +- **Before adding or bumping a dependency**, check it has no known high/critical CVEs and is maintained. Snyk fires on the *transitive* graph too, so a new direct dep can drag in a flagged one. +- Reuse what's already in `gradle/libs.versions.toml` (our standing rule is to avoid new deps anyway). Pin versions in the catalog; don't introduce dynamic/`+` versions. +- When Snyk flags an existing dep, prefer the smallest safe upgrade; if no fix exists, that's a conversation (and a tracked exception), not a silent ignore. +- Watch licenses too — Snyk/legal flags incompatible licenses for a GPLv3 project. + +### 9. Reliability "blockers" (not strictly security) +A chunk of Sonar **blocker** findings are reliability bugs, not vulnerabilities: guaranteed NPEs, resource leaks (unclosed streams/cursors/connections), and dead/always-true conditions. These overlap our leak guidance — see [REVIEW.md](REVIEW.md) §1–2. Use `use {}`, null-safety, and exhaustive `when` to keep them out. + +--- + +## Before you push + +- **Self-review against §1–9** for the surfaces your change touches (input handling, crypto, network, WebView, manifest, new deps). +- If you have the SonarQube MCP / `sonar` CLI handy, scan your branch locally and confirm **no new** blocker/high on changed files. Remember: official analysis runs in CI — local is verification only (`AGENTS.md`). +- If a finding is genuinely a false positive, document the reasoning and get reviewer sign-off **before** suppressing — and never suppress on code we own without that. + +## Reporting a vulnerability + +Found a security issue in Code On The Go? **Do not open a public issue.** Email **feedback@appdevforall.org** with details and reproduction steps, and allow time for a fix before public disclosure. diff --git a/docs/adr/0001-persistence-without-room.md b/docs/adr/0001-persistence-without-room.md new file mode 100644 index 0000000000..ca51979788 --- /dev/null +++ b/docs/adr/0001-persistence-without-room.md @@ -0,0 +1,42 @@ +# 0001. Persistence uses SQLite/filesystem, not Room + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +Code On The Go stores modest amounts of local state — recent projects, tooltips, breakpoints, preferences, and assorted IDE settings — on resource-constrained Android devices, entirely offline. We care about APK size, method count, build time, and full control over storage, and we already run a large multi-module build where every annotation processor adds real cost. + +Android's default persistence library, Room, generates code via `kapt`/KSP, ships a runtime, and manages a schema. It saves boilerplate but adds footprint and build overhead, and in practice it was applied inconsistently across the codebase. + +## Decision + +New persistence uses **raw SQLite** (`SQLiteOpenHelper` / `SQLiteDatabase`) or the **filesystem / preferences**. Room is **not** used for new code. + +The one existing exception is the **Recent Projects** feature (`app/src/main/java/com/itsaky/androidide/roomData/recentproject/`, `@Database version = 4`), which predates this decision and is grandfathered in. It is not to be extended with new entities or tables. (`idetooltips` still declares Room Gradle deps that it does not use; its tooltip store is raw SQLite — those deps should be removed.) + +## Consequences + +**Positive** +- Smaller footprint; no annotation processing for persistence. +- Full control over SQL, migrations, and threading. +- One fewer library to track for CVEs/updates. + +**Negative / costs** +- Hand-written SQL, migrations, and row→object mapping. +- No compile-time query verification (a Room benefit we forgo). + +**Follow-ups** +- Provide shared SQLite helper utilities to keep raw-SQLite boilerplate consistent and safe (parameterized queries — see SECURITY.md). +- Remove the unused Room dependencies from `idetooltips`. + +## Alternatives considered + +- **Room everywhere** — rejected: adds `kapt`/runtime/footprint and build cost; its convenience didn't justify the cost on constrained devices, and it had crept in unevenly. +- **DataStore / SharedPreferences only** — insufficient for the small amount of relational/queryable data (e.g. recent projects). +- **A third-party ORM** — more dependencies and surface for little gain over raw SQLite. + +## Related + +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — persistence policy (authoritative model). diff --git a/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md b/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md new file mode 100644 index 0000000000..29dd30e573 --- /dev/null +++ b/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md @@ -0,0 +1,44 @@ +# 0002. On-device builds run real Gradle out-of-process via the Tooling API + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +The product's core promise is to build and deploy *real* Android apps on the device, offline. That means producing the same results as a desktop Gradle build — correct dependency resolution, AGP behavior, manifest merging, R8/D8 — not an approximation. + +Gradle is memory-hungry and can OOM or crash, especially on a phone. Running it inside the IDE process would couple the build's lifecycle and memory to the UI: a build OOM would take down the editor, and a stuck build couldn't be cleanly killed. + +## Decision + +Run builds with the **Gradle Tooling API in a separate JVM process**, and have the app drive it over IPC. + +- `:subprojects:tooling-api` — the client-facing API and `ToolingApiLauncher`. +- `:subprojects:tooling-api-impl` — the forked process (`tooling.impl.Main`) that hosts the Tooling API connection and runs the build. +- `:subprojects:tooling-api-events` / `:subprojects:tooling-api-model` — the serializable event/model types exchanged across the process boundary. + +The app streams progress/events back from this process and renders them (e.g. `BuildState`, build output). This depends on the vendored Gradle/toolchain from [ADR 0003](0003-vendored-forked-desktop-toolchain.md). + +## Consequences + +**Positive** +- Build crashes and OOMs are isolated from the IDE — the UI survives a failed build. +- Real Gradle semantics, so on-device builds match desktop builds. +- Builds can be cancelled/killed and the process restarted independently. + +**Negative / costs** +- IPC complexity: every model and event crossing the boundary must be serializable and versioned (`tooling-api-events`/`-model`). +- Process startup time and additional memory for a second JVM. +- The Tooling API client/impl must stay in lockstep with the vendored Gradle version. + +## Alternatives considered + +- **Run Gradle in-process** — rejected: an OOM or crash would kill the IDE, and the build lifecycle would be entangled with the UI. +- **A custom/cut-down build engine** — rejected: enormous effort and a permanent correctness gap versus real Gradle/AGP. +- **Cloud/remote builds** — rejected: violates the offline-first requirement. + +## Related + +- [ADR 0003](0003-vendored-forked-desktop-toolchain.md) — the forked toolchain the build process runs. +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — build engine module group. diff --git a/docs/adr/0003-vendored-forked-desktop-toolchain.md b/docs/adr/0003-vendored-forked-desktop-toolchain.md new file mode 100644 index 0000000000..b439255401 --- /dev/null +++ b/docs/adr/0003-vendored-forked-desktop-toolchain.md @@ -0,0 +1,39 @@ +# 0003. Vendor & fork the desktop toolchain via composite-build substitution + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +Compiling Java/Kotlin/Android projects requires desktop developer tools — `javac`, the JDK compiler/`jdeps`, the Eclipse JDT compiler, `layoutlib`, AAPT, JAXP, etc. These assume a desktop JDK environment and are not published in forms that run on Android's ART runtime. To build on-device (see [ADR 0002](0002-on-device-builds-via-gradle-tooling-api.md)), they have to be made to work there. + +## Decision + +**Fork and vendor** these tools into the repository as composite builds under `composite-builds/build-deps` and `composite-builds/build-deps-common`, and **substitute** them for their `com.itsaky.androidide.build:*` Maven coordinates via `dependencySubstitution` in `settings.gradle.kts`. + +Substituted modules include `javac`, `jdk-compiler`, `jdk-jdeps`, `jdt`, `layoutlib-api`, `java-compiler`, `jaxp`, `javapoet`, and `google-java-format`, among others. Any module needing one of these gets the forked, Android-compatible version transparently. + +## Consequences + +**Positive** +- The desktop toolchain runs on-device — the thing that makes the whole product possible. +- We can patch compiler/tooling internals for Android compatibility and bug fixes. +- Consumers reference normal coordinates; the substitution is centralized and invisible to them. + +**Negative / costs** +- A large vendored source subtree and a significant ongoing maintenance burden: tracking upstream changes and JDK/AGP/Gradle updates. +- The **majority of suppressed scanner findings live in these subtrees** — they are treated as vendored code (suppress-only, never "fixed" to our standards) per [SECURITY.md](../../SECURITY.md). +- Onboarding cost: contributors must understand the substitution model before touching builds. + +## Alternatives considered + +- **Depend on upstream artifacts** — rejected: they don't run on Android. +- **Require a companion desktop** — rejected: defeats the product's reason to exist. +- **Bundle a full JDK** — rejected: not feasible/licensable for on-device use in this form. + +## Related + +- [ADR 0002](0002-on-device-builds-via-gradle-tooling-api.md) — the build process that runs this toolchain. +- [SECURITY.md](../../SECURITY.md) — vendored-code suppression policy. +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — dependency-substitution rules. diff --git a/docs/adr/0004-embedded-termux-runtime.md b/docs/adr/0004-embedded-termux-runtime.md new file mode 100644 index 0000000000..59fd72d59e --- /dev/null +++ b/docs/adr/0004-embedded-termux-runtime.md @@ -0,0 +1,42 @@ +# 0004. Embed Termux as the shell & toolchain runtime + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +A self-contained on-device IDE needs a real POSIX shell, a package ecosystem (git, clang, coreutils, and the binaries the build depends on), and an interactive terminal. Building any of that from scratch — a shell, a package manager, a terminal emulator/view — is a multi-year effort and a distraction from the IDE itself. + +Termux is a mature, GPL-licensed Android terminal and package environment that already solves exactly this. + +## Decision + +**Embed Termux** as the shell and toolchain runtime, vendored as modules: + +- `:termux:termux-app` — the Termux application/bootstrap layer. +- `:termux:termux-shared` — shared runtime, file/PTY, and package plumbing. +- `:termux:termux-view` / `:termux:termux-emulator` — the terminal view and terminal emulator used in the IDE's terminal UI. + +The IDE's build and shell flows run through this environment. + +## Consequences + +**Positive** +- A proven shell + package ecosystem and a battle-tested terminal emulator, for free. +- License-compatible with our GPLv3 project. + +**Negative / costs** +- A large native/runtime surface and a meaningful **security consideration**: Termux executes arbitrary commands, so command-execution sinks must be handled carefully (see [SECURITY.md](../../SECURITY.md)). +- Maintenance: tracking Termux upstream and integrating it with our build orchestration. +- Adds to APK size and the set of native components per ABI ([ADR 0005](0005-per-abi-product-flavors.md)). + +## Alternatives considered + +- **A custom shell / NDK-only toolchain** — rejected: enormous scope, and we'd reinvent a package ecosystem. +- **Remote/SSH execution** — rejected: violates offline-first. + +## Related + +- [SECURITY.md](../../SECURITY.md) — command-execution surface. +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — shell module group. diff --git a/docs/adr/0005-per-abi-product-flavors.md b/docs/adr/0005-per-abi-product-flavors.md new file mode 100644 index 0000000000..42dd5004e5 --- /dev/null +++ b/docs/adr/0005-per-abi-product-flavors.md @@ -0,0 +1,38 @@ +# 0005. Ship per-ABI product flavors (v7/v8), not a universal APK + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +The app bundles large native components per ABI — llama.cpp, plus the on-device toolchain/Termux binaries. A universal APK carrying both `armeabi-v7a` and `arm64-v8a` copies of everything would be very large. Code On The Go is distributed primarily as a **direct APK download** from the App Dev for All website, not exclusively through Google Play, so we cannot rely on Play's automatic per-ABI splitting to slim downloads. + +## Decision + +Define two **product flavors** on an `abi` dimension — `v7` (`armeabi-v7a`) and `v8` (`arm64-v8a`) — **centrally** in `composite-builds/build-logic` (`conf/AndroidModuleConf.kt`), applied to every Android module except `:plugin-api`. + +Consequences for the build: +- Build tasks are flavor-qualified: `assembleV8Debug`, `assembleV7Release`, etc. **There is no flavorless `assembleDebug`.** +- Native assets (e.g. the llama AAR) are bundled per flavor via the root `build.gradle.kts`; prebuilt assets live under `assets/release/v7/` and `assets/release/v8/`. + +## Consequences + +**Positive** +- Each APK ships only one ABI's native libraries → much smaller downloads. +- Explicit, centralized control over per-ABI native bundling. + +**Negative / costs** +- No flavorless variant; every build/test/release task is doubled into V7/V8. +- Larger build matrix and more release artifacts to produce and track. +- A recurring onboarding gotcha: contributors must use `assembleV8Debug` (etc.), not `assembleDebug`. + +## Alternatives considered + +- **Universal (fat) APK** — rejected: size, given the large per-ABI native payload. +- **Android App Bundle ABI splits** — rejected: our primary distribution is direct APK download, so we can't depend on Play-side splitting; flavors give us split artifacts on every channel. + +## Related + +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — Build & Module Configuration. +- [CLAUDE.md](../../CLAUDE.md) — build task commands. diff --git a/docs/adr/0006-koin-dependency-injection.md b/docs/adr/0006-koin-dependency-injection.md new file mode 100644 index 0000000000..539f2a5f9f --- /dev/null +++ b/docs/adr/0006-koin-dependency-injection.md @@ -0,0 +1,36 @@ +# 0006. Use Koin for dependency injection, not Hilt/Dagger + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +The app needs dependency injection to wire ViewModels, repositories, and managers across many modules. On a build this large (~80 modules), annotation-processing-based DI (Dagger/Hilt via `kapt`/KSP) adds noticeable compile time and couples DI to the Android Gradle plugin and the build graph. The team values fast iteration and simple, testable wiring. + +## Decision + +Use **Koin** for dependency injection. + +- Modules `coreModule` and `pluginModule` declare dependencies via Koin's DSL (`single { }`, `viewModel { }`); `startKoin { }` runs in `IDEApplication`. +- Prefer **constructor injection** into ViewModels/classes. +- A small `ServiceLocator` (a `KoinComponent`) provides lazy access for components that must resolve a dependency *after* Koin starts but *before* their screen exists. + +## Consequences + +**Positive** +- No annotation processing → faster builds, no `kapt`/KSP for DI. +- Simple, readable DSL; trivial to override dependencies in tests. + +**Negative / costs** +- DI errors surface at **runtime**, not compile time — a missing binding fails when resolved, so DI paths need test coverage. +- `ServiceLocator` is a deliberate service-locator escape hatch; over-use reintroduces hidden global state. Keep it limited and prefer constructor injection. + +## Alternatives considered + +- **Hilt / Dagger** — rejected: compile-time safety is real, but the `kapt`/KSP build cost and AGP coupling are significant at this module scale, and the team prioritized iteration speed. +- **Manual DI / by-hand wiring** — rejected: boilerplate and poor ergonomics across this many modules. + +## Related + +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — technology stack (DI). diff --git a/docs/adr/0007-strictmode-whitelist-engine.md b/docs/adr/0007-strictmode-whitelist-engine.md new file mode 100644 index 0000000000..fd3e4ec3ae --- /dev/null +++ b/docs/adr/0007-strictmode-whitelist-engine.md @@ -0,0 +1,40 @@ +# 0007. Enforce StrictMode via a custom whitelist engine + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +StrictMode is valuable for catching main-thread disk/network I/O and leaked resources early. But Code On The Go is a large app built on inherited and vendored code (the AndroidIDE lineage, the forked toolchain in [ADR 0003](0003-vendored-forked-desktop-toolchain.md), Termux) that trips many StrictMode violations we cannot reasonably fix. + +The two blunt options both fail us: disabling StrictMode hides regressions in **our** code, while enabling it with a death/penalty policy crashes or spams on **third-party** violations we don't control. + +## Decision + +Build a **custom StrictMode whitelist engine** that suppresses specific, known violations while still surfacing everything else. + +Components (in `app/.../app/strictmode/`): `StrictModeManager`, `WhitelistBuilder`, `WhitelistEngine`, `ViolationDispatcher`, `ViolationListener`, `ViolationHandler`, `StrictModeConfig`, with tests (`WhitelistEngineTest`, `WhitelistRulesTest`). + +**Policy:** the whitelist is **only** for vendor/framework violations we can't change. App-owned violations must be **fixed**, not whitelisted. (This is the same suppress-only-what-we-don't-own philosophy applied to security scanners in [SECURITY.md](../../SECURITY.md).) + +## Consequences + +**Positive** +- StrictMode stays useful in a large inherited codebase — our regressions still surface. +- Suppressions are explicit, rule-based, reviewable, and unit-tested. + +**Negative / costs** +- Custom infrastructure to maintain and understand. +- Risk of over-whitelisting if the policy isn't enforced — so an app-owned entry added to the whitelist is a review red flag, not a fix. + +## Alternatives considered + +- **StrictMode off** — rejected: loses regression detection on our own code. +- **StrictMode with `penaltyDeath`/`penaltyLog` globally** — rejected: crashes/spams on vendored violations we can't fix. +- **Per-call `allowThreadDiskReads()` suppressions** — rejected: scatters suppressions through the code, easy to abuse, and impossible to audit centrally. + +## Related + +- [REVIEW.md](../../REVIEW.md) — §3 threading & StrictMode review guidance. +- [SECURITY.md](../../SECURITY.md) — the parallel suppress-only-vendor policy for scanners. diff --git a/docs/adr/0008-retain-androidide-namespace.md b/docs/adr/0008-retain-androidide-namespace.md new file mode 100644 index 0000000000..3aaf408612 --- /dev/null +++ b/docs/adr/0008-retain-androidide-namespace.md @@ -0,0 +1,35 @@ +# 0008. Retain the `com.itsaky.androidide` namespace after rebrand + +- **Status:** Accepted +- **Date:** 2026-06-18 +- **Deciders:** Code On The Go team + +## Context + +Code On The Go is the rebranded successor to **AndroidIDE**. The product name, branding, and assets changed, but the inherited codebase carries the original identity deeply: the application id and Gradle namespace are `com.itsaky.androidide` (`BuildConfig.PACKAGE_NAME`), `rootProject.name` is `AndroidIDE`, and there are many thousands of references plus the generated `R` class, the manifest, package-qualified vendored substitutions, signing identity, and existing installs in the field. + +Changing an Android **application id** breaks the update path for installed users (a different app id is a different app) and disrupts signing/identity continuity. A package rename of this size also ripples through the vendored `com.itsaky.androidide.build:*` substitutions ([ADR 0003](0003-vendored-forked-desktop-toolchain.md)). + +## Decision + +**Keep** the `com.itsaky.androidide` application id / namespace and the internal Gradle project names. Rebranding (`INTERNAL_NAME = "CodeOnTheGo"`, the app label, icons, and user-facing assets) is applied at the **presentation layer only**, not the package identity. + +## Consequences + +**Positive** +- Preserves the update path and signing continuity for existing installs. +- Avoids a massive, high-risk refactor and the churn it would cause across vendored substitutions and the `R` class. + +**Negative / costs** +- The codebase namespace doesn't match the product name, which is confusing to newcomers. This is called out explicitly in [CLAUDE.md](../../CLAUDE.md) and [ARCHITECTURE.md](../../ARCHITECTURE.md) so the mismatch is expected, not surprising. +- Care is needed to ensure user-facing strings/branding are overridden and no "AndroidIDE" naming leaks to users. + +## Alternatives considered + +- **Full rename to `org.appdevforall.*`** — rejected: breaks updates for existing users, requires enormous error-prone churn, and risks destabilizing the vendored substitutions for little functional gain. +- **Partial rename** (some modules) — rejected: produces an inconsistent namespace with the same risks and no clean payoff. + +## Related + +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — overview note on the namespace/product-name mismatch. +- [ADR 0003](0003-vendored-forked-desktop-toolchain.md) — vendored coordinates that a rename would disrupt. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000000..610502e65f --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,24 @@ +# Architecture Decision Records + +This directory holds **Architecture Decision Records (ADRs)** — short documents capturing a significant architectural decision, its context, and its consequences. They explain *why* the codebase is the way it is, so a decision isn't silently undone later. + +Format is lightweight **MADR / Nygard**: Context → Decision → Consequences → Alternatives. Most records here are *retroactive* — they document decisions already embedded in the code. + +## Conventions + +- One decision per file, named `NNNN-kebab-title.md` with a zero-padded sequence number. +- **Status** lifecycle: `Proposed` → `Accepted` → `Superseded by NNNN` / `Deprecated`. Don't edit a decision after it's accepted; write a new ADR that supersedes it. +- Keep it short and concrete. Link related ADRs and the relevant code paths. + +## Index + +| # | Decision | Status | +|---|---|---| +| [0001](0001-persistence-without-room.md) | Persistence uses SQLite/filesystem, not Room | Accepted | +| [0002](0002-on-device-builds-via-gradle-tooling-api.md) | On-device builds run real Gradle out-of-process via the Tooling API | Accepted | +| [0003](0003-vendored-forked-desktop-toolchain.md) | Vendor & fork the desktop toolchain via composite-build substitution | Accepted | +| [0004](0004-embedded-termux-runtime.md) | Embed Termux as the shell & toolchain runtime | Accepted | +| [0005](0005-per-abi-product-flavors.md) | Ship per-ABI product flavors (v7/v8), not a universal APK | Accepted | +| [0006](0006-koin-dependency-injection.md) | Use Koin for dependency injection, not Hilt/Dagger | Accepted | +| [0007](0007-strictmode-whitelist-engine.md) | Enforce StrictMode via a custom whitelist engine | Accepted | +| [0008](0008-retain-androidide-namespace.md) | Retain the `com.itsaky.androidide` namespace after rebrand | Accepted | From ff0383be1ce8381f0a1e540cc745d554f7255067 Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Thu, 18 Jun 2026 23:36:10 -0400 Subject: [PATCH 02/12] ADFA-4357 Document accessibility & contextual-help review rules Promote accessibility from a proposed item to an enforced review section and add a parallel contextual-help (long-press 3-tier) rule, both keyed to existing patterns (ADFA-2667 screen-reader work, the idetooltips module). - REVIEW.md: new sections for content-description coverage and long-press help; matching 60-second-checklist entries; renumber trailing sections. - idetooltips/README.md: state the long-press-for-help-everywhere principle and the three-tier (tooltip / tooltip / web page) help model. --- REVIEW.md | 27 ++++++++++++++++++++++++--- idetooltips/README.md | 18 ++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index 7d3e131994..09388a6a6c 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -18,6 +18,8 @@ How to give a good review on Code On The Go. This is a coaching doc, not a gate - [ ] **No duplication:** the change reuses existing helpers instead of copy-pasting. - [ ] **Docs:** public classes/functions have KDoc/Javadoc explaining *why*, not *what*. - [ ] **Strings** are in `strings.xml`, not inline literals. +- [ ] **Accessibility:** every actionable view has a `contentDescription` (XML *or* programmatic); decorative views are marked `importantForAccessibility="no"`. +- [ ] **Contextual help:** new interactive elements (and any new screen/panel) have long-press help wired to the 3-tier tooltip system. - [ ] **Analytics:** meaningful user/build actions emit an event (see below). - [ ] **Scope/size:** PR is focused and within the ~500 LOC / 10-file guideline (`AGENTS.md`). @@ -113,7 +115,27 @@ Keep event names/params stable and low-cardinality; **no PII, file paths with us - **Strings in `strings.xml`.** User-facing text must be a string resource, never an inline literal — lint flags `HardcodedText`, and externalized strings feed our Crowdin translation flow. Use plurals/`getQuantityString` and positional args for formatting. Log messages and analytics keys are *not* user-facing and stay in code. - **Dependencies:** don't add one without checking `gradle/libs.versions.toml` first — we probably already have it (`AGENTS.md`). -## 8. Architecture alignment +## 8. Accessibility — every actionable view speaks + +CoGo serves visually-impaired developers, so screen-reader support (TalkBack) is a correctness requirement for UI work, not a nice-to-have. The pattern below is what ADFA-2667 established — hold new UI to it. + +- **Label every actionable view.** Buttons, `ImageButton`s, and icon-only controls need a `contentDescription`. In layouts: `android:contentDescription="@string/cd_…"`. An unlabeled icon button is announced as "unlabeled button" — useless. +- **Don't forget views built in code.** The easy miss is anything *not* in XML — toolbar action items, RecyclerView rows, dynamically-inflated buttons. Set `contentDescription` programmatically when you create them (see `EditorHandlerActivity.getToolbarContentDescription()`, `MainActionsListAdapter`, `DiagnosticItemAdapter` from ADFA-2667). +- **Silence decoration.** Purely visual views — separators, background images, an icon sitting next to a label that already says the same thing — get `android:importantForAccessibility="no"` (or `View.IMPORTANT_FOR_ACCESSIBILITY_NO`) so TalkBack skips them instead of reading noise. +- **Describe the action, not the picture.** Prefer what tapping *does* over what the icon *looks like*: `cd_sync_project`, not "circular arrows icon". When a control toggles state, make the description state-aware — `cd_drawer_open` vs. `cd_drawer_close`, expand vs. collapse — rather than one ambiguous label. +- **Externalize, with the `cd_` convention.** Content-description strings live in `strings.xml` named `cd_*` (so they're greppable, translatable via Crowdin, and not inline literals). Reuse an existing `cd_` string before adding a near-duplicate. +- **Bonus: it stabilizes tests.** Our UI tests drive views through accessibility actions (`ACTION_CLICK`), which need these labels — so good a11y and reliable instrumentation tests are the same work. + +## 9. Contextual help — long-press works everywhere + +Help in CoGo is reached by **long-press**, anywhere, and opens a progressive three-tier experience: **Tier 1 & 2 are tooltips** (anchored popups from `idetooltips`, via `showIDETooltip`'s `level` argument), and **Tier 3 is a full help web page** reached from the tooltip's "See More" link. This is a product promise — a long-press should never be met with silence. See [`idetooltips/README.md`](idetooltips/README.md) for the system itself. + +- **Wire up help on new interactive elements.** Any view that does something when tapped — buttons, icon controls, menu items, list rows, toolbar actions — gets long-press help. A new actionable view with no tooltip affordance is an incomplete change, the same as a missing `contentDescription`. +- **Cover new screens and panels too.** Even where individual pixels aren't interactive, a new screen/panel/dialog needs at least a top-level help entry so help is reachable from anywhere on that surface. +- **The affordance is the requirement, not finished copy.** Tooltip *content* may still be getting authored — that's fine — but the long-press must already be wired and route into the tier system. Don't ship UI that can never surface help. +- **Reuse the system; don't reinvent it.** Use `showIDETooltip` / the `idetooltips` module rather than a one-off popup, so all three tiers and the tooltip store stay consistent. + +## 10. Architecture alignment Hold the change to the patterns in [ARCHITECTURE.md](ARCHITECTURE.md): - New screens follow **UDF**: `ViewModel` + `StateFlow`, sealed `UiEvent`/`UiEffect`, repository for data. Don't put I/O or business logic in Fragments/Activities. @@ -121,7 +143,7 @@ Hold the change to the patterns in [ARCHITECTURE.md](ARCHITECTURE.md): - **Persistence:** raw SQLite or filesystem — **not Room** (Recent Projects is the lone legacy exception). - **UI safety:** never place our UI over the two Android system bars — the top status bar and the bottom navigation bar (`AGENTS.md`). -## 9. PR hygiene +## 11. PR hygiene - Focused and reviewable: aim for the **~500 LOC / 10-file** ceiling; split larger work into stacked PRs. - Title/branch follow `ADFA-####`; description says *what changed, why, how it was verified*, and flags anything intentionally out of scope (e.g. "UI-only, no unit test"). @@ -133,7 +155,6 @@ Hold the change to the patterns in [ARCHITECTURE.md](ARCHITECTURE.md): These aren't established rules yet — flagging them as candidates for the team: -- **Accessibility:** require `contentDescription` on actionable views and adequate touch targets. (We already lean on accessibility actions in UI tests — `ACTION_CLICK` — so this is low-cost and improves both a11y and test stability.) - **Backward compatibility:** `MIN_SDK=28` — review new APIs for guard/desugaring; remember user-built apps target `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16`. - **Performance budget for startup & editor:** watch added work in `Application.onCreate`, `tooling-api` startup, and per-keystroke editor paths; flag synchronous heavy work there. - **Offline-first:** CoGo is meant to work without a network. New features should degrade gracefully offline; analytics/Sentry/Gemini calls must be non-blocking and failure-tolerant. diff --git a/idetooltips/README.md b/idetooltips/README.md index 5f9ac508ce..76eb643461 100644 --- a/idetooltips/README.md +++ b/idetooltips/README.md @@ -9,6 +9,24 @@ This module provides functionalities such as: * Fetching tooltip data from a local database. * Handling user interactions within tooltips (e.g., "See More" links). +## Design principle: long-press-for-help is everywhere + +Help in Code On The Go is reached the same way no matter where you are: **long-press**. This is a product promise, not a per-screen decision — a user who long-presses to learn what something does should never be met with silence. + +Concretely, that means: + +* **Every interactive element provides help** — buttons, icon-only controls, menu items, list rows, editor-toolbar actions. If a view does something when tapped, long-pressing it must surface help. +* **Every major screen or panel is covered too.** Even where an individual pixel isn't itself interactive, its containing surface (screen, panel, dialog) must offer at least a top-level help entry, so help is always reachable from anywhere in that surface. + +### The three-tier help system + +A single long-press opens a progressive, three-tier help experience: + +* **Tier 1 & Tier 2 — tooltips.** Rendered by this module as anchored popups (the `level` argument to `showIDETooltip` selects the depth — a short summary first, then more detailed help). These stay in-context and don't leave the screen. +* **Tier 3 — a full help web page.** Reached from a tooltip's "See More" / help link, this opens proper standalone documentation for users who need the complete explanation. + +> Tooltip *content* may be incomplete while help is still being authored, but the *affordance* must exist: new UI ships with long-press help wired up. See `REVIEW.md` (Contextual help) for the review-time rule. + ## Features * **Dynamic Tooltip Display:** Show tooltips anchored to specific UI elements or coordinates. From ca27959c95f9db1d37a8cdcd7d93e222f1896f50 Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 01:52:07 -0400 Subject: [PATCH 03/12] ADFA-4357 Record Compose-for-new-UI decision; offline-first rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ADR 0009: new IDE UI is Jetpack Compose, no new XML View screens; the UDF/Koin/StateFlow stack is unchanged. Indexed in docs/adr/README.md. - ARCHITECTURE.md: tech-stack UI row + overview now point to ADR 0009 instead of claiming the IDE is 'Not Compose'. - REVIEW.md: new Compose-only rule in Architecture alignment; accessibility (§8) now gives View + Compose forms for each rule (semantics, clearAndSetSemantics, the HardcodedText lint gap); contextual help (§9) notes idetooltips has no Compose entry point yet (displayTooltipOnLongPress is View-based); promote Offline-first from proposed to an accepted section. --- ARCHITECTURE.md | 4 +- REVIEW.md | 39 +++++++++++++------ docs/adr/0009-jetpack-compose-for-new-ui.md | 43 +++++++++++++++++++++ docs/adr/README.md | 1 + 4 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 docs/adr/0009-jetpack-compose-for-new-ui.md diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3021eb313c..e9d30445e7 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -6,7 +6,7 @@ Code On The Go (CoGo) is a full Android IDE that runs **on the device** — it edits, builds, and deploys real Android apps offline, embedding a Termux toolchain and running an actual Gradle build in a separate process via the `tooling-api`. It is the maintained successor to AndroidIDE, so the codebase namespace is still `com.itsaky.androidide`. -There is **no single architectural philosophy** across the whole app. It is a large, layered, View-based application where newer feature surfaces (plugin manager, AI agent, git, project list) follow a deliberate **Unidirectional Data Flow (UDF)** with Koin DI, `ViewModel` + `StateFlow`, sealed UI-state/effect types, and repositories — while older surfaces still use `LiveData` and talk to GreenRobot EventBus directly. New work should follow the UDF pattern documented below. +There is **no single architectural philosophy** across the whole app. It is a large, layered application that is still **predominantly View-based**, where newer feature surfaces (plugin manager, AI agent, git, project list) follow a deliberate **Unidirectional Data Flow (UDF)** with Koin DI, `ViewModel` + `StateFlow`, sealed UI-state/effect types, and repositories — while older surfaces still use `LiveData` and talk to GreenRobot EventBus directly. New work follows the UDF pattern documented below, and new UI is built in **Jetpack Compose** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)) — Compose replaces the view layer only; the UDF stack (ViewModel + `StateFlow`, Koin, repositories) is unchanged. Existing XML/View screens remain until they're substantially reworked. ## Core Architecture & Data Flow @@ -88,7 +88,7 @@ These structural facts shape every module. The day-to-day build *commands* live | Concern | Library / Approach | |---|---| -| UI | Android Views + Fragments + `RecyclerView` (Material Components). **Not Compose** for the IDE's own UI. | +| UI | **Jetpack Compose for all new UI** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)). The existing majority is still Android Views + Fragments + `RecyclerView` (Material Components); those legacy screens stay until reworked, but new IDE UI is Compose-only. | | Dependency Injection | **Koin** (`org.koin`) — `coreModule`/`pluginModule`, `startKoin` in `IDEApplication`, plus a `ServiceLocator : KoinComponent` for lazy post-startup access. No Hilt/Dagger. | | Asynchronous work | **Kotlin Coroutines + Flow** (`StateFlow`/`SharedFlow`, `viewModelScope`, app-scoped `CoroutineScope(SupervisorJob() + Dispatchers.IO)`); **GreenRobot EventBus** for cross-subsystem events. | | Networking | Offline-first; no general REST layer. External I/O is **Google GenAI SDK** (Gemini), **on-device llama.cpp**, and **JGit** (git). Retrofit is in the catalog but effectively unused in app code. | diff --git a/REVIEW.md b/REVIEW.md index 09388a6a6c..b47a2a38d1 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -117,14 +117,20 @@ Keep event names/params stable and low-cardinality; **no PII, file paths with us ## 8. Accessibility — every actionable view speaks -CoGo serves visually-impaired developers, so screen-reader support (TalkBack) is a correctness requirement for UI work, not a nice-to-have. The pattern below is what ADFA-2667 established — hold new UI to it. - -- **Label every actionable view.** Buttons, `ImageButton`s, and icon-only controls need a `contentDescription`. In layouts: `android:contentDescription="@string/cd_…"`. An unlabeled icon button is announced as "unlabeled button" — useless. -- **Don't forget views built in code.** The easy miss is anything *not* in XML — toolbar action items, RecyclerView rows, dynamically-inflated buttons. Set `contentDescription` programmatically when you create them (see `EditorHandlerActivity.getToolbarContentDescription()`, `MainActionsListAdapter`, `DiagnosticItemAdapter` from ADFA-2667). -- **Silence decoration.** Purely visual views — separators, background images, an icon sitting next to a label that already says the same thing — get `android:importantForAccessibility="no"` (or `View.IMPORTANT_FOR_ACCESSIBILITY_NO`) so TalkBack skips them instead of reading noise. -- **Describe the action, not the picture.** Prefer what tapping *does* over what the icon *looks like*: `cd_sync_project`, not "circular arrows icon". When a control toggles state, make the description state-aware — `cd_drawer_open` vs. `cd_drawer_close`, expand vs. collapse — rather than one ambiguous label. -- **Externalize, with the `cd_` convention.** Content-description strings live in `strings.xml` named `cd_*` (so they're greppable, translatable via Crowdin, and not inline literals). Reuse an existing `cd_` string before adding a near-duplicate. -- **Bonus: it stabilizes tests.** Our UI tests drive views through accessibility actions (`ACTION_CLICK`), which need these labels — so good a11y and reliable instrumentation tests are the same work. +CoGo serves visually-impaired developers, so screen-reader support (TalkBack) is a correctness requirement for UI work, not a nice-to-have. The pattern below is what ADFA-2667 established — hold new UI to it. New UI is Compose ([§10](#10-architecture-alignment) / [ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)), so each rule below gives both the View and the Compose form; the requirement is identical in either toolkit. + +- **Label every actionable view.** Buttons, `ImageButton`s, and icon-only controls need a `contentDescription`. + - *View:* `android:contentDescription="@string/cd_…"`. An unlabeled icon button is announced as "unlabeled button" — useless. + - *Compose:* pass the `contentDescription` argument on `Icon`/`Image`; for a custom clickable or an `IconButton` (whose inner `Icon` is usually `contentDescription = null`), put the label on the control with `Modifier.semantics { contentDescription = … }`. +- **Don't forget the elements built in code.** + - *View:* the easy miss is anything *not* in XML — toolbar action items, RecyclerView rows, dynamically-inflated buttons. Set `contentDescription` programmatically when you create them (see `EditorHandlerActivity.getToolbarContentDescription()`, `MainActionsListAdapter`, `DiagnosticItemAdapter` from ADFA-2667). + - *Compose:* `Icon`/`Image` *force* you to pass `contentDescription`, so the miss is supplying a vague one — or `null` on something that's actually actionable. Don't reach for `null` just to satisfy the signature. +- **Silence decoration.** Purely visual elements — separators, background images, an icon sitting next to a label that already says the same thing — should be skipped by TalkBack, not read as noise. + - *View:* `android:importantForAccessibility="no"` (or `View.IMPORTANT_FOR_ACCESSIBILITY_NO`). + - *Compose:* pass `contentDescription = null` on the decorative `Icon`/`Image`; to drop a whole subtree from the tree, use `Modifier.clearAndSetSemantics { }`. +- **Describe the action, not the picture.** Prefer what tapping *does* over what the icon *looks like*: `cd_sync_project`, not "circular arrows icon". When a control toggles state, make the description state-aware — `cd_drawer_open` vs. `cd_drawer_close`, expand vs. collapse — rather than one ambiguous label. (Same in both toolkits; in Compose pull the text with `stringResource(R.string.cd_…)`.) +- **Externalize, with the `cd_` convention.** Content-description strings live in `strings.xml` named `cd_*` (so they're greppable, translatable via Crowdin, and not inline literals). Reuse an existing `cd_` string before adding a near-duplicate. Note the lint `HardcodedText` check does **not** see Compose literals — a hardcoded `contentDescription = "Sync"` won't be flagged, so reviewers must catch it. +- **Bonus: it stabilizes tests.** Screen-reader semantics are also what UI tests match on — `ACTION_CLICK` for Views, `onNodeWithContentDescription(…)` for Compose — so good a11y and reliable instrumentation tests are the same work. ## 9. Contextual help — long-press works everywhere @@ -133,17 +139,27 @@ Help in CoGo is reached by **long-press**, anywhere, and opens a progressive thr - **Wire up help on new interactive elements.** Any view that does something when tapped — buttons, icon controls, menu items, list rows, toolbar actions — gets long-press help. A new actionable view with no tooltip affordance is an incomplete change, the same as a missing `contentDescription`. - **Cover new screens and panels too.** Even where individual pixels aren't interactive, a new screen/panel/dialog needs at least a top-level help entry so help is reachable from anywhere on that surface. - **The affordance is the requirement, not finished copy.** Tooltip *content* may still be getting authored — that's fine — but the long-press must already be wired and route into the tier system. Don't ship UI that can never surface help. -- **Reuse the system; don't reinvent it.** Use `showIDETooltip` / the `idetooltips` module rather than a one-off popup, so all three tiers and the tooltip store stay consistent. +- **Reuse the system; don't reinvent it.** Wire help through the `idetooltips` module — today that's the `View.displayTooltipOnLongPress(context, anchorView, category, tag)` extension (`setOnLongClickListener` → `TooltipManager.showTooltip`) — rather than a one-off popup, so all three tiers and the tooltip store stay consistent. +- **Compose has no native entry point yet.** The helper above is View-based (it needs an `anchorView`). Until `idetooltips` grows a Compose API, a new composable surface wires help via `AndroidView` interop or a thin wrapper that exposes the anchor — flag it in review rather than skipping help, and prefer building the reusable `Modifier`/wrapper once over copy-pasting interop at each call site. ## 10. Architecture alignment Hold the change to the patterns in [ARCHITECTURE.md](ARCHITECTURE.md): -- New screens follow **UDF**: `ViewModel` + `StateFlow`, sealed `UiEvent`/`UiEffect`, repository for data. Don't put I/O or business logic in Fragments/Activities. +- **New UI is Jetpack Compose** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)) — a new XML layout / `Fragment`-rendered screen for the IDE's own UI should be sent back. Compose changes only the view layer; the rest of this list still applies. (Existing XML screens are fine until reworked.) +- New screens follow **UDF**: `ViewModel` + `StateFlow`, sealed `UiEvent`/`UiEffect`, repository for data. Composables collect state via `collectAsState()`; keep I/O and business logic out of composables/Activities. - DI via **Koin** (constructor injection); register new singletons/viewModels in the module. - **Persistence:** raw SQLite or filesystem — **not Room** (Recent Projects is the lone legacy exception). - **UI safety:** never place our UI over the two Android system bars — the top status bar and the bottom navigation bar (`AGENTS.md`). -## 11. PR hygiene +## 11. Offline-first + +CoGo is meant to work **without a network** — editing, building, and running an app on-device must not depend on connectivity. Hold new work to that: + +- **Degrade gracefully offline.** A feature that needs the network must still launch, explain itself, and leave the rest of the app usable when there's no connection — never block a core flow (edit/build/run) on a request. +- **Network calls are non-blocking and failure-tolerant.** Analytics, Sentry, and Gemini calls run off the main thread and must tolerate timeouts/failures silently (no crash, no hang, no lost user action). A dropped analytics event is acceptable; a dropped keystroke is not. +- **No network on the critical path.** Don't introduce a connectivity dependency into startup, the editor, or the build pipeline. + +## 12. PR hygiene - Focused and reviewable: aim for the **~500 LOC / 10-file** ceiling; split larger work into stacked PRs. - Title/branch follow `ADFA-####`; description says *what changed, why, how it was verified*, and flags anything intentionally out of scope (e.g. "UI-only, no unit test"). @@ -157,7 +173,6 @@ These aren't established rules yet — flagging them as candidates for the team: - **Backward compatibility:** `MIN_SDK=28` — review new APIs for guard/desugaring; remember user-built apps target `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16`. - **Performance budget for startup & editor:** watch added work in `Application.onCreate`, `tooling-api` startup, and per-keystroke editor paths; flag synchronous heavy work there. -- **Offline-first:** CoGo is meant to work without a network. New features should degrade gracefully offline; analytics/Sentry/Gemini calls must be non-blocking and failure-tolerant. - **Feature flags / kill switches** for risky surfaces (AI agent, plugins, web server) so we can disable in the field without a release. Add to or push back on any of these — this doc is meant to evolve with the team. diff --git a/docs/adr/0009-jetpack-compose-for-new-ui.md b/docs/adr/0009-jetpack-compose-for-new-ui.md new file mode 100644 index 0000000000..8e4b15d79a --- /dev/null +++ b/docs/adr/0009-jetpack-compose-for-new-ui.md @@ -0,0 +1,43 @@ +# 0009. Build new UI in Jetpack Compose, not XML Views + +- **Status:** Accepted +- **Date:** 2026-06-19 +- **Deciders:** Code On The Go team + +## Context + +Historically the IDE's own UI is **View-based** — XML layouts, `Fragment`s, and `RecyclerView` with Material Components (see [ADR 0006](0006-koin-dependency-injection.md) context and ARCHITECTURE.md). Newer surfaces already moved to a Unidirectional Data Flow (UDF) architecture — `ViewModel` + `StateFlow`, sealed UI-state/effect types, repositories, Koin DI — but still rendered through XML and `findViewById`/binding. + +That split has a cost: two ways to build a screen, manual view-state wiring, boilerplate binding code, and UI logic that's awkward to unit-test. Jetpack Compose collapses the view layer into Kotlin, binds naturally to `StateFlow` via `collectAsState()`, and fits the UDF pattern the team already follows. Unlike most ADRs here, this one is **forward-looking** — it sets direction for new work rather than documenting an existing decision. + +## Decision + +**All new UI for the IDE is built in Jetpack Compose. New XML View-based screens are not permitted.** + +- New screens, panels, dialogs, and reusable UI components are composables. Reviewers should reject a new XML layout / `Fragment`-rendered screen for the IDE's own UI. +- **The rest of the stack is unchanged.** Compose replaces only the view layer. UDF still holds: `ViewModel` + `StateFlow`, sealed `UiEvent`/`UiEffect`, and repositories for data. Composables collect state with `collectAsState()` and send events up to the ViewModel. +- **DI is still Koin** ([ADR 0006](0006-koin-dependency-injection.md)) — ViewModels are resolved through Koin, not a new mechanism. +- **Existing XML/View screens stay as-is.** This is not a migration mandate; legacy surfaces are rewritten in Compose only when they're being substantially reworked anyway. +- This does **not** apply to the *user's* app or the visual design tooling (`layouteditor`, `uidesigner`, `xml-inflater`), which manipulate XML by definition. + +## Consequences + +**Positive** +- One way to build new screens; less boilerplate (no binding/`findViewById`), state-driven rendering that maps cleanly onto the existing `StateFlow` UDF. +- UI logic is easier to test and preview (`compose-preview` already exists in the tree). + +**Negative / costs** +- A **mixed codebase** for the foreseeable future — Compose and Views coexist; contributors must know both, and interop (`ComposeView` / `AndroidView`) is needed at the seams. +- New Compose dependencies and compiler plugin; some learning curve and added build surface. +- **Cross-cutting UI rules written for XML need Compose equivalents** — notably accessibility (`contentDescription` → `Modifier.semantics`/`contentDescription`, decorative views → `null` semantics) and the long-press 3-tier help affordance. See REVIEW.md §8–§9. + +## Alternatives considered + +- **Stay View-based** — rejected: perpetuates two-way-to-build-a-screen and the binding boilerplate; doesn't leverage the `StateFlow` UDF the team already standardized on. +- **Compose, and mandate migrating existing screens** — rejected: a forced migration of all legacy UI is high-risk churn with little user-facing value. Migrate opportunistically instead. + +## Related + +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — technology stack (UI), state management & UDF. +- [ADR 0006](0006-koin-dependency-injection.md) — Koin DI (unchanged by this decision). +- [REVIEW.md](../../REVIEW.md) — §8 Accessibility, §9 Contextual help (rules that need Compose equivalents). diff --git a/docs/adr/README.md b/docs/adr/README.md index 610502e65f..35ba4b44f3 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -22,3 +22,4 @@ Format is lightweight **MADR / Nygard**: Context → Decision → Consequences | [0006](0006-koin-dependency-injection.md) | Use Koin for dependency injection, not Hilt/Dagger | Accepted | | [0007](0007-strictmode-whitelist-engine.md) | Enforce StrictMode via a custom whitelist engine | Accepted | | [0008](0008-retain-androidide-namespace.md) | Retain the `com.itsaky.androidide` namespace after rebrand | Accepted | +| [0009](0009-jetpack-compose-for-new-ui.md) | Build new UI in Jetpack Compose, not XML Views | Accepted | From 0af326e60af337802fd8aaf421cb67230eca062e Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 01:56:07 -0400 Subject: [PATCH 04/12] ADFA-4357 Note ADFA-4381 follow-up for the Compose long-press help bridge The Compose-only mandate (ADR 0009) and the long-press-everywhere rule (REVIEW.md section 9) need a Compose entry point into the View-based idetooltips system, which does not exist yet. Reference the follow-up ticket from both docs so the gap is tracked, not forgotten. Docs only. --- REVIEW.md | 2 +- docs/adr/0009-jetpack-compose-for-new-ui.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index b47a2a38d1..8ab47cb4d8 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -140,7 +140,7 @@ Help in CoGo is reached by **long-press**, anywhere, and opens a progressive thr - **Cover new screens and panels too.** Even where individual pixels aren't interactive, a new screen/panel/dialog needs at least a top-level help entry so help is reachable from anywhere on that surface. - **The affordance is the requirement, not finished copy.** Tooltip *content* may still be getting authored — that's fine — but the long-press must already be wired and route into the tier system. Don't ship UI that can never surface help. - **Reuse the system; don't reinvent it.** Wire help through the `idetooltips` module — today that's the `View.displayTooltipOnLongPress(context, anchorView, category, tag)` extension (`setOnLongClickListener` → `TooltipManager.showTooltip`) — rather than a one-off popup, so all three tiers and the tooltip store stay consistent. -- **Compose has no native entry point yet.** The helper above is View-based (it needs an `anchorView`). Until `idetooltips` grows a Compose API, a new composable surface wires help via `AndroidView` interop or a thin wrapper that exposes the anchor — flag it in review rather than skipping help, and prefer building the reusable `Modifier`/wrapper once over copy-pasting interop at each call site. +- **Compose has no native entry point yet** (tracked by **ADFA-4381**). The helper above is View-based (it needs an `anchorView`). Until `idetooltips` grows a Compose API, a new composable surface wires help via `AndroidView` interop or a thin wrapper that exposes the anchor — flag it in review rather than skipping help, and prefer building the reusable `Modifier`/wrapper once over copy-pasting interop at each call site. ## 10. Architecture alignment diff --git a/docs/adr/0009-jetpack-compose-for-new-ui.md b/docs/adr/0009-jetpack-compose-for-new-ui.md index 8e4b15d79a..9ae3e76ad0 100644 --- a/docs/adr/0009-jetpack-compose-for-new-ui.md +++ b/docs/adr/0009-jetpack-compose-for-new-ui.md @@ -29,7 +29,7 @@ That split has a cost: two ways to build a screen, manual view-state wiring, boi **Negative / costs** - A **mixed codebase** for the foreseeable future — Compose and Views coexist; contributors must know both, and interop (`ComposeView` / `AndroidView`) is needed at the seams. - New Compose dependencies and compiler plugin; some learning curve and added build surface. -- **Cross-cutting UI rules written for XML need Compose equivalents** — notably accessibility (`contentDescription` → `Modifier.semantics`/`contentDescription`, decorative views → `null` semantics) and the long-press 3-tier help affordance. See REVIEW.md §8–§9. +- **Cross-cutting UI rules written for XML need Compose equivalents** — notably accessibility (`contentDescription` → `Modifier.semantics`/`contentDescription`, decorative views → `null` semantics) and the long-press 3-tier help affordance. See REVIEW.md §8–§9. The help affordance has no Compose entry point yet — building the bridge is tracked as a prerequisite in **ADFA-4381**; sequence it before the first Compose feature screen. ## Alternatives considered From af9a3c29cefbaa27f0278bd72f12e6257deb7655 Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 01:59:14 -0400 Subject: [PATCH 05/12] ADFA-4357 Flag idetooltips/README.md as stale; track refresh in ADFA-4382 The README's usage examples document a showIDETooltip() API that no longer exists (real API: TooltipManager.showTooltip / displayTooltipOnLongPress) and claim a Room store the module doesn't use (it's raw SQLite). Add a banner so contributors trust the code until the refresh lands. Docs only. --- idetooltips/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/idetooltips/README.md b/idetooltips/README.md index 76eb643461..eaab863c65 100644 --- a/idetooltips/README.md +++ b/idetooltips/README.md @@ -1,5 +1,8 @@ # IDE Tooltips Module (`idetooltips`) +> **⚠️ This README is partly out of date — refresh tracked by [ADFA-4382](https://appdevforall.atlassian.net/browse/ADFA-4382).** +> The usage examples below call a `showIDETooltip(...)` function that no longer exists — the real API is `TooltipManager.showTooltip(...)` and the `View.displayTooltipOnLongPress(...)` extension (keyed by `category` + `tag`). The "Database Integration" note citing a Room `IDETooltipDatabase` is also wrong: the store is raw SQLite. Trust the code (`ToolTipManager.kt`, `ViewUtils.kt`) over the examples here until this is reconciled. The design-principle section is current. + ## Overview The `idetooltips` module is responsible for providing a flexible and reusable system for displaying contextual tooltips in Code On the Go. These tooltips can display short summaries, detailed help content, and external links, all within a floating popup anchored to UI components. From c8d2ae34cfff8195aee62431573847e6972b09df Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:01:50 -0400 Subject: [PATCH 06/12] ADFA-4357 Revert all idetooltips/README.md changes on this branch Leave idetooltips/README.md untouched on this PR. Removes both the design-principle section and the staleness banner added earlier; the README refresh is handled wholesale in ADFA-4382 instead. --- idetooltips/README.md | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/idetooltips/README.md b/idetooltips/README.md index eaab863c65..5f9ac508ce 100644 --- a/idetooltips/README.md +++ b/idetooltips/README.md @@ -1,8 +1,5 @@ # IDE Tooltips Module (`idetooltips`) -> **⚠️ This README is partly out of date — refresh tracked by [ADFA-4382](https://appdevforall.atlassian.net/browse/ADFA-4382).** -> The usage examples below call a `showIDETooltip(...)` function that no longer exists — the real API is `TooltipManager.showTooltip(...)` and the `View.displayTooltipOnLongPress(...)` extension (keyed by `category` + `tag`). The "Database Integration" note citing a Room `IDETooltipDatabase` is also wrong: the store is raw SQLite. Trust the code (`ToolTipManager.kt`, `ViewUtils.kt`) over the examples here until this is reconciled. The design-principle section is current. - ## Overview The `idetooltips` module is responsible for providing a flexible and reusable system for displaying contextual tooltips in Code On the Go. These tooltips can display short summaries, detailed help content, and external links, all within a floating popup anchored to UI components. @@ -12,24 +9,6 @@ This module provides functionalities such as: * Fetching tooltip data from a local database. * Handling user interactions within tooltips (e.g., "See More" links). -## Design principle: long-press-for-help is everywhere - -Help in Code On The Go is reached the same way no matter where you are: **long-press**. This is a product promise, not a per-screen decision — a user who long-presses to learn what something does should never be met with silence. - -Concretely, that means: - -* **Every interactive element provides help** — buttons, icon-only controls, menu items, list rows, editor-toolbar actions. If a view does something when tapped, long-pressing it must surface help. -* **Every major screen or panel is covered too.** Even where an individual pixel isn't itself interactive, its containing surface (screen, panel, dialog) must offer at least a top-level help entry, so help is always reachable from anywhere in that surface. - -### The three-tier help system - -A single long-press opens a progressive, three-tier help experience: - -* **Tier 1 & Tier 2 — tooltips.** Rendered by this module as anchored popups (the `level` argument to `showIDETooltip` selects the depth — a short summary first, then more detailed help). These stay in-context and don't leave the screen. -* **Tier 3 — a full help web page.** Reached from a tooltip's "See More" / help link, this opens proper standalone documentation for users who need the complete explanation. - -> Tooltip *content* may be incomplete while help is still being authored, but the *affordance* must exist: new UI ships with long-press help wired up. See `REVIEW.md` (Contextual help) for the review-time rule. - ## Features * **Dynamic Tooltip Display:** Show tooltips anchored to specific UI elements or coordinates. From b5908b3cf6b04c8b9369fa1b515c1c7310a9590e Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:07:17 -0400 Subject: [PATCH 07/12] ADFA-4357 Add doc-sync rule: update module docs in the same change - REVIEW.md: new Code-quality rule + 60-second-checklist entry requiring a change to update any module README/ARCHITECTURE.md/ADR it affects, or leave a tracked note. - AGENTS.md: one-line operational pointer to the REVIEW.md rule, so agents that read AGENTS.md (but not REVIEW.md) still apply it. --- AGENTS.md | 2 ++ REVIEW.md | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 296bc1cc5c..50fbb0d3b5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,8 @@ Avoid adding dependencies - we probably already have everything loaded that you Always plan before building. As part of the plan, estimate the projected size of the change, both number of files and lines of code. When that estimate is greater than 500 lines of code or more than 10 files, you must organize the work into 2 or more smaller change sets, so that the user can help you make 2 or more PRs (Pull Requests). +When you change code, update the docs that describe it in the same change — a module's `README.md`, `ARCHITECTURE.md`, or an ADR — so a doc never outlives the API it documents. See REVIEW.md (Code quality) for the rule. If the doc fix is genuinely out of scope, file a ticket rather than leaving it to drift. + Always protect the two critical Android interaction locations: (1) the top bar containing the time, notification icons, and status icons, and (2) the bottom bar containing the home button, back button, and app selector. Never put UI elements of our app on top of those Android reserved areas. ## Official/Public Actions Run in CI, Not Locally diff --git a/REVIEW.md b/REVIEW.md index 8ab47cb4d8..f03c3a16e1 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -16,7 +16,7 @@ How to give a good review on Code On The Go. This is a coaching doc, not a gate - [ ] **Security:** untrusted input (zip entries, URLs, file paths, web-server requests) is validated; no secrets in code, logs, or analytics. - [ ] **Tests:** non-UI logic has unit coverage. Where coverage is thin, there's logging to diagnose it in the field. - [ ] **No duplication:** the change reuses existing helpers instead of copy-pasting. -- [ ] **Docs:** public classes/functions have KDoc/Javadoc explaining *why*, not *what*. +- [ ] **Docs:** public classes/functions have KDoc/Javadoc explaining *why*, not *what*; any module `README`/`ARCHITECTURE.md`/ADR the change affects is updated in the same PR. - [ ] **Strings** are in `strings.xml`, not inline literals. - [ ] **Accessibility:** every actionable view has a `contentDescription` (XML *or* programmatic); decorative views are marked `importantForAccessibility="no"`. - [ ] **Contextual help:** new interactive elements (and any new screen/panel) have long-press help wired to the 3-tier tooltip system. @@ -112,6 +112,7 @@ Keep event names/params stable and low-cardinality; **no PII, file paths with us - **No duplication.** If you copy-pasted a block, extract a function/extension into the right `common`/`utils` module. Before adding a helper, grep — we likely already have it. Repeated literals/magic numbers → named constants. - **Docstrings.** Public classes, functions, and non-obvious logic get KDoc/Javadoc. Document the *contract and the why* (threading expectations, nullability, side effects, units), not a restatement of the signature. +- **Keep docs in sync with the code.** If a change alters a module's public API, commands, or behavior that's described in its `README.md` (or `ARCHITECTURE.md` / an ADR), update that doc in the *same* PR. A doc that documents a non-existent API is worse than no doc. If the doc-fix is genuinely out of scope, leave a tracked note (a ticket) rather than silently letting it drift. - **Strings in `strings.xml`.** User-facing text must be a string resource, never an inline literal — lint flags `HardcodedText`, and externalized strings feed our Crowdin translation flow. Use plurals/`getQuantityString` and positional args for formatting. Log messages and analytics keys are *not* user-facing and stay in code. - **Dependencies:** don't add one without checking `gradle/libs.versions.toml` first — we probably already have it (`AGENTS.md`). From 6535faa0f9f75509988458dbfc03e8a7ec83aede Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:10:43 -0400 Subject: [PATCH 08/12] ADFA-4357 Add brevity directive for docs/tickets/messages to AGENTS.md --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 50fbb0d3b5..ad5c335cdc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,6 +10,8 @@ Always plan before building. As part of the plan, estimate the projected size of When you change code, update the docs that describe it in the same change — a module's `README.md`, `ARCHITECTURE.md`, or an ADR — so a doc never outlives the API it documents. See REVIEW.md (Code quality) for the rule. If the doc fix is genuinely out of scope, file a ticket rather than leaving it to drift. +Keep docs, tickets, commit messages, and PR descriptions crisp — say it once, lead with the point, cut hedging and restated context. Brevity is the soul of wit; a reader's attention is the scarce resource. + Always protect the two critical Android interaction locations: (1) the top bar containing the time, notification icons, and status icons, and (2) the bottom bar containing the home button, back button, and app selector. Never put UI elements of our app on top of those Android reserved areas. ## Official/Public Actions Run in CI, Not Locally From 73673854ccb910a9d60db54d408b4db1d41e3c18 Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:20:38 -0400 Subject: [PATCH 09/12] ADFA-4357 Concision pass on the doc set; fix stale tooltip API ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten prose across CLAUDE.md, AGENTS.md, ARCHITECTURE.md, REVIEW.md, and the ADRs — cut hedging, doubled phrasings, and restated context; no facts, paths, commands, or decisions changed. Also: - REVIEW.md §9: drop the stale showIDETooltip reference in the intro. - ARCHITECTURE.md: reconcile the data-flow UI note with ADR 0009 (existing UI is Views; new UI is Compose) instead of a flat 'not Compose'. --- AGENTS.md | 24 ++++++------- ARCHITECTURE.md | 21 ++++++------ CLAUDE.md | 21 ++++++------ REVIEW.md | 34 +++++++++---------- docs/adr/0001-persistence-without-room.md | 10 +++--- ...on-device-builds-via-gradle-tooling-api.md | 2 +- .../0003-vendored-forked-desktop-toolchain.md | 8 ++--- docs/adr/0004-embedded-termux-runtime.md | 6 ++-- docs/adr/0005-per-abi-product-flavors.md | 2 +- docs/adr/0006-koin-dependency-injection.md | 4 +-- docs/adr/0007-strictmode-whitelist-engine.md | 4 +-- docs/adr/0008-retain-androidide-namespace.md | 8 ++--- docs/adr/0009-jetpack-compose-for-new-ui.md | 6 ++-- docs/adr/README.md | 4 +-- 14 files changed, 76 insertions(+), 78 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ad5c335cdc..ffa7ea7260 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,37 +1,37 @@ -Code On The Go is an Android app which allows the user to edit, build and deploy their own new Android apps. It is an Integrated Development Environment similar to Eclipse or VSCode. +Code On The Go is an Android IDE — it lets users edit, build, and deploy their own Android apps on-device, like Eclipse or VSCode. -There is at least one Android emulator available. Use `adb devices -l | grep -v offline` to find it. Then use the ANDROID_SERIAL environment variable. +There is at least one Android emulator available. Find it with `adb devices -l | grep -v offline`, then use the `ANDROID_SERIAL` env var. -For new persistence, use SQLite or the filesystem — never add Room. (See ARCHITECTURE.md for the full persistence model and the one legacy Room exception.) +For new persistence, use SQLite or the filesystem — never add Room. (See ARCHITECTURE.md for the full model and the one legacy exception.) -Avoid adding dependencies - we probably already have everything loaded that you need. Look in build.gradle.kts for details. +Avoid adding dependencies — we almost certainly already have what you need. Check `build.gradle.kts`. -Always plan before building. As part of the plan, estimate the projected size of the change, both number of files and lines of code. When that estimate is greater than 500 lines of code or more than 10 files, you must organize the work into 2 or more smaller change sets, so that the user can help you make 2 or more PRs (Pull Requests). +Plan before building, and size the change (files + LOC). If it will exceed 500 LOC or 10 files, split it into 2+ change sets so the user can land them as separate PRs. -When you change code, update the docs that describe it in the same change — a module's `README.md`, `ARCHITECTURE.md`, or an ADR — so a doc never outlives the API it documents. See REVIEW.md (Code quality) for the rule. If the doc fix is genuinely out of scope, file a ticket rather than leaving it to drift. +When you change code, update the docs that describe it in the same change — a module's `README.md`, `ARCHITECTURE.md`, or an ADR — so a doc never outlives the API it documents. See REVIEW.md (Code quality). If the doc fix is out of scope, file a ticket rather than let it drift. Keep docs, tickets, commit messages, and PR descriptions crisp — say it once, lead with the point, cut hedging and restated context. Brevity is the soul of wit; a reader's attention is the scarce resource. -Always protect the two critical Android interaction locations: (1) the top bar containing the time, notification icons, and status icons, and (2) the bottom bar containing the home button, back button, and app selector. Never put UI elements of our app on top of those Android reserved areas. +Never draw our UI over the two Android system bars: the top status bar (clock, notifications, status icons) and the bottom navigation bar (home, back, recents). ## Official/Public Actions Run in CI, Not Locally -Anything official or public-facing must happen through version-controlled GitHub Actions (`.github/workflows/*.yml`) — never from the user's laptop or a local machine. This includes uploading SonarQube/SonarCloud analyses, publishing releases or artifacts, deploying, and pushing to external services. The repo holds tokens like `SONAR_TOKEN` as GitHub secrets scoped to those workflows by design; do not go hunting for them locally. When asked to run something like the sonar task on the local machine, treat it as **local verification only** (run the tests/build to confirm a fix works) and let the official analysis happen in CI. +Anything official or public-facing runs only through version-controlled GitHub Actions (`.github/workflows/*.yml`), never locally — SonarQube/SonarCloud uploads, releases, artifact publishing, deploys, pushes to external services. Tokens like `SONAR_TOKEN` are GitHub secrets scoped to those workflows; don't hunt for them locally. Asked to run e.g. the sonar task locally, treat it as verification only (build/test to confirm a fix) and let the official analysis happen in CI. ## Reading Jira Tickets -Prefer the local authenticated `jira` CLI for reading tickets — it is configured via the `JIRA_API_TOKEN`, `JIRA_HOST`, and `JIRA_USER` environment variables. Example: `jira issue view ADFA-1234`. Do NOT start the Atlassian MCP OAuth flow for ticket reads; that heavyweight authentication is unnecessary when the CLI is available. +Read tickets with the local authenticated `jira` CLI (e.g. `jira issue view ADFA-1234`), configured via `JIRA_API_TOKEN`, `JIRA_HOST`, and `JIRA_USER`. Don't start the Atlassian MCP OAuth flow for reads — it's unnecessary when the CLI works. ## SonarQube MCP Server -The sonarqube MCP server runs in a Docker container, so Docker must be running before launching Claude Code. Its first launch pulls a ~225MB image (`mcp/sonarqube:latest`), which exceeds Claude Code's 30s MCP handshake timeout — so the initial connect reports a timeout even though nothing is broken. Pre-pull the image (or let one launch finish) so subsequent `/mcp` reconnects succeed. A `docker system prune` removes the image and reintroduces the slow first launch. +The sonarqube MCP server runs in Docker, so Docker must be up before launching Claude Code. Its first launch pulls a ~225MB image (`mcp/sonarqube:latest`) that exceeds Claude Code's 30s MCP handshake timeout — so the first connect reports a timeout though nothing is broken. Pre-pull the image (or let one launch finish) so later `/mcp` reconnects succeed. `docker system prune` removes it and brings back the slow first launch. ## Resolving CI Job Names -When the user references a CI/CD job by name (e.g., "the sonar job", "the analyze workflow"), read `.github/workflows/*.yml` first. The workflow YAML is the authoritative source for the exact gradle/shell invocation — do not introspect gradle tasks or grep build files to reverse-engineer the command. +When the user names a CI/CD job ("the sonar job", "the analyze workflow"), read `.github/workflows/*.yml` — the YAML is the authoritative gradle/shell invocation. Don't reverse-engineer it from gradle tasks or build files. ## Multi-line git/gh Messages -**Default to writing the body to a tempfile** via the Write tool, then `git commit -F /tmp/msg.txt` or `gh pr create --body-file /tmp/body.md`. Treat heredoc/`--body "$(cat < Audience: engineers working in this repository. This describes how the code is *actually* organized today, including the places where the codebase is mid-migration. Where a pattern is the agreed target for new code, it is called out explicitly. +> Audience: engineers working in this repository. This describes how the code is *actually* organized today, including where the codebase is mid-migration. Patterns that are the agreed target for new code are called out explicitly. ## Overview Code On The Go (CoGo) is a full Android IDE that runs **on the device** — it edits, builds, and deploys real Android apps offline, embedding a Termux toolchain and running an actual Gradle build in a separate process via the `tooling-api`. It is the maintained successor to AndroidIDE, so the codebase namespace is still `com.itsaky.androidide`. -There is **no single architectural philosophy** across the whole app. It is a large, layered application that is still **predominantly View-based**, where newer feature surfaces (plugin manager, AI agent, git, project list) follow a deliberate **Unidirectional Data Flow (UDF)** with Koin DI, `ViewModel` + `StateFlow`, sealed UI-state/effect types, and repositories — while older surfaces still use `LiveData` and talk to GreenRobot EventBus directly. New work follows the UDF pattern documented below, and new UI is built in **Jetpack Compose** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)) — Compose replaces the view layer only; the UDF stack (ViewModel + `StateFlow`, Koin, repositories) is unchanged. Existing XML/View screens remain until they're substantially reworked. +There is **no single architectural philosophy** across the whole app. This large, layered application is still **predominantly View-based**: newer feature surfaces (plugin manager, AI agent, git, project list) follow a deliberate **Unidirectional Data Flow (UDF)** with Koin DI, `ViewModel` + `StateFlow`, sealed UI-state/effect types, and repositories, while older surfaces still use `LiveData` and talk to GreenRobot EventBus directly. New work follows the UDF pattern documented below, and new UI is built in **Jetpack Compose** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)) — Compose replaces the view layer only; the UDF stack (ViewModel + `StateFlow`, Koin, repositories) is unchanged. Existing XML/View screens remain until substantially reworked. ## Core Architecture & Data Flow -The intended layering for feature code is **UI → ViewModel → Repository → data source**, with state flowing up and events/intents flowing down. Dependencies are provided by Koin (`coreModule`, `pluginModule`) and constructor-injected into ViewModels. +Feature code layers as **UI → ViewModel → Repository → data source**, with state flowing up and events/intents flowing down. Koin provides dependencies (`coreModule`, `pluginModule`), constructor-injected into ViewModels. - **Data sources** — Room (`RecentProjectRoomDatabase` + DAO, `suspend` functions), raw SQLite (`SQLiteOpenHelper`, e.g. `localWebServer/WebServer`), the filesystem/preferences, the embedded `tooling-api` (on-device Gradle), and external clients (Gemini via the Google GenAI SDK, on-device llama.cpp, JGit). Most are exposed through `suspend` functions. - **Repositories** — e.g. `agent/repository/GeminiRepository`, `repositories/PluginRepository`, `repositories/BreakpointRepository`. They wrap data sources and hide threading/IO from the ViewModel. - **ViewModels** — run work in `viewModelScope` on `Dispatchers.IO`, hold a private `MutableStateFlow`/`MutableSharedFlow`, and expose read-only `StateFlow`/`SharedFlow`. One-shot effects (toasts, navigation, dialogs) go through a separate `SharedFlow` of a sealed `*UiEffect` type. -- **UI (Fragments / Activities / Views)** — collect state in a lifecycle-aware coroutine and render it; user actions are sent back to the ViewModel as method calls or sealed `*UiEvent` intents. The UI is **Android Views + Fragments + RecyclerView adapters**, not Jetpack Compose (`compose-preview` is a tool for previewing the *user's* Compose code, not CoGo's own UI). +- **UI (Fragments / Activities / Views)** — collect state in a lifecycle-aware coroutine and render it; user actions return to the ViewModel as method calls or sealed `*UiEvent` intents. The existing UI is **Android Views + Fragments + RecyclerView adapters**; new UI is Jetpack Compose ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)). (`compose-preview` previews the *user's* Compose code, not CoGo's own.) ``` ┌─────────────────────────────────────────────┐ @@ -49,7 +49,7 @@ The intended layering for feature code is **UI → ViewModel → Repository → (build progress, install results, editor signals) outside the UDF spine. ``` -**EventBus is a deliberate side-channel.** Long-running, cross-module signals (build/install lifecycle, editor events) are broadcast via GreenRobot EventBus (`@Subscribe(threadMode = ThreadMode.MAIN)`) and the `eventbus-events` module's shared event types. Treat it as the integration bus *between* subsystems; do not use it to replace a ViewModel's own state inside a single screen. +**EventBus is a deliberate side-channel.** Long-running, cross-module signals (build/install lifecycle, editor events) are broadcast via GreenRobot EventBus (`@Subscribe(threadMode = ThreadMode.MAIN)`) and the `eventbus-events` module's shared event types. Treat it as the integration bus *between* subsystems; don't use it to replace a ViewModel's own state inside a single screen. ## Module Structure @@ -76,9 +76,9 @@ Strategy: **layer-and-subsystem based**, not feature-by-feature. The Gradle buil ## Build & Module Configuration -These structural facts shape every module. The day-to-day build *commands* live in `CLAUDE.md`; the rules that produce those commands live here. +These structural facts shape every module. Day-to-day build *commands* live in `CLAUDE.md`; the rules that produce them live here. -- **Centralized convention logic.** All Android module setup flows through `composite-builds/build-logic` (`conf/AndroidModuleConf.kt`). Modules stay thin; configuration is shared, so understanding any module's setup starts here. +- **Centralized convention logic.** All Android module setup flows through `composite-builds/build-logic` (`conf/AndroidModuleConf.kt`). Modules stay thin and share configuration, so understanding any module's setup starts here. - **ABI product flavors.** Every Android module *except* `:plugin-api` gets two flavors on the `abi` dimension — `v7` (`armeabi-v7a`) and `v8` (`arm64-v8a`) — defined centrally. There is no flavorless variant; tasks are `assembleV8Debug`, `assembleV7Release`, etc. - **SDK levels** (`build-logic/.../build/config/BuildConfig.kt`): `COMPILE_SDK=36`, `MIN_SDK=28`, `TARGET_SDK=28`. `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16` is the floor for the apps a *user* builds with CoGo — distinct from CoGo's own `MIN_SDK`. - **Native asset bundling.** The on-device LLM (`llama-impl`) ships as a per-flavor native AAR, wired through the root `build.gradle.kts` (`bundleLlamaV8Assets` / `assembleV8Assets`, …); prebuilt per-flavor assets live under `assets/release/v7/` and `assets/release/v8/`. @@ -110,7 +110,7 @@ These structural facts shape every module. The day-to-day build *commands* live - **Process/long-task state** uses dedicated sealed hierarchies — e.g. `BuildState`, `TaskState`, `InstallationState`, `ApkInstallationViewModel.SessionState`, `agent/AgentState`. - **Legacy screens** still expose `LiveData` (~8 ViewModels) instead of `StateFlow` (~20). When touching one substantially, prefer migrating it to `StateFlow`. -Real example from `ui/models/PluginManagerUiState.kt` — the pattern to copy for new screens: +Real example from `ui/models/PluginManagerUiState.kt` — copy this pattern for new screens: ```kotlin // Persistent, immutable UI state — exposed as StateFlow @@ -163,8 +163,7 @@ fun onEvent(event: PluginManagerUiEvent) = viewModelScope.launch(Dispatchers.IO) ## Testing Guidelines -Test code lives both alongside each module and in the shared `testing:{unit,android,lsp,tooling,common}` harnesses. Run with the flox wrapper, e.g. -`flox activate -d flox/local -- ./gradlew :testing:unit:test` or a module's `:module:test --tests "…"`. +Test code lives both alongside each module and in the shared `testing:{unit,android,lsp,tooling,common}` harnesses. Run with the flox wrapper, e.g. `flox activate -d flox/local -- ./gradlew :testing:unit:test` or a module's `:module:test --tests "…"`. | Layer | Runner / Tools | What to test | |---|---|---| @@ -176,5 +175,5 @@ Preferences and conventions: - **Assertions: Google Truth** (`assertThat(x).isEqualTo(...)`) over raw JUnit asserts. - **Mocking: MockK** for new code; relax it deliberately rather than over-stubbing. - For UDF ViewModels, drive `onEvent(...)`/method calls against a fake or mocked repository and assert the emitted `UiState` sequence (collect the `StateFlow`); assert effects by collecting the effect `SharedFlow`. -- On the M2 emulator, prefer short `flakySafely` timeouts (2–3s) and `AccessibilityNodeInfo.ACTION_CLICK` over raw coordinate taps — the bottom system-bar region swallows coordinate clicks, and the two system bars (top status, bottom nav) must never be obstructed by tests. +- On the M2 emulator, prefer short `flakySafely` timeouts (2–3s) and `AccessibilityNodeInfo.ACTION_CLICK` over raw coordinate taps — the bottom system-bar region swallows coordinate clicks, and tests must never obstruct the two system bars (top status, bottom nav). ``` diff --git a/CLAUDE.md b/CLAUDE.md index 2a6c013e5c..4deab70079 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,43 +1,42 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +Guidance for Claude Code (claude.ai/code) in this repository. ## What this is -Code On The Go (CoGo) is an Android IDE that runs *on an Android device* — it lets users edit, build, and deploy real Android apps from a phone, offline. It is the actively-maintained successor to AndroidIDE, so the Gradle/AGP namespace is still `com.itsaky.androidide` throughout the codebase even though the product is "Code On The Go". A bundled Termux provides the shell/toolchain, and a `tooling-api` runs a real Gradle build inside the app. +Code On The Go (CoGo) is an Android IDE that runs *on the device* — edit, build, and deploy real Android apps from a phone, offline. It's the maintained successor to AndroidIDE, so the Gradle/AGP namespace stays `com.itsaky.androidide` even though the product is "Code On The Go". A bundled Termux provides the shell/toolchain; a `tooling-api` runs a real Gradle build inside the app. -`AGENTS.md` holds operational rules (branch naming, Jira CLI, SonarQube MCP, session/push protocol). Read it — this file does not duplicate it. +`AGENTS.md` holds the operational rules (branch naming, Jira CLI, SonarQube MCP, session/push protocol) — read it; this file doesn't duplicate it. ## Build & test -All Gradle invocations must be wrapped in `flox` for the correct toolchain: +Wrap every Gradle invocation in `flox` for the right toolchain: ```bash flox activate -d flox/local -- ./gradlew ``` -Common tasks: - **Debug APK (arm64, the usual target):** `flox activate -d flox/local -- ./gradlew :app:assembleV8Debug --parallel --max-workers=6` - **Single unit test:** `flox activate -d flox/local -- ./gradlew :module:test --tests "com.itsaky.androidide.SomeTest"` - **Module unit tests:** `flox activate -d flox/local -- ./gradlew :testing:unit:test` -When a CI/CD job is referenced by name ("the sonar job", "the analyze workflow"), `.github/workflows/*.yml` is the authoritative source for the exact command — don't reverse-engineer it from Gradle tasks. Official/public actions (Sonar upload, releases, deploys) run only in CI, never locally; local runs are verification only. +For a CI/CD job named by the user ("the sonar job", "the analyze workflow"), `.github/workflows/*.yml` is the authoritative command — don't reverse-engineer it from Gradle tasks. Official/public actions (Sonar upload, releases, deploys) run only in CI; local runs are verification only. ### ABI product flavors (v7 / v8) -Every Android module carries `v7` (`armeabi-v7a`) and `v8` (`arm64-v8a`) flavors, so build tasks are `assembleV8Debug`, `assembleV7Release`, etc. — there is **no** flavorless `assembleDebug`. See [ARCHITECTURE.md](ARCHITECTURE.md) (Build & Module Configuration) for how flavors, native asset bundling, and SDK levels are configured. +Every module carries `v7` (`armeabi-v7a`) and `v8` (`arm64-v8a`) flavors, so build tasks are `assembleV8Debug`, `assembleV7Release`, etc. — there is **no** flavorless `assembleDebug`. See [ARCHITECTURE.md](ARCHITECTURE.md) (Build & Module Configuration) for flavors, native asset bundling, and SDK levels. ## Architecture -See **[ARCHITECTURE.md](ARCHITECTURE.md)** — the single source of truth for the module map, layering and data flow, dependency rules, the technology stack (DI, async, persistence, networking), state management, and the testing strategy. Don't re-document those here; update ARCHITECTURE.md instead. +See **[ARCHITECTURE.md](ARCHITECTURE.md)** — the single source of truth for the module map, layering/data flow, dependency rules, tech stack (DI, async, persistence, networking), state management, and testing strategy. Don't re-document those here; update ARCHITECTURE.md. ## Project-specific constraints - **Avoid new dependencies** — the build almost certainly already has what's needed. Check `gradle/libs.versions.toml` and `build.gradle.kts` first. - **Protect the two Android system bars** in any UI work: the top status bar (clock, notifications, status icons) and the bottom navigation bar (home, back, recents). Don't draw over or intercept them. -- **Plan and size before building.** If a change is estimated at >500 LOC or >10 files, split it into 2+ smaller PRs. -- `.androidide_root` is a sentinel file used by tests to locate the project root — do not delete it. +- **Plan and size before building.** Estimated >500 LOC or >10 files → split into 2+ PRs. +- `.androidide_root` is a sentinel file tests use to locate the project root — don't delete it. ## Code style -2-space indents everywhere. Java uses Google style (`google-java-format`); Kotlin uses `ktfmt` Google-internal style; XML uses the Android Studio formatter. Branch names must match `.../ADFA-#####` (3–5 digits) — see CONTRIBUTING.md; a pre-commit hook enforces it (`sh ./scripts/install-git-hooks.sh`). +2-space indents everywhere. Java: Google style (`google-java-format`); Kotlin: `ktfmt` Google-internal style; XML: Android Studio formatter. Branch names must match `.../ADFA-#####` (3–5 digits) — see CONTRIBUTING.md; a pre-commit hook enforces it (`sh ./scripts/install-git-hooks.sh`). diff --git a/REVIEW.md b/REVIEW.md index f03c3a16e1..1cd0a2c3c1 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -118,30 +118,30 @@ Keep event names/params stable and low-cardinality; **no PII, file paths with us ## 8. Accessibility — every actionable view speaks -CoGo serves visually-impaired developers, so screen-reader support (TalkBack) is a correctness requirement for UI work, not a nice-to-have. The pattern below is what ADFA-2667 established — hold new UI to it. New UI is Compose ([§10](#10-architecture-alignment) / [ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)), so each rule below gives both the View and the Compose form; the requirement is identical in either toolkit. +CoGo serves visually-impaired developers, so TalkBack support is a correctness requirement, not a nice-to-have (pattern set by ADFA-2667). New UI is Compose ([§10](#10-architecture-alignment) / [ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)), so each rule gives the View and Compose form — the requirement is the same in either. - **Label every actionable view.** Buttons, `ImageButton`s, and icon-only controls need a `contentDescription`. - - *View:* `android:contentDescription="@string/cd_…"`. An unlabeled icon button is announced as "unlabeled button" — useless. - - *Compose:* pass the `contentDescription` argument on `Icon`/`Image`; for a custom clickable or an `IconButton` (whose inner `Icon` is usually `contentDescription = null`), put the label on the control with `Modifier.semantics { contentDescription = … }`. -- **Don't forget the elements built in code.** - - *View:* the easy miss is anything *not* in XML — toolbar action items, RecyclerView rows, dynamically-inflated buttons. Set `contentDescription` programmatically when you create them (see `EditorHandlerActivity.getToolbarContentDescription()`, `MainActionsListAdapter`, `DiagnosticItemAdapter` from ADFA-2667). - - *Compose:* `Icon`/`Image` *force* you to pass `contentDescription`, so the miss is supplying a vague one — or `null` on something that's actually actionable. Don't reach for `null` just to satisfy the signature. -- **Silence decoration.** Purely visual elements — separators, background images, an icon sitting next to a label that already says the same thing — should be skipped by TalkBack, not read as noise. + - *View:* `android:contentDescription="@string/cd_…"`; an unlabeled icon button reads as "unlabeled button". + - *Compose:* pass `contentDescription` on `Icon`/`Image`; for a custom clickable or `IconButton` (whose inner `Icon` is usually `null`), label the control via `Modifier.semantics { contentDescription = … }`. +- **Don't forget elements built in code.** + - *View:* the miss is anything *not* in XML — toolbar actions, RecyclerView rows, inflated buttons. Set `contentDescription` when you create them (see `EditorHandlerActivity.getToolbarContentDescription()`, `MainActionsListAdapter`, `DiagnosticItemAdapter`). + - *Compose:* `Icon`/`Image` force a `contentDescription`, so the miss is a vague label — or a lazy `null` on something actionable. +- **Silence decoration.** Separators, background images, an icon beside a label that already says the same thing — skip them, don't read them as noise. - *View:* `android:importantForAccessibility="no"` (or `View.IMPORTANT_FOR_ACCESSIBILITY_NO`). - - *Compose:* pass `contentDescription = null` on the decorative `Icon`/`Image`; to drop a whole subtree from the tree, use `Modifier.clearAndSetSemantics { }`. -- **Describe the action, not the picture.** Prefer what tapping *does* over what the icon *looks like*: `cd_sync_project`, not "circular arrows icon". When a control toggles state, make the description state-aware — `cd_drawer_open` vs. `cd_drawer_close`, expand vs. collapse — rather than one ambiguous label. (Same in both toolkits; in Compose pull the text with `stringResource(R.string.cd_…)`.) -- **Externalize, with the `cd_` convention.** Content-description strings live in `strings.xml` named `cd_*` (so they're greppable, translatable via Crowdin, and not inline literals). Reuse an existing `cd_` string before adding a near-duplicate. Note the lint `HardcodedText` check does **not** see Compose literals — a hardcoded `contentDescription = "Sync"` won't be flagged, so reviewers must catch it. -- **Bonus: it stabilizes tests.** Screen-reader semantics are also what UI tests match on — `ACTION_CLICK` for Views, `onNodeWithContentDescription(…)` for Compose — so good a11y and reliable instrumentation tests are the same work. + - *Compose:* `contentDescription = null` on the decorative `Icon`/`Image`, or `Modifier.clearAndSetSemantics { }` to drop a subtree. +- **Describe the action, not the picture.** `cd_sync_project`, not "circular arrows icon". For toggles, make it state-aware — `cd_drawer_open` vs. `cd_drawer_close` — not one ambiguous label. (In Compose, pull text with `stringResource(R.string.cd_…)`.) +- **Externalize, with the `cd_` convention.** Content descriptions live in `strings.xml` as `cd_*` — greppable, translatable, reusable; check for an existing one first. `HardcodedText` lint does **not** catch Compose literals, so reviewers must. +- **Bonus — it stabilizes tests.** Screen-reader semantics are what UI tests match on (`ACTION_CLICK` for Views, `onNodeWithContentDescription(…)` for Compose), so a11y and reliable instrumentation tests are the same work. ## 9. Contextual help — long-press works everywhere -Help in CoGo is reached by **long-press**, anywhere, and opens a progressive three-tier experience: **Tier 1 & 2 are tooltips** (anchored popups from `idetooltips`, via `showIDETooltip`'s `level` argument), and **Tier 3 is a full help web page** reached from the tooltip's "See More" link. This is a product promise — a long-press should never be met with silence. See [`idetooltips/README.md`](idetooltips/README.md) for the system itself. +Help in CoGo is reached by **long-press**, anywhere: a progressive three-tier experience — **Tiers 1 & 2 are tooltips** (anchored popups from `idetooltips`), **Tier 3 is a full help web page** via the tooltip's "See More" link. A long-press should never be met with silence. -- **Wire up help on new interactive elements.** Any view that does something when tapped — buttons, icon controls, menu items, list rows, toolbar actions — gets long-press help. A new actionable view with no tooltip affordance is an incomplete change, the same as a missing `contentDescription`. -- **Cover new screens and panels too.** Even where individual pixels aren't interactive, a new screen/panel/dialog needs at least a top-level help entry so help is reachable from anywhere on that surface. -- **The affordance is the requirement, not finished copy.** Tooltip *content* may still be getting authored — that's fine — but the long-press must already be wired and route into the tier system. Don't ship UI that can never surface help. -- **Reuse the system; don't reinvent it.** Wire help through the `idetooltips` module — today that's the `View.displayTooltipOnLongPress(context, anchorView, category, tag)` extension (`setOnLongClickListener` → `TooltipManager.showTooltip`) — rather than a one-off popup, so all three tiers and the tooltip store stay consistent. -- **Compose has no native entry point yet** (tracked by **ADFA-4381**). The helper above is View-based (it needs an `anchorView`). Until `idetooltips` grows a Compose API, a new composable surface wires help via `AndroidView` interop or a thin wrapper that exposes the anchor — flag it in review rather than skipping help, and prefer building the reusable `Modifier`/wrapper once over copy-pasting interop at each call site. +- **Wire up help on new interactive elements.** Anything tappable — buttons, icon controls, menu items, list rows, toolbar actions — gets long-press help. A new actionable view with no tooltip is as incomplete as a missing `contentDescription`. +- **Cover new screens and panels too.** Even where pixels aren't interactive, a new screen/panel/dialog needs a top-level help entry so help is always reachable. +- **The affordance is the requirement, not finished copy.** Tooltip content may still be in authoring — fine — but the long-press must be wired and routed into the tier system. Don't ship UI that can never surface help. +- **Reuse the system.** Wire help through `idetooltips` — today the `View.displayTooltipOnLongPress(context, anchorView, category, tag)` extension (`setOnLongClickListener` → `TooltipManager.showTooltip`) — not a one-off popup. +- **Compose has no native entry point yet** (tracked by **ADFA-4381**). The helper is View-based (needs an `anchorView`), so until `idetooltips` grows a Compose API, a composable wires help via `AndroidView` interop. Flag it in review rather than skipping help, and build the reusable `Modifier`/wrapper once instead of copy-pasting interop. ## 10. Architecture alignment diff --git a/docs/adr/0001-persistence-without-room.md b/docs/adr/0001-persistence-without-room.md index ca51979788..534f657aa3 100644 --- a/docs/adr/0001-persistence-without-room.md +++ b/docs/adr/0001-persistence-without-room.md @@ -6,15 +6,15 @@ ## Context -Code On The Go stores modest amounts of local state — recent projects, tooltips, breakpoints, preferences, and assorted IDE settings — on resource-constrained Android devices, entirely offline. We care about APK size, method count, build time, and full control over storage, and we already run a large multi-module build where every annotation processor adds real cost. +Code On The Go stores modest local state — recent projects, tooltips, breakpoints, preferences, and assorted IDE settings — on resource-constrained Android devices, entirely offline. We care about APK size, method count, build time, and full control over storage, and we already run a large multi-module build where every annotation processor adds real cost. -Android's default persistence library, Room, generates code via `kapt`/KSP, ships a runtime, and manages a schema. It saves boilerplate but adds footprint and build overhead, and in practice it was applied inconsistently across the codebase. +Room, Android's default persistence library, generates code via `kapt`/KSP, ships a runtime, and manages a schema. It saves boilerplate but adds footprint and build overhead, and was applied inconsistently across the codebase. ## Decision New persistence uses **raw SQLite** (`SQLiteOpenHelper` / `SQLiteDatabase`) or the **filesystem / preferences**. Room is **not** used for new code. -The one existing exception is the **Recent Projects** feature (`app/src/main/java/com/itsaky/androidide/roomData/recentproject/`, `@Database version = 4`), which predates this decision and is grandfathered in. It is not to be extended with new entities or tables. (`idetooltips` still declares Room Gradle deps that it does not use; its tooltip store is raw SQLite — those deps should be removed.) +The one exception is the **Recent Projects** feature (`app/src/main/java/com/itsaky/androidide/roomData/recentproject/`, `@Database version = 4`), which predates this decision and is grandfathered in. Do not extend it with new entities or tables. (`idetooltips` still declares unused Room Gradle deps; its tooltip store is raw SQLite — remove those deps.) ## Consequences @@ -33,8 +33,8 @@ The one existing exception is the **Recent Projects** feature (`app/src/main/jav ## Alternatives considered -- **Room everywhere** — rejected: adds `kapt`/runtime/footprint and build cost; its convenience didn't justify the cost on constrained devices, and it had crept in unevenly. -- **DataStore / SharedPreferences only** — insufficient for the small amount of relational/queryable data (e.g. recent projects). +- **Room everywhere** — rejected: adds `kapt`/runtime/footprint and build cost not justified on constrained devices, and it had crept in unevenly. +- **DataStore / SharedPreferences only** — insufficient for relational/queryable data (e.g. recent projects). - **A third-party ORM** — more dependencies and surface for little gain over raw SQLite. ## Related diff --git a/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md b/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md index 29dd30e573..0cd4059956 100644 --- a/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md +++ b/docs/adr/0002-on-device-builds-via-gradle-tooling-api.md @@ -6,7 +6,7 @@ ## Context -The product's core promise is to build and deploy *real* Android apps on the device, offline. That means producing the same results as a desktop Gradle build — correct dependency resolution, AGP behavior, manifest merging, R8/D8 — not an approximation. +The product's core promise is to build and deploy *real* Android apps on-device, offline — producing the same results as a desktop Gradle build (correct dependency resolution, AGP behavior, manifest merging, R8/D8), not an approximation. Gradle is memory-hungry and can OOM or crash, especially on a phone. Running it inside the IDE process would couple the build's lifecycle and memory to the UI: a build OOM would take down the editor, and a stuck build couldn't be cleanly killed. diff --git a/docs/adr/0003-vendored-forked-desktop-toolchain.md b/docs/adr/0003-vendored-forked-desktop-toolchain.md index b439255401..4fc2f87840 100644 --- a/docs/adr/0003-vendored-forked-desktop-toolchain.md +++ b/docs/adr/0003-vendored-forked-desktop-toolchain.md @@ -6,13 +6,13 @@ ## Context -Compiling Java/Kotlin/Android projects requires desktop developer tools — `javac`, the JDK compiler/`jdeps`, the Eclipse JDT compiler, `layoutlib`, AAPT, JAXP, etc. These assume a desktop JDK environment and are not published in forms that run on Android's ART runtime. To build on-device (see [ADR 0002](0002-on-device-builds-via-gradle-tooling-api.md)), they have to be made to work there. +Compiling Java/Kotlin/Android projects requires desktop developer tools — `javac`, the JDK compiler/`jdeps`, the Eclipse JDT compiler, `layoutlib`, AAPT, JAXP, etc. These assume a desktop JDK environment and aren't published in forms that run on Android's ART runtime. On-device builds (see [ADR 0002](0002-on-device-builds-via-gradle-tooling-api.md)) require making them work there. ## Decision **Fork and vendor** these tools into the repository as composite builds under `composite-builds/build-deps` and `composite-builds/build-deps-common`, and **substitute** them for their `com.itsaky.androidide.build:*` Maven coordinates via `dependencySubstitution` in `settings.gradle.kts`. -Substituted modules include `javac`, `jdk-compiler`, `jdk-jdeps`, `jdt`, `layoutlib-api`, `java-compiler`, `jaxp`, `javapoet`, and `google-java-format`, among others. Any module needing one of these gets the forked, Android-compatible version transparently. +Substituted modules include `javac`, `jdk-compiler`, `jdk-jdeps`, `jdt`, `layoutlib-api`, `java-compiler`, `jaxp`, `javapoet`, and `google-java-format`, among others. Any consuming module gets the forked, Android-compatible version transparently. ## Consequences @@ -22,8 +22,8 @@ Substituted modules include `javac`, `jdk-compiler`, `jdk-jdeps`, `jdt`, `layout - Consumers reference normal coordinates; the substitution is centralized and invisible to them. **Negative / costs** -- A large vendored source subtree and a significant ongoing maintenance burden: tracking upstream changes and JDK/AGP/Gradle updates. -- The **majority of suppressed scanner findings live in these subtrees** — they are treated as vendored code (suppress-only, never "fixed" to our standards) per [SECURITY.md](../../SECURITY.md). +- A large vendored source subtree and ongoing maintenance: tracking upstream changes and JDK/AGP/Gradle updates. +- The **majority of suppressed scanner findings live in these subtrees** — treated as vendored code (suppress-only, never "fixed" to our standards) per [SECURITY.md](../../SECURITY.md). - Onboarding cost: contributors must understand the substitution model before touching builds. ## Alternatives considered diff --git a/docs/adr/0004-embedded-termux-runtime.md b/docs/adr/0004-embedded-termux-runtime.md index 59fd72d59e..d227d2e23d 100644 --- a/docs/adr/0004-embedded-termux-runtime.md +++ b/docs/adr/0004-embedded-termux-runtime.md @@ -6,9 +6,9 @@ ## Context -A self-contained on-device IDE needs a real POSIX shell, a package ecosystem (git, clang, coreutils, and the binaries the build depends on), and an interactive terminal. Building any of that from scratch — a shell, a package manager, a terminal emulator/view — is a multi-year effort and a distraction from the IDE itself. +A self-contained on-device IDE needs a real POSIX shell, a package ecosystem (git, clang, coreutils, and the binaries the build depends on), and an interactive terminal. Building any of that from scratch — shell, package manager, terminal emulator/view — is a multi-year effort and a distraction from the IDE itself. -Termux is a mature, GPL-licensed Android terminal and package environment that already solves exactly this. +Termux is a mature, GPL-licensed Android terminal and package environment that already solves this. ## Decision @@ -27,7 +27,7 @@ The IDE's build and shell flows run through this environment. - License-compatible with our GPLv3 project. **Negative / costs** -- A large native/runtime surface and a meaningful **security consideration**: Termux executes arbitrary commands, so command-execution sinks must be handled carefully (see [SECURITY.md](../../SECURITY.md)). +- A large native/runtime surface and a real **security consideration**: Termux executes arbitrary commands, so command-execution sinks must be handled carefully (see [SECURITY.md](../../SECURITY.md)). - Maintenance: tracking Termux upstream and integrating it with our build orchestration. - Adds to APK size and the set of native components per ABI ([ADR 0005](0005-per-abi-product-flavors.md)). diff --git a/docs/adr/0005-per-abi-product-flavors.md b/docs/adr/0005-per-abi-product-flavors.md index 42dd5004e5..dea7c6f12d 100644 --- a/docs/adr/0005-per-abi-product-flavors.md +++ b/docs/adr/0005-per-abi-product-flavors.md @@ -6,7 +6,7 @@ ## Context -The app bundles large native components per ABI — llama.cpp, plus the on-device toolchain/Termux binaries. A universal APK carrying both `armeabi-v7a` and `arm64-v8a` copies of everything would be very large. Code On The Go is distributed primarily as a **direct APK download** from the App Dev for All website, not exclusively through Google Play, so we cannot rely on Play's automatic per-ABI splitting to slim downloads. +The app bundles large native components per ABI — llama.cpp, plus the on-device toolchain/Termux binaries. A universal APK carrying both `armeabi-v7a` and `arm64-v8a` copies of everything would be very large. Code On The Go is distributed primarily as a **direct APK download** from the App Dev for All website, not exclusively through Google Play, so we can't rely on Play's automatic per-ABI splitting to slim downloads. ## Decision diff --git a/docs/adr/0006-koin-dependency-injection.md b/docs/adr/0006-koin-dependency-injection.md index 539f2a5f9f..ebeaf8dab2 100644 --- a/docs/adr/0006-koin-dependency-injection.md +++ b/docs/adr/0006-koin-dependency-injection.md @@ -6,7 +6,7 @@ ## Context -The app needs dependency injection to wire ViewModels, repositories, and managers across many modules. On a build this large (~80 modules), annotation-processing-based DI (Dagger/Hilt via `kapt`/KSP) adds noticeable compile time and couples DI to the Android Gradle plugin and the build graph. The team values fast iteration and simple, testable wiring. +The app needs dependency injection to wire ViewModels, repositories, and managers across many modules. On a build this large (~80 modules), annotation-processing DI (Dagger/Hilt via `kapt`/KSP) adds noticeable compile time and couples DI to the Android Gradle plugin and the build graph. The team values fast iteration and simple, testable wiring. ## Decision @@ -24,7 +24,7 @@ Use **Koin** for dependency injection. **Negative / costs** - DI errors surface at **runtime**, not compile time — a missing binding fails when resolved, so DI paths need test coverage. -- `ServiceLocator` is a deliberate service-locator escape hatch; over-use reintroduces hidden global state. Keep it limited and prefer constructor injection. +- `ServiceLocator` is a deliberate escape hatch; over-use reintroduces hidden global state. Keep it limited and prefer constructor injection. ## Alternatives considered diff --git a/docs/adr/0007-strictmode-whitelist-engine.md b/docs/adr/0007-strictmode-whitelist-engine.md index fd3e4ec3ae..e3e2d6a24f 100644 --- a/docs/adr/0007-strictmode-whitelist-engine.md +++ b/docs/adr/0007-strictmode-whitelist-engine.md @@ -6,9 +6,9 @@ ## Context -StrictMode is valuable for catching main-thread disk/network I/O and leaked resources early. But Code On The Go is a large app built on inherited and vendored code (the AndroidIDE lineage, the forked toolchain in [ADR 0003](0003-vendored-forked-desktop-toolchain.md), Termux) that trips many StrictMode violations we cannot reasonably fix. +StrictMode catches main-thread disk/network I/O and leaked resources early. But Code On The Go is a large app built on inherited and vendored code (the AndroidIDE lineage, the forked toolchain in [ADR 0003](0003-vendored-forked-desktop-toolchain.md), Termux) that trips many StrictMode violations we can't reasonably fix. -The two blunt options both fail us: disabling StrictMode hides regressions in **our** code, while enabling it with a death/penalty policy crashes or spams on **third-party** violations we don't control. +Both blunt options fail us: disabling StrictMode hides regressions in **our** code, while a death/penalty policy crashes or spams on **third-party** violations we don't control. ## Decision diff --git a/docs/adr/0008-retain-androidide-namespace.md b/docs/adr/0008-retain-androidide-namespace.md index 3aaf408612..cd8ab5f316 100644 --- a/docs/adr/0008-retain-androidide-namespace.md +++ b/docs/adr/0008-retain-androidide-namespace.md @@ -6,9 +6,9 @@ ## Context -Code On The Go is the rebranded successor to **AndroidIDE**. The product name, branding, and assets changed, but the inherited codebase carries the original identity deeply: the application id and Gradle namespace are `com.itsaky.androidide` (`BuildConfig.PACKAGE_NAME`), `rootProject.name` is `AndroidIDE`, and there are many thousands of references plus the generated `R` class, the manifest, package-qualified vendored substitutions, signing identity, and existing installs in the field. +Code On The Go is the rebranded successor to **AndroidIDE**. The product name, branding, and assets changed, but the inherited codebase carries the original identity deeply: the application id and Gradle namespace are `com.itsaky.androidide` (`BuildConfig.PACKAGE_NAME`), `rootProject.name` is `AndroidIDE`, plus many thousands of references, the generated `R` class, the manifest, package-qualified vendored substitutions, signing identity, and existing installs in the field. -Changing an Android **application id** breaks the update path for installed users (a different app id is a different app) and disrupts signing/identity continuity. A package rename of this size also ripples through the vendored `com.itsaky.androidide.build:*` substitutions ([ADR 0003](0003-vendored-forked-desktop-toolchain.md)). +Changing an Android **application id** breaks the update path for installed users (a different app id is a different app) and disrupts signing/identity continuity. A rename of this size also ripples through the vendored `com.itsaky.androidide.build:*` substitutions ([ADR 0003](0003-vendored-forked-desktop-toolchain.md)). ## Decision @@ -21,12 +21,12 @@ Changing an Android **application id** breaks the update path for installed user - Avoids a massive, high-risk refactor and the churn it would cause across vendored substitutions and the `R` class. **Negative / costs** -- The codebase namespace doesn't match the product name, which is confusing to newcomers. This is called out explicitly in [CLAUDE.md](../../CLAUDE.md) and [ARCHITECTURE.md](../../ARCHITECTURE.md) so the mismatch is expected, not surprising. +- The codebase namespace doesn't match the product name, confusing to newcomers. [CLAUDE.md](../../CLAUDE.md) and [ARCHITECTURE.md](../../ARCHITECTURE.md) call this out so the mismatch is expected, not surprising. - Care is needed to ensure user-facing strings/branding are overridden and no "AndroidIDE" naming leaks to users. ## Alternatives considered -- **Full rename to `org.appdevforall.*`** — rejected: breaks updates for existing users, requires enormous error-prone churn, and risks destabilizing the vendored substitutions for little functional gain. +- **Full rename to `org.appdevforall.*`** — rejected: breaks updates for existing users, requires enormous error-prone churn, and risks destabilizing the vendored substitutions for little gain. - **Partial rename** (some modules) — rejected: produces an inconsistent namespace with the same risks and no clean payoff. ## Related diff --git a/docs/adr/0009-jetpack-compose-for-new-ui.md b/docs/adr/0009-jetpack-compose-for-new-ui.md index 9ae3e76ad0..bc20fd7d8b 100644 --- a/docs/adr/0009-jetpack-compose-for-new-ui.md +++ b/docs/adr/0009-jetpack-compose-for-new-ui.md @@ -6,7 +6,7 @@ ## Context -Historically the IDE's own UI is **View-based** — XML layouts, `Fragment`s, and `RecyclerView` with Material Components (see [ADR 0006](0006-koin-dependency-injection.md) context and ARCHITECTURE.md). Newer surfaces already moved to a Unidirectional Data Flow (UDF) architecture — `ViewModel` + `StateFlow`, sealed UI-state/effect types, repositories, Koin DI — but still rendered through XML and `findViewById`/binding. +Historically the IDE's own UI is **View-based** — XML layouts, `Fragment`s, and `RecyclerView` with Material Components (see [ADR 0006](0006-koin-dependency-injection.md) context and ARCHITECTURE.md). Newer surfaces already moved to a Unidirectional Data Flow (UDF) architecture — `ViewModel` + `StateFlow`, sealed UI-state/effect types, repositories, Koin DI — but still render through XML and `findViewById`/binding. That split has a cost: two ways to build a screen, manual view-state wiring, boilerplate binding code, and UI logic that's awkward to unit-test. Jetpack Compose collapses the view layer into Kotlin, binds naturally to `StateFlow` via `collectAsState()`, and fits the UDF pattern the team already follows. Unlike most ADRs here, this one is **forward-looking** — it sets direction for new work rather than documenting an existing decision. @@ -29,11 +29,11 @@ That split has a cost: two ways to build a screen, manual view-state wiring, boi **Negative / costs** - A **mixed codebase** for the foreseeable future — Compose and Views coexist; contributors must know both, and interop (`ComposeView` / `AndroidView`) is needed at the seams. - New Compose dependencies and compiler plugin; some learning curve and added build surface. -- **Cross-cutting UI rules written for XML need Compose equivalents** — notably accessibility (`contentDescription` → `Modifier.semantics`/`contentDescription`, decorative views → `null` semantics) and the long-press 3-tier help affordance. See REVIEW.md §8–§9. The help affordance has no Compose entry point yet — building the bridge is tracked as a prerequisite in **ADFA-4381**; sequence it before the first Compose feature screen. +- **Cross-cutting UI rules written for XML need Compose equivalents** — notably accessibility (`contentDescription` → `Modifier.semantics`/`contentDescription`, decorative views → `null` semantics) and the long-press 3-tier help affordance (see REVIEW.md §8–§9). The help affordance has no Compose entry point yet — building the bridge is tracked as a prerequisite in **ADFA-4381**; sequence it before the first Compose feature screen. ## Alternatives considered -- **Stay View-based** — rejected: perpetuates two-way-to-build-a-screen and the binding boilerplate; doesn't leverage the `StateFlow` UDF the team already standardized on. +- **Stay View-based** — rejected: perpetuates two ways to build a screen and the binding boilerplate; doesn't leverage the `StateFlow` UDF the team already standardized on. - **Compose, and mandate migrating existing screens** — rejected: a forced migration of all legacy UI is high-risk churn with little user-facing value. Migrate opportunistically instead. ## Related diff --git a/docs/adr/README.md b/docs/adr/README.md index 35ba4b44f3..36c86c5437 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,13 +2,13 @@ This directory holds **Architecture Decision Records (ADRs)** — short documents capturing a significant architectural decision, its context, and its consequences. They explain *why* the codebase is the way it is, so a decision isn't silently undone later. -Format is lightweight **MADR / Nygard**: Context → Decision → Consequences → Alternatives. Most records here are *retroactive* — they document decisions already embedded in the code. +Format is lightweight **MADR / Nygard**: Context → Decision → Consequences → Alternatives. Most records here are *retroactive*, documenting decisions already embedded in the code. ## Conventions - One decision per file, named `NNNN-kebab-title.md` with a zero-padded sequence number. - **Status** lifecycle: `Proposed` → `Accepted` → `Superseded by NNNN` / `Deprecated`. Don't edit a decision after it's accepted; write a new ADR that supersedes it. -- Keep it short and concrete. Link related ADRs and the relevant code paths. +- Keep it short and concrete. Link related ADRs and relevant code paths. ## Index From 6b4d0ece0b727084cdbcfd4f3aa50b450325f1ba Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:31:56 -0400 Subject: [PATCH 10/12] ADFA-4357 Reframe experimental-flag item; move perf budget to ADFA-4383 - Experimental feature flag: clarify it's a user-facing early-access opt-in (singular flag), not a kill switch for us to disable features in the field. - Remove the performance-budget proposal; captured as ADFA-4383 instead. --- REVIEW.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index 1cd0a2c3c1..5393536eb7 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -173,7 +173,6 @@ CoGo is meant to work **without a network** — editing, building, and running a These aren't established rules yet — flagging them as candidates for the team: - **Backward compatibility:** `MIN_SDK=28` — review new APIs for guard/desugaring; remember user-built apps target `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16`. -- **Performance budget for startup & editor:** watch added work in `Application.onCreate`, `tooling-api` startup, and per-keystroke editor paths; flag synchronous heavy work there. -- **Feature flags / kill switches** for risky surfaces (AI agent, plugins, web server) so we can disable in the field without a release. +- **Experimental feature flag** — lets end users opt into not-yet-stabilized features (early access), with stable defaults for everyone else. This is a user-facing early-access toggle, not a kill switch for us to disable features in the field. Add to or push back on any of these — this doc is meant to evolve with the team. From eeaf8efdf96c78d124b9a54b67a126c32707a9aa Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:33:08 -0400 Subject: [PATCH 11/12] =?UTF-8?q?ADFA-4357=20Promote=20experimental-flag?= =?UTF-8?q?=20rule=20to=20accepted=20=C2=A712=20in=20REVIEW.md?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move it out of 'Open for discussion' into a numbered review section; gate not-yet-stable features behind the user-facing early-access flag. Renumber PR hygiene to §13. --- REVIEW.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index 5393536eb7..51efa831a3 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -160,7 +160,14 @@ CoGo is meant to work **without a network** — editing, building, and running a - **Network calls are non-blocking and failure-tolerant.** Analytics, Sentry, and Gemini calls run off the main thread and must tolerate timeouts/failures silently (no crash, no hang, no lost user action). A dropped analytics event is acceptable; a dropped keystroke is not. - **No network on the critical path.** Don't introduce a connectivity dependency into startup, the editor, or the build pipeline. -## 12. PR hygiene +## 12. Experimental features — gate behind the early-access flag + +Features that aren't fully stabilized ship behind the **experimental feature flag** — a user-facing opt-in for early access — so the default experience stays stable. + +- **Gate not-yet-stable work behind the flag, off by default.** A user who hasn't opted in never sees in-progress behavior. +- **It's an early-access opt-in, not a kill switch.** The flag is for users who want features early — not a lever for us to disable things in the field. + +## 13. PR hygiene - Focused and reviewable: aim for the **~500 LOC / 10-file** ceiling; split larger work into stacked PRs. - Title/branch follow `ADFA-####`; description says *what changed, why, how it was verified*, and flags anything intentionally out of scope (e.g. "UI-only, no unit test"). @@ -173,6 +180,5 @@ CoGo is meant to work **without a network** — editing, building, and running a These aren't established rules yet — flagging them as candidates for the team: - **Backward compatibility:** `MIN_SDK=28` — review new APIs for guard/desugaring; remember user-built apps target `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16`. -- **Experimental feature flag** — lets end users opt into not-yet-stabilized features (early access), with stable defaults for everyone else. This is a user-facing early-access toggle, not a kill switch for us to disable features in the field. Add to or push back on any of these — this doc is meant to evolve with the team. From 3805e5f340cf00be312e81d197ff3d643779b091 Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Fri, 19 Jun 2026 02:37:10 -0400 Subject: [PATCH 12/12] ADFA-4357 Drop backward-compat proposal and the now-empty Open-for-discussion section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MIN_SDK guard concern doesn't arise in practice; remove the item. It was the last proposal, so remove the empty section scaffolding too. REVIEW.md now ends at §13 PR hygiene. --- REVIEW.md | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index 51efa831a3..aa24a62247 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -172,13 +172,3 @@ Features that aren't fully stabilized ship behind the **experimental feature fla - Focused and reviewable: aim for the **~500 LOC / 10-file** ceiling; split larger work into stacked PRs. - Title/branch follow `ADFA-####`; description says *what changed, why, how it was verified*, and flags anything intentionally out of scope (e.g. "UI-only, no unit test"). - No stray debug logs, commented-out code, `dummy.apk`-style artifacts, or unrelated reformatting that buries the real diff. - ---- - -## Open for discussion (proposed additions) - -These aren't established rules yet — flagging them as candidates for the team: - -- **Backward compatibility:** `MIN_SDK=28` — review new APIs for guard/desugaring; remember user-built apps target `MIN_SDK_FOR_APPS_BUILT_WITH_COGO=16`. - -Add to or push back on any of these — this doc is meant to evolve with the team.