From 5bde56e8ae406de755b93a39c216afb6ee5681c1 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 21 Aug 2026 14:13:58 -0700 Subject: [PATCH 1/3] =?UTF-8?q?ADFA-4128:=20qb=2003/12=20protocol=20?= =?UTF-8?q?=E2=80=94=20The=20daemon=20wire=20format=20=E2=80=94=20the=20co?= =?UTF-8?q?ntract=20the=20IDE=20and=20compile=20daemon=20share?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W --- quickbuild/protocol/README.md | 179 ++++++ quickbuild/protocol/build.gradle.kts | 42 ++ .../quickbuild/protocol/DaemonProtocol.kt | 553 ++++++++++++++++++ .../protocol/DaemonProtocolDtoTest.kt | 194 ++++++ .../quickbuild/protocol/DaemonProtocolTest.kt | 101 ++++ .../quickbuild/testfixtures/OfflineGuard.kt | 113 ++++ settings.gradle.kts | 1 + 7 files changed, 1183 insertions(+) create mode 100644 quickbuild/protocol/README.md create mode 100644 quickbuild/protocol/build.gradle.kts create mode 100644 quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt create mode 100644 quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt create mode 100644 quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolTest.kt create mode 100644 quickbuild/protocol/src/testFixtures/kotlin/org/appdevforall/cotg/quickbuild/testfixtures/OfflineGuard.kt diff --git a/quickbuild/protocol/README.md b/quickbuild/protocol/README.md new file mode 100644 index 0000000000..31a6457618 --- /dev/null +++ b/quickbuild/protocol/README.md @@ -0,0 +1,179 @@ +# Quick Build wire formats (`:quickbuild:protocol`) + +Quick Build runs in three processes: CoGo (the IDE), the compile daemon (a JVM child process), +and the runtime living inside the user's proxy app. This module holds the wire types the first +two share, and a change to any format below is a change to three processes at once - which are +**not** upgraded together. + +Start at [`../README.md`](../README.md) for what Quick Build is and how a save flows through it. + +**This is deliberately not a field reference.** Every message shape is declared in code, linked +per section. What lives here is only what the code cannot tell you: invariants, traps, and the +compatibility rules. + +## Three formats, and only one of them lives in this module + +| Format | Transport | Declared in | +| --- | --- | --- | +| Daemon protocol | line-delimited JSON on the daemon's stdin/stdout | [`DaemonProtocol.kt`](src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt) | +| Deploy metadata | `metadataJson` on `IQuickBuildTarget.onPayload` | [`PayloadDeployer.kt`](../core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/PayloadDeployer.kt) writes, [`DeployMetadata.java`](../runtime/src/main/java/com/itsaky/androidide/quickbuild/runtime/DeployMetadata.java) parses | +| Build status | `statusJson` on `IQuickBuildTarget.onBuildStatus` | [`BuildStatusJson.kt`](../core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/BuildStatusJson.kt) writes, [`BuildStatus.java`](../runtime/src/main/java/com/itsaky/androidide/quickbuild/runtime/BuildStatus.java) parses | + +Only the daemon protocol has shared types, because both its ends compile against this module - and +since the wire itself is untyped JSON, the module holds the **names** too: `DaemonOps`, +`RequestKeys` and `ResponseKeys` are the op and field-name constants both ends use, so renaming one +is a compile error rather than a runtime one. + +`:quickbuild:runtime` does not compile against this module, so the two binder formats are an +encoder/parser *pair*: adding a field means editing both files, and **no compiler will tell you if +you edit only one.** + +## Daemon protocol: one JSON object per line, one reply per request + +### Transport rules + +- **Stdout carries protocol only.** [`DaemonMain`](../daemon/src/main/kotlin/org/appdevforall/cotg/quickbuild/daemon/DaemonMain.kt) + captures the real stdout for responses and redirects `System.out` to stderr, because the + in-process Kotlin compiler prints to stdout and one stray line would corrupt the stream. +- **One request in flight.** CoGo serializes every call behind a mutex, and the daemon's loop is + single-threaded on purpose. +- **Exit contract:** build errors never exit; a malformed line answers `ok:false` and the loop + keeps serving. `shutdown` or stdin EOF exits 0. Only a fatal internal error exits non-zero, + which CoGo reads as daemon death and respawns from. +- The daemon has no log file - progress goes to stderr, which CoGo drains and re-logs + ([debugging.md](../docs/debugging.md#the-daemon-has-no-log-of-its-own)). + +### Requests + +Six ops: `configure`, `compile`, `dex`, `relink`, `ping`, `shutdown`. + +- **`configure` opens the session** and fixes everything constant for it; a repeat replaces that + state, so there is no reconfigure op. It existence-checks every classpath entry, plugin and tool, + so a missing file fails at session start rather than mid-build. The build ops answer `ok:false` + if no `configure` ran. +- **The caller supplies every tool path; the daemon never guesses one.** `aapt2`, `d8Jar` and + `androidJar` are required, and `configure` answers `ok:false` with one diagnostic per missing + field. A guessed path could compile against another SDK's `android.jar` and only fail on device. + +### Responses + +`{"id", "ok", , "diagnostics"}`. Values are flat scalars, never nested. + +- `diagnostics` **can appear on success** - a build can succeed with warnings. +- **`line` and `column` are JSON numbers, and they stop here.** CoGo keeps error positions for + its own surfaces (Build Output, jump-to-error); the build-status format below is deliberately + position-free. +- **An absent `classesChanged` means *unknown*; an empty array means *nothing changed*.** The + deploy policy treats unknown conservatively; the two are not interchangeable. +- **An absent output path (`classesDir`, `dexFile`, `resourcesArsc`) fails the op.** The key is + mandatory on purpose: a conventional fallback under `outDir` would resolve whatever the previous + build left there, so the client would dex and deploy stale artifacts and report success with the + user's edit missing. `resourcesArsc` is the full relinked resource **apk**, not a bare table; the + key keeps its old name for protocol stability. + +### Per-build statistics: what the op did, not just how long two compilers took + +The compiler spans are only about half a warm edit `[measured on a56]`; the rest is output-tree +snapshots, the Java-ABI re-parse and per-file I/O. The counters (`CompileStats` / `DexStats`) exist +so that gap cannot be misread - javac looked like "the bottleneck" at 19-27% of a warm edit, and a +53 s first build looked like a per-edit cost when it was the cold compile seeding caches. Every +field is a counter or duration derived from no path, name or content, so the set is safe to forward +to analytics. + +Two are context rather than cost, and a timing row is unreadable without them: **`compileOrdinal`** +(`1` is the cold build; a fresh `configure`, including a respawn, correctly restarts the count) and +**`scratchFsType`** (rewriting the same class tree costs ~52x more on FUSE-backed emulated storage +than on the app's own filesystem `[measured on a56]`, so durations cannot be compared across +configurations without it). + +**Absent versus zero.** `fromValues` returns `null` when *none* of a group's keys are present, so an +older daemon never yields a zero-filled row reading as "measured, and free". Within a group a single +missing key defaults to `0`, indistinguishable from a measured zero. + +### `protocolVersion` is a session gate, and adding a key must not bump it + +CoGo aborts the session when `configure`'s `protocolVersion` differs **or is absent** - absence +fails too, because the daemon has stamped it since the protocol existed, so a missing field means +"not our daemon". + +**Adding a response key is additive and must NOT bump the version.** A staged daemon jar can lag +the client that talks to it, so bumping for a new optional key would break exactly the pairing the +additive shape exists to support. + +### Adding a numeric stat touches five places, and the codec is not one of them + +`ProtocolCodec.encode` is generic over the values, so it needs no change. The five all sit in +`CompileStats` / `DexStats`: the property, its `KEY_*` constant, `toValues()`, `fromValues()`, and +**the private `KEYS` list** - `fromValues` uses that list to decide "no keys present at all", so +missing it quietly weakens the absent-versus-zero guarantee above. + +**The analytics path has a hard cap:** Firebase allows 25 parameters per event, the reload bundle +is already near it, and a test enforces the bound. A new parameter may force merging an existing +one - the path is lossy on purpose. + +## Deploy metadata JSON (`IQuickBuildTarget.onPayload`) + +Sent with every payload; the dex, resources and assets travel beside it as ParcelFileDescriptors. + +- `restart` marks a deploy whose recompiled set touched a service, provider or custom + `Application`: the runtime persists, acks and exits instead of hot-swapping, and CoGo relaunches + into the persisted generation. Decided in [`DeployPolicy.kt`](../core/src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/DeployPolicy.kt), + contract in [`component-proxying-design.md`](../docs/component-proxying-design.md). +- **`restart` matches the exact string `"true"`.** Any other value - `"TRUE"`, a JSON boolean - + reads as false. +- **The `generation` argument travels beside this string, not inside it**, and a payload applies + only when strictly newer than the generation the app runs. + +## Build status JSON (`IQuickBuildTarget.onBuildStatus`) + +A compile error produces no payload, so this is how a running proxy app learns a build failed. +Kinds: `build_failed`, `build_ok`, `building`, `reinstall_pending`. + +- **Every value is a string, including the numbers.** The runtime's hand-rolled + [`MiniJson`](../runtime/src/main/java/com/itsaky/androidide/quickbuild/runtime/MiniJson.java) + keeps only strings and arrays of strings; it consumes numbers, booleans, nulls and nested objects + so extra fields still parse, but **drops those keys entirely** - send a JSON number and the + runtime sees an absent field. Unknown kinds and fields are ignored, so CoGo can extend the schema + without breaking installed proxy apps. +- `build_failed` reports one error, not a log - the first ERROR's first message line only, with + `moreErrors` counting the rest. **No file, line or column**: the overlay is a "your build failed, + this app is stale" warning, and locating the error is CoGo-side work, so position data never + crosses to the device. +- `reinstall_pending` is kind-only: a rebuilt update is waiting on an install confirmation that + Android will only show while CoGo is foregrounded, so the overlay tells the user - the one + person the CoGo-side signals cannot reach - to switch back. The copy is static and lives + runtime-side. An older runtime ignores the unknown kind, so the banner is simply absent there. + +## Version skew is normal here, and each format handles it differently + +A staged daemon jar can lag CoGo; the runtime AAR is compiled **into** the proxy app, so it only +changes after a proxy app rebuild and reinstall - reinstalling CoGo alone changes nothing in a +running app. + +| Change | Effect on an older peer | Safe? | +| --- | --- | --- | +| Add a daemon response key | older client ignores it; newer client reads absent as null | yes, and must not bump `protocolVersion` | +| Add a daemon request field | must be optional; the codec rejects an unknown required field as malformed | yes if optional | +| Bump `protocolVersion` | `configure` aborts the session with a mismatch error | breaking, by design | +| Add a deploy-metadata or build-status field | older runtime ignores it | yes, if the value is a string | +| Add a build-status kind | older runtime parses it to null and drops the message; its banner is simply absent | yes | +| Remove a build-status field | older runtime reads it as absent, same as a field CoGo never populated | yes; no version to bump - the build-status format carries none | +| Add an AIDL method | append at the end only; an older `oneway` stub answers "not handled" and the caller never notices | yes | +| Reorder or remove an AIDL method | silently calls the wrong transaction | never do this | + +A protocol regression that compiles is caught by no test: see the known gap in +[README, "How to Test"](../README.md#how-to-test). The name constants close the narrow half of +that gap - a renamed op or field no longer compiles on both sides - but nothing checks that a +*value's meaning* stayed the same, and the two binder formats have no shared names at all. + +## Key files + +| File | Role | +| --- | --- | +| [`DaemonProtocol.kt`](src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt) | request/response types, stat groups, the op and field-name constants, `PROTOCOL_VERSION` | +| [`ProtocolCodec.kt`](../daemon/src/main/kotlin/org/appdevforall/cotg/quickbuild/daemon/protocol/ProtocolCodec.kt) | parse and encode | +| [`RequestRouter.kt`](../daemon/src/main/kotlin/org/appdevforall/cotg/quickbuild/daemon/protocol/RequestRouter.kt) | dispatch, plus the backstop that keeps a build error from killing the process | +| [`DaemonMain.kt`](../daemon/src/main/kotlin/org/appdevforall/cotg/quickbuild/daemon/DaemonMain.kt) | serve loop, stdout/stderr split | +| [`DaemonService.kt`](../daemon/src/main/kotlin/org/appdevforall/cotg/quickbuild/daemon/DaemonService.kt) | the op implementations that fill the response values | +| [`DaemonProcessClient.kt`](../core/src/main/java/org/appdevforall/cotg/quickbuild/data/DaemonProcessClient.kt) | CoGo's client: spawn, serialize, version gate, unpacking | +| [`IQuickBuildTarget.aidl`](../runtime/src/main/aidl/com/itsaky/androidide/quickbuild/IQuickBuildTarget.aidl) / [`IQuickBuildHost.aidl`](../runtime/src/main/aidl/com/itsaky/androidide/quickbuild/IQuickBuildHost.aidl) | authoritative signatures for both binder directions | diff --git a/quickbuild/protocol/build.gradle.kts b/quickbuild/protocol/build.gradle.kts new file mode 100644 index 0000000000..b4399f7dca --- /dev/null +++ b/quickbuild/protocol/build.gradle.kts @@ -0,0 +1,42 @@ +plugins { + id("java-library") + id("org.jetbrains.kotlin.jvm") + // Publishes src/testFixtures as a variant consumable by the other quickbuild modules' + // tests. Home of the shared offline-guard scanner: this is the only module all of + // them already depend on. + id("java-test-fixtures") +} + +description = + "Quick Build daemon wire-protocol model: the request/response DTOs and protocol constants shared by the daemon and CoGo's client (ADFA-4128)" + +java { + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 +} + +kotlin { + jvmToolchain(17) +} + +tasks.withType { + useJUnitPlatform() +} + +// DoD coverage gate: >=90% line+branch on non-UI (domain/data) code. +// Same wiring as :quickbuild-daemon: the root-applied jacoco plugin auto-creates +// jacocoTestReport for JVM modules with the XML report off and no test dependency; +// the exec lands at the JVM default build/jacoco/test.exec. +tasks.named("jacocoTestReport") { + dependsOn(tasks.test) + reports { + xml.required.set(true) + html.required.set(true) + } +} + +dependencies { + testImplementation(libs.tests.junit.jupiter) + testImplementation(libs.tests.google.truth) + testRuntimeOnly(libs.tests.junit.platformLauncher) +} diff --git a/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt b/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt new file mode 100644 index 0000000000..3058cf3423 --- /dev/null +++ b/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt @@ -0,0 +1,553 @@ +package org.appdevforall.cotg.quickbuild.protocol + +/** + * A request the daemon accepts: one line-delimited JSON object over stdin, answered by one + * [DaemonResponse] line over stdout. + * + * These types live in :quickbuild:protocol so the daemon and CoGo's client compile against one + * definition of the wire rather than two conventions pinned by prose. + * + * @property id caller-assigned request id, echoed back on the matching [DaemonResponse] so a + * client can correlate answers; uniqueness is a convention the daemon does not enforce. + */ +sealed interface DaemonRequest { + val id: Long +} + +/** + * Opens a session and fixes everything that stays constant for it. Every path arrives absolute. + * + * @property id request id, echoed on the response; see [DaemonRequest.id]. + * @property projectRoot the user project root; identifies the session and is never written to. + * @property classpath compile classpath jars, snapshotted once here so later compiles skip + * per-build classpath re-verification. + * @property outDir daemon-owned work directory: classes, dex, IC caches, aapt2 output. + * @property aapt2 the aapt2 binary on device; required, as the daemon never guesses a tool path. + * @property d8Jar build-tools' r8.jar, loaded reflectively so the daemon needs no AGP/r8 build + * dependency; required, like [aapt2]. + * @property androidJar the platform android.jar; required, like [aapt2]. + * @property minApi min API level for d8; 30 is the quick-build floor. + * @property compilerPlugins Kotlin compiler plugin jars passed as `-Xplugin=` to every compile of + * the session, e.g. the Compose compiler plugin. + */ +data class ConfigureRequest( + override val id: Long, + val projectRoot: String, + val classpath: List, + val outDir: String, + val aapt2: String? = null, + val d8Jar: String? = null, + val androidJar: String? = null, + val minApi: Int = DEFAULT_MIN_API, + val compilerPlugins: List = emptyList(), +) : DaemonRequest { + companion object { + /** Default [minApi]: the lowest API level quick build supports. */ + const val DEFAULT_MIN_API = 30 + } +} + +/** + * Compiles the project incrementally. + * + * @property id request id, echoed on the response; see [DaemonRequest.id]. + * @property allSources the full source set, which the IC engine needs on every build. + * @property changedFiles sources edited since the last build, driving `SourcesChanges.Known`; + * CoGo repeats all of [allSources] here on a session's first build to seed the IC caches. + * @property removedFiles sources deleted since the last build; Kotlin removals feed + * `SourcesChanges.Known`, Java removals have their stale `.class` deleted explicitly. + */ +data class CompileRequest( + override val id: Long, + val allSources: List, + val changedFiles: List, + val removedFiles: List = emptyList(), +) : DaemonRequest + +/** + * Dexes the given classes directories into a single classes.dex. + * + * @property id request id, echoed on the response; see [DaemonRequest.id]. + * @property classesDirs absolute class-tree roots to dex, merged into one output in the order + * given; every `.class` under each is re-dexed, changed or not. + */ +data class DexRequest( + override val id: Long, + val classesDirs: List, +) : DaemonRequest + +/** + * Recompiles and relinks resources; the response carries the extracted resources.arsc. + * + * @property id request id, echoed on the response; see [DaemonRequest.id]. + * @property resDirs the project's own `res/` roots to recompile, absolute. Library resources are + * not walked here - they arrive pre-compiled via [libraryResources]. + * @property manifest absolute path to the merged AndroidManifest.xml the relink links against. + * @property stableIds AGP's `stableIds.txt` from the proxy app build, passed to `aapt2 link + * --stable-ids`; optional, but omitting it lets aapt2's unpinned type ids shift out from under + * the baseline manifest (see Aapt2Link). + * @property libraryResources pre-compiled `.flat` units from the proxy app build, passed as `-R` + * overlays so a library-provided reference still resolves; optional, and omitting it relinks + * against the project's res/ alone. + */ +data class RelinkRequest( + override val id: Long, + val resDirs: List, + val manifest: String, + val stableIds: String? = null, + val libraryResources: List = emptyList(), +) : DaemonRequest + +/** + * Liveness check; the response stamps the protocol version. + * + * @property id request id, echoed on the response; see [DaemonRequest.id]. + */ +data class PingRequest( + override val id: Long, +) : DaemonRequest + +/** + * Ends the session and stops the daemon process. + * + * @property id request id, echoed on the response; see [DaemonRequest.id]. + */ +data class ShutdownRequest( + override val id: Long, +) : DaemonRequest + +/** + * One compiler or linker message in the protocol shape. Only ERROR and WARNING travel here; + * anything a tool reports below warning stays on stderr. + * + * @property severity ERROR or WARNING; a response carrying any ERROR is a failed op. + * @property message the tool's text, verbatim and possibly multi-line. + * @property file absolute path the message points at, or null when the tool reported no location + * (a whole-invocation failure such as a bad aapt2 argument). + * @property line 1-based line in [file], or null when the tool gave none. + * @property column 1-based column in [line], or null when the tool gave none. + */ +data class Diagnostic( + val severity: Severity, + val message: String, + val file: String? = null, + val line: Int? = null, + val column: Int? = null, +) { + /** How bad a [Diagnostic] is; the only two levels the wire format carries. */ + enum class Severity { ERROR, WARNING } +} + +/** + * The `op` values the daemon dispatches on, one per [DaemonRequest] type. + * + * Named here rather than spelled on each side because the client writes the value and the daemon + * matches it: an unrecognised op is a runtime "unknown op" rejection, never a compile error. + */ +object DaemonOps { + /** [ConfigureRequest]. */ + const val CONFIGURE = "configure" + + /** [CompileRequest]. */ + const val COMPILE = "compile" + + /** [DexRequest]. */ + const val DEX = "dex" + + /** [RelinkRequest]. */ + const val RELINK = "relink" + + /** [PingRequest]. */ + const val PING = "ping" + + /** [ShutdownRequest]. */ + const val SHUTDOWN = "shutdown" +} + +/** + * The field names of every request line, one constant per property of the [DaemonRequest] types. + * + * The wire is untyped JSON, so nothing links the name the client writes to the name the daemon + * reads: renaming one side alone compiles clean on both and fails only when a real build runs - + * a required field then reads as missing and the op is rejected as malformed. These constants are + * the link. Change a name here and both ends move together. + */ +object RequestKeys { + /** Request id, echoed on the response as [ResponseKeys.ID]; see [DaemonRequest.id]. */ + const val ID = "id" + + /** Which op this line is; one of [DaemonOps]. */ + const val OP = "op" + + /** [ConfigureRequest.projectRoot]. */ + const val PROJECT_ROOT = "projectRoot" + + /** [ConfigureRequest.classpath]. */ + const val CLASSPATH = "classpath" + + /** [ConfigureRequest.outDir]. */ + const val OUT_DIR = "outDir" + + /** [ConfigureRequest.aapt2]. */ + const val AAPT2 = "aapt2" + + /** [ConfigureRequest.d8Jar]. */ + const val D8_JAR = "d8Jar" + + /** [ConfigureRequest.androidJar]. */ + const val ANDROID_JAR = "androidJar" + + /** [ConfigureRequest.minApi]. */ + const val MIN_API = "minApi" + + /** [ConfigureRequest.compilerPlugins]. */ + const val COMPILER_PLUGINS = "compilerPlugins" + + /** [CompileRequest.allSources]. */ + const val ALL_SOURCES = "allSources" + + /** [CompileRequest.changedFiles]. */ + const val CHANGED_FILES = "changedFiles" + + /** [CompileRequest.removedFiles]. */ + const val REMOVED_FILES = "removedFiles" + + /** [DexRequest.classesDirs]. */ + const val CLASSES_DIRS = "classesDirs" + + /** [RelinkRequest.resDirs]. */ + const val RES_DIRS = "resDirs" + + /** [RelinkRequest.manifest]. */ + const val MANIFEST = "manifest" + + /** [RelinkRequest.stableIds]. */ + const val STABLE_IDS = "stableIds" + + /** [RelinkRequest.libraryResources]. */ + const val LIBRARY_RESOURCES = "libraryResources" +} + +/** + * Response field names that do not belong to a stats group: the envelope, and the op-specific + * scalars of [DaemonResponse.values]. + * + * Same reason as [RequestKeys]: the daemon writes these and the client reads them with no type + * between the two. A renamed output-path key such as [CLASSES_DIR] simply reads back absent, which + * the client's mandatory-key rule turns into a failed build rather than a stale deploy. + */ +object ResponseKeys { + /** The [DaemonRequest.id] being answered - the same field the request carried. */ + const val ID = RequestKeys.ID + + /** [DaemonResponse.ok]. */ + const val OK = "ok" + + /** [DaemonResponse.diagnostics]; absent rather than empty when there are none. */ + const val DIAGNOSTICS = "diagnostics" + + /** Stamped into `ping`/`configure` success; see [DaemonResponse.PROTOCOL_VERSION]. */ + const val PROTOCOL_VERSION = "protocolVersion" + + /** + * Filesystem type of the daemon's work directory as `configure` observed it (`ext4`, `f2fs`, + * `fuse`, ...). Reported because it is the strongest predictor of on-device build time - a + * class tree copies ~50x slower on FUSE-backed emulated storage than on the app's own + * filesystem `[measured on a56]` - so a timing row cannot be interpreted without it. + */ + const val SCRATCH_FS_TYPE = "scratchFsType" + + /** How long the op took inside the daemon; reported by every op. */ + const val DURATION_MILLIS = "durationMillis" + + /** `compile`: the class-output tree root. */ + const val CLASSES_DIR = "classesDir" + + /** `compile`: relative `.class` paths this run emitted, for the deploy policy to intersect. */ + const val CLASSES_CHANGED = "classesChanged" + + /** `compile`: the Kotlin half. */ + const val KOTLIN_MILLIS = "kotlinMillis" + + /** `compile`: the Java half. */ + const val JAVA_MILLIS = "javaMillis" + + /** `dex`: the produced classes.dex. */ + const val DEX_FILE = "dexFile" + + /** `dex`: stripping the class tree down to what d8 is fed. */ + const val STRIP_MILLIS = "stripMillis" + + /** `dex`: dexing that stripped tree. */ + const val D8_MILLIS = "d8Millis" + + /** + * `relink`: the relinked resource apk. The name says `arsc` for protocol stability, but the + * payload is the full apk (resources.arsc plus every compiled resource file) - see Aapt2Link. + */ + const val RESOURCES_ARSC = "resourcesArsc" + + /** `relink`: compiling the changed resources. */ + const val AAPT2_COMPILE_MILLIS = "aapt2CompileMillis" + + /** `relink`: relinking the resource table. */ + const val AAPT2_LINK_MILLIS = "aapt2LinkMillis" + + /** Field names inside one entry of the [DIAGNOSTICS] array; one per [Diagnostic] property. */ + object Diagnostics { + /** [Diagnostic.severity], as the enum's `name`. */ + const val SEVERITY = "severity" + + /** [Diagnostic.message]. */ + const val MESSAGE = "message" + + /** [Diagnostic.file]; omitted when the tool reported no location. */ + const val FILE = "file" + + /** [Diagnostic.line]; omitted when the tool gave none. */ + const val LINE = "line" + + /** [Diagnostic.column]; omitted when the tool gave none. */ + const val COLUMN = "column" + } +} + +/** + * The phase counters a `compile` op measures beyond `kotlinMillis` / `javaMillis`. + * + * Those two cover only about half of a warm edit; the rest is the output-tree snapshots, the + * Java-ABI re-parse and the source I/O around them - so without these fields javac reads like the + * bottleneck, when it is 19-27% of a warm edit `[measured on a56]`. Every field is a counter or a + * duration, never derived from a path, a name or source content. + * + * @property preSnapMillis walking the output tree before the compile, to diff against. + * @property postSnapMillis the same walk after, which yields the changed-class set. + * @property javaAbiSnapMillis re-parsing every `.java` source's declarations to decide whether a + * Java ABI moved, which forces a full Kotlin recompile. + * @property allSources size of the source set handed to the compiler. + * @property kotlinToCompile Kotlin sources the daemon DECLARED changed to the Kotlin engine, + * which is not the same as the number recompiled: the engine widens that set from its own + * dependency graph and can recompile files we declared nothing about. Read it as the size of + * the dirty set we handed over - an ABI-changing Java edit hands over all of them. + * @property javaSources `.java` sources, all of which javac recompiles every build. + * @property changedClasses `.class` files this build emitted or rewrote. + * @property compileOrdinal 1-based index of this compile within the session, where `1` is the cold + * build that seeds the caches and pays kotlinc's warm-up, so reading it as a warm edit badly + * overstates per-edit cost. + */ +data class CompileStats( + val preSnapMillis: Long = 0, + val postSnapMillis: Long = 0, + val javaAbiSnapMillis: Long = 0, + val allSources: Int = 0, + val kotlinToCompile: Int = 0, + val javaSources: Int = 0, + val changedClasses: Int = 0, + val compileOrdinal: Long = 0, +) { + /** + * Flattens the stats into response values, keyed by the constants below. + * + * @return every field, one entry per `KEY_*` constant, ready to merge into + * [DaemonResponse.values]; zero-valued fields are emitted too, so telling an unmeasured field + * from a measured zero is [fromValues]'s job. + */ + fun toValues(): Map = + mapOf( + KEY_PRE_SNAP_MILLIS to preSnapMillis, + KEY_POST_SNAP_MILLIS to postSnapMillis, + KEY_JAVA_ABI_SNAP_MILLIS to javaAbiSnapMillis, + KEY_ALL_SOURCES to allSources, + KEY_KOTLIN_TO_COMPILE to kotlinToCompile, + KEY_JAVA_SOURCES to javaSources, + KEY_CHANGED_CLASSES to changedClasses, + KEY_COMPILE_ORDINAL to compileOrdinal, + ) + + companion object { + const val KEY_PRE_SNAP_MILLIS = "preSnapMillis" + const val KEY_POST_SNAP_MILLIS = "postSnapMillis" + const val KEY_JAVA_ABI_SNAP_MILLIS = "javaAbiSnapMillis" + const val KEY_ALL_SOURCES = "nAllSources" + const val KEY_KOTLIN_TO_COMPILE = "nKotlinToCompile" + const val KEY_JAVA_SOURCES = "nJavaSources" + const val KEY_CHANGED_CLASSES = "nChangedClasses" + const val KEY_COMPILE_ORDINAL = "compileOrdinal" + + private val KEYS = + listOf( + KEY_PRE_SNAP_MILLIS, + KEY_POST_SNAP_MILLIS, + KEY_JAVA_ABI_SNAP_MILLIS, + KEY_ALL_SOURCES, + KEY_KOTLIN_TO_COMPILE, + KEY_JAVA_SOURCES, + KEY_CHANGED_CLASSES, + KEY_COMPILE_ORDINAL, + ) + + /** + * Reads the stats back out of a response, or null if it carries none of them. + * + * The null return matters: a daemon predating these fields must not yield a zero-filled + * row, which would read as "measured, and it was free". A single missing key does + * default to 0, so a future daemon may drop one. + * + * @param lookup reads one response value by key as a Long, returning null when the key is + * absent; the caller owns the numeric coercion from whatever the JSON carried. + * @return the stats, or null if the response carries none of the `KEY_*` keys. + */ + fun fromValues(lookup: (String) -> Long?): CompileStats? { + if (KEYS.none { lookup(it) != null }) return null + return CompileStats( + preSnapMillis = lookup(KEY_PRE_SNAP_MILLIS) ?: 0, + postSnapMillis = lookup(KEY_POST_SNAP_MILLIS) ?: 0, + javaAbiSnapMillis = lookup(KEY_JAVA_ABI_SNAP_MILLIS) ?: 0, + allSources = lookup(KEY_ALL_SOURCES)?.toInt() ?: 0, + kotlinToCompile = lookup(KEY_KOTLIN_TO_COMPILE)?.toInt() ?: 0, + javaSources = lookup(KEY_JAVA_SOURCES)?.toInt() ?: 0, + changedClasses = lookup(KEY_CHANGED_CLASSES)?.toInt() ?: 0, + compileOrdinal = lookup(KEY_COMPILE_ORDINAL) ?: 0, + ) + } + } +} + +/** + * What a `dex` op processed. The step rewrites and re-dexes the whole class tree every build, + * changed or not, so its cost scales with these two numbers rather than the changed-file count. + * + * @property classFiles `.class` files read, stripped and dexed. + * @property classBytes their total size in bytes. + */ +data class DexStats( + val classFiles: Int = 0, + val classBytes: Long = 0, +) { + /** + * Flattens the stats into response values, keyed by the constants below. + * + * @return both fields, keyed by [KEY_CLASS_FILES] and [KEY_CLASS_BYTES], ready to merge into + * [DaemonResponse.values]. + */ + fun toValues(): Map = + mapOf( + KEY_CLASS_FILES to classFiles, + KEY_CLASS_BYTES to classBytes, + ) + + companion object { + const val KEY_CLASS_FILES = "nClassFiles" + const val KEY_CLASS_BYTES = "classBytes" + + /** + * Same absent-vs-zero convention as [CompileStats.fromValues]. + * + * @param lookup reads one response value by key as a Long, returning null when absent. + * @return the stats, or null if the response carries neither key. + */ + fun fromValues(lookup: (String) -> Long?): DexStats? { + val files = lookup(KEY_CLASS_FILES) + val bytes = lookup(KEY_CLASS_BYTES) + if (files == null && bytes == null) return null + return DexStats(classFiles = files?.toInt() ?: 0, classBytes = bytes ?: 0) + } + } +} + +/** + * The daemon's answer to one request. [values] holds the op-specific scalars (`classesDir`, + * `dexFile`, `resourcesArsc`, `durationMillis`, ...), serialized flat into the response object. + * + * Adding a response key must not bump [PROTOCOL_VERSION]: the version is a hard gate that aborts + * the session on mismatch, and a staged daemon jar can lag the client, so bumping it for an + * additive field would break a pairing that would otherwise work. + * + * @property id the [DaemonRequest.id] this answers; the client's only correlation handle. + * @property ok whether the op succeeded, false implying at least one ERROR in [diagnostics]. + * @property values op-specific scalars, flat and JSON-scalar-only, keyed by the `KEY_*` constants + * and [ResponseKeys], so a client may read one key and ignore the rest. + * @property diagnostics compiler and linker messages, present on success too since a build can + * succeed with warnings. + */ +data class DaemonResponse( + val id: Long, + val ok: Boolean, + val values: Map = emptyMap(), + val diagnostics: List = emptyList(), +) { + companion object { + /** + * Wire-protocol version, stamped into `ping` and `configure` success responses so a + * caller can pin it and abort loudly on drift rather than misread a changed wire shape. + */ + const val PROTOCOL_VERSION = 1 + + /** + * Builds a success response, with no diagnostics. + * + * @param id the [DaemonRequest.id] being answered. + * @param values op-specific scalars to return; see [DaemonResponse.values]. + * @return an `ok = true` response carrying [values] and an empty diagnostic list. + */ + fun ok( + id: Long, + values: Map = emptyMap(), + ): DaemonResponse = DaemonResponse(id, true, values) + + /** + * Builds a failure response from already-parsed tool messages. + * + * @param id the [DaemonRequest.id] being answered. + * @param diagnostics the messages to report; the caller is expected to include at least + * one ERROR, since `ok = false` with warnings alone would not explain the failure. + * @return an `ok = false` response with no [values]. + */ + fun failure( + id: Long, + diagnostics: List, + ): DaemonResponse = DaemonResponse(id, false, emptyMap(), diagnostics) + + /** + * Builds a failure response for a whole-op error with no source location. + * + * @param id the [DaemonRequest.id] being answered. + * @param message the error text, wrapped as a single locationless ERROR [Diagnostic]. + * @return an `ok = false` response carrying that one diagnostic. + */ + fun failure( + id: Long, + message: String, + ): DaemonResponse = failure(id, listOf(Diagnostic(Diagnostic.Severity.ERROR, message))) + } +} + +/** Outcome of parsing one stdin line. Malformed input never throws past the codec. */ +sealed interface ParseResult { + /** + * A line that decoded into a request the daemon can dispatch. + * + * @property request the decoded request; its `id` is the one to answer on. + */ + data class Parsed( + val request: DaemonRequest, + ) : ParseResult + + /** + * A line the codec could not turn into a request. + * + * @property id the request id when it could be recovered from the broken input, else + * [UNKNOWN_ID] so the client can still correlate "something failed". + * @property message why the line was rejected, reported back as the failure response's one + * ERROR diagnostic. + */ + data class Malformed( + val id: Long, + val message: String, + ) : ParseResult { + companion object { + /** [id] when the broken line yielded no usable request id. No real request uses it. */ + const val UNKNOWN_ID = -1L + } + } +} diff --git a/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt b/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt new file mode 100644 index 0000000000..3bed586b89 --- /dev/null +++ b/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt @@ -0,0 +1,194 @@ +package org.appdevforall.cotg.quickbuild.protocol + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** + * The wire-contract details of the request/result DTOs that the codec and the client (both + * in other modules) rely on: documented defaults for optional fields (an absent field must + * mean the documented fallback behavior, never an error) and value semantics (a request + * rebuilt from the same wire fields IS the same request - what every codec round-trip + * assertion stands on). + */ +class DaemonProtocolDtoTest { + @Test + fun `configure without optional toolchain paths means self-discovery and the v1 minApi floor`() { + val request = ConfigureRequest(1, "/p", listOf("/a.jar"), "/out") + + // Null here is the "discover from ANDROID_HOME" signal, not an error. + assertThat(request.aapt2).isNull() + assertThat(request.d8Jar).isNull() + assertThat(request.androidJar).isNull() + assertThat(request.minApi).isEqualTo(30) + assertThat(request.minApi).isEqualTo(ConfigureRequest.DEFAULT_MIN_API) + assertThat(request.compilerPlugins).isEmpty() + } + + @Test + fun `configure carries explicit toolchain paths and session inputs verbatim`() { + val request = + ConfigureRequest( + id = 9, + projectRoot = "/projects/demo", + classpath = listOf("/android.jar", "/kotlin-stdlib.jar"), + outDir = "/work", + aapt2 = "/sdk/aapt2", + d8Jar = "/sdk/r8.jar", + androidJar = "/sdk/android.jar", + minApi = 26, + compilerPlugins = listOf("/compose-compiler-plugin.jar"), + ) + + assertThat(request.id).isEqualTo(9) + assertThat(request.projectRoot).isEqualTo("/projects/demo") + assertThat(request.classpath).containsExactly("/android.jar", "/kotlin-stdlib.jar").inOrder() + assertThat(request.outDir).isEqualTo("/work") + assertThat(request.aapt2).isEqualTo("/sdk/aapt2") + assertThat(request.d8Jar).isEqualTo("/sdk/r8.jar") + assertThat(request.androidJar).isEqualTo("/sdk/android.jar") + assertThat(request.minApi).isEqualTo(26) + assertThat(request.compilerPlugins).containsExactly("/compose-compiler-plugin.jar") + } + + @Test + fun `compile defaults removedFiles to empty - the pre-removal-support behavior`() { + val request = CompileRequest(2, listOf("/A.kt", "/B.kt"), listOf("/A.kt")) + + assertThat(request.id).isEqualTo(2) + assertThat(request.allSources).containsExactly("/A.kt", "/B.kt").inOrder() + assertThat(request.changedFiles).containsExactly("/A.kt") + assertThat(request.removedFiles).isEmpty() + } + + @Test + fun `relink defaults stableIds to null and libraryResources to empty - the documented fallbacks`() { + val request = RelinkRequest(4, listOf("/res"), "/AndroidManifest.xml") + + assertThat(request.id).isEqualTo(4) + assertThat(request.resDirs).containsExactly("/res") + assertThat(request.manifest).isEqualTo("/AndroidManifest.xml") + // Null = unpinned relink (pre-Bug-6), empty = project res only (pre-Bug-8): + // documented protocol behavior, not an error. + assertThat(request.stableIds).isNull() + assertThat(request.libraryResources).isEmpty() + } + + @Test + fun `every request exposes its id through DaemonRequest - the correlation contract`() { + // The router and client correlate responses purely by this polymorphic id. + val requests: List = + listOf( + ConfigureRequest(11, "/p", emptyList(), "/out"), + CompileRequest(12, emptyList(), emptyList()), + DexRequest(13, listOf("/classes")), + RelinkRequest(14, listOf("/res"), "/M.xml"), + PingRequest(15), + ShutdownRequest(16), + ) + + assertThat(requests.map { it.id }).containsExactly(11L, 12L, 13L, 14L, 15L, 16L).inOrder() + assertThat((requests[2] as DexRequest).classesDirs).containsExactly("/classes") + } + + @Test + fun `requests rebuilt from the same wire fields are equal - value semantics`() { + // Codec round-trip tests compare a re-parsed request to the original; that only + // proves anything because these are value types, pinned here. + assertThat(ConfigureRequest(1, "/p", listOf("/a.jar"), "/out")) + .isEqualTo(ConfigureRequest(1, "/p", listOf("/a.jar"), "/out")) + assertThat(PingRequest(5)).isEqualTo(PingRequest(5)) + assertThat(ShutdownRequest(5)).isNotEqualTo(ShutdownRequest(6)) + assertThat(DexRequest(3, listOf("/classes"))).isNotEqualTo(DexRequest(3, listOf("/other"))) + } + + @Test + fun `a diagnostic without a location is just a severity and message`() { + val diagnostic = Diagnostic(Diagnostic.Severity.ERROR, "boom") + + assertThat(diagnostic.severity).isEqualTo(Diagnostic.Severity.ERROR) + assertThat(diagnostic.message).isEqualTo("boom") + assertThat(diagnostic.file).isNull() + assertThat(diagnostic.line).isNull() + assertThat(diagnostic.column).isNull() + } + + @Test + fun `parsed wraps the request it recovered`() { + val parsed = ParseResult.Parsed(PingRequest(3)) + + assertThat(parsed.request).isEqualTo(PingRequest(3)) + } + + @Test + fun `malformed keeps the recovered id and the reason - and the unknown id is -1`() { + val malformed = ParseResult.Malformed(7, "missing 'op'") + + assertThat(malformed.id).isEqualTo(7) + assertThat(malformed.message).isEqualTo("missing 'op'") + // -1 is on the wire whenever the id could not be recovered; the client keys its + // "something failed but I don't know what" handling on this exact value. + assertThat(ParseResult.Malformed.UNKNOWN_ID).isEqualTo(-1L) + } + + @Test + fun `a directly constructed response defaults to no values and no diagnostics`() { + val direct = DaemonResponse(4, true) + val helper = DaemonResponse.ok(5) + + assertThat(direct.values).isEmpty() + assertThat(direct.diagnostics).isEmpty() + assertThat(helper.ok).isTrue() + assertThat(helper.values).isEmpty() + assertThat(helper.diagnostics).isEmpty() + } + + @Test + fun `an unmeasured CompileStats serializes every key as zero, not as absent`() { + // The absent-vs-zero convention: a daemon that HAS the stats fields always writes + // all keys (zeros mean "measured, and it was free"); only a daemon predating the + // fields omits them (fromValues then yields null). A default row must therefore + // serialize all-zero, never skip keys. + val values = CompileStats().toValues() + + assertThat(values.keys) + .containsExactly( + CompileStats.KEY_PRE_SNAP_MILLIS, + CompileStats.KEY_POST_SNAP_MILLIS, + CompileStats.KEY_JAVA_ABI_SNAP_MILLIS, + CompileStats.KEY_ALL_SOURCES, + CompileStats.KEY_KOTLIN_TO_COMPILE, + CompileStats.KEY_JAVA_SOURCES, + CompileStats.KEY_CHANGED_CLASSES, + CompileStats.KEY_COMPILE_ORDINAL, + ) + assertThat(values.values.map { (it as Number).toLong() }).containsExactlyElementsIn(LongArray(8).toList()) + } + + @Test + fun `fromValues defaults a missing ordinal to zero when another key is present`() { + // The mirror of the existing ordinal-only test: any single surviving key keeps the + // row alive, and the ORDINAL side of the per-key elvis must also fill with 0. + val stats = + CompileStats.fromValues { key -> + if (key == CompileStats.KEY_PRE_SNAP_MILLIS) 42L else null + } + + assertThat(stats).isEqualTo(CompileStats(preSnapMillis = 42, compileOrdinal = 0)) + } + + @Test + fun `an unmeasured DexStats serializes both keys as zero`() { + assertThat(DexStats().toValues()) + .containsExactly(DexStats.KEY_CLASS_FILES, 0, DexStats.KEY_CLASS_BYTES, 0L) + } + + @Test + fun `DexStats fromValues with only classBytes fills classFiles with zero`() { + val stats = + DexStats.fromValues { key -> + if (key == DexStats.KEY_CLASS_BYTES) 1_234L else null + } + + assertThat(stats).isEqualTo(DexStats(classFiles = 0, classBytes = 1_234)) + } +} diff --git a/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolTest.kt b/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolTest.kt new file mode 100644 index 0000000000..146c80b96a --- /dev/null +++ b/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolTest.kt @@ -0,0 +1,101 @@ +package org.appdevforall.cotg.quickbuild.protocol + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** The wire-model logic: stats round-trips, response helpers, malformed-input ids. */ +class DaemonProtocolTest { + @Test + fun `CompileStats round-trips through toValues and fromValues`() { + val stats = + CompileStats( + preSnapMillis = 11, + postSnapMillis = 22, + javaAbiSnapMillis = 33, + allSources = 40, + kotlinToCompile = 3, + javaSources = 2, + changedClasses = 5, + compileOrdinal = 7, + ) + + val values = stats.toValues() + val restored = CompileStats.fromValues { key -> (values[key] as? Number)?.toLong() } + + assertThat(restored).isEqualTo(stats) + } + + @Test + fun `CompileStats fromValues is null when no key is present`() { + // A response from a daemon predating the stats fields must read back as + // "not measured", not as a zero-filled row that reads as "measured, free". + assertThat(CompileStats.fromValues { null }).isNull() + } + + @Test + fun `CompileStats fromValues defaults an individually missing key to zero`() { + val restored = + CompileStats.fromValues { key -> + if (key == CompileStats.KEY_COMPILE_ORDINAL) 3L else null + } + + assertThat(restored).isEqualTo(CompileStats(compileOrdinal = 3)) + } + + @Test + fun `DexStats round-trips through toValues and fromValues`() { + val stats = DexStats(classFiles = 464, classBytes = 1_234_567) + + val values = stats.toValues() + val restored = DexStats.fromValues { key -> (values[key] as? Number)?.toLong() } + + assertThat(restored).isEqualTo(stats) + } + + @Test + fun `DexStats fromValues is null only when both keys are absent`() { + assertThat(DexStats.fromValues { null }).isNull() + + val filesOnly = + DexStats.fromValues { key -> + if (key == DexStats.KEY_CLASS_FILES) 9L else null + } + + assertThat(filesOnly).isEqualTo(DexStats(classFiles = 9, classBytes = 0)) + } + + @Test + fun `ok builds a success response carrying the values and no diagnostics`() { + val response = DaemonResponse.ok(5, mapOf("classesDir" to "/out/classes")) + + assertThat(response.id).isEqualTo(5) + assertThat(response.ok).isTrue() + assertThat(response.values).containsExactly("classesDir", "/out/classes") + assertThat(response.diagnostics).isEmpty() + } + + @Test + fun `failure from a message wraps it as a single ERROR diagnostic`() { + val response = DaemonResponse.failure(9, "aapt2 exited 1") + + assertThat(response.id).isEqualTo(9) + assertThat(response.ok).isFalse() + assertThat(response.values).isEmpty() + assertThat(response.diagnostics) + .containsExactly(Diagnostic(Diagnostic.Severity.ERROR, "aapt2 exited 1")) + } + + @Test + fun `failure from diagnostics keeps them verbatim`() { + val diagnostics = + listOf( + Diagnostic(Diagnostic.Severity.ERROR, "unresolved reference", file = "A.kt", line = 3, column = 7), + Diagnostic(Diagnostic.Severity.WARNING, "unused variable"), + ) + + val response = DaemonResponse.failure(2, diagnostics) + + assertThat(response.ok).isFalse() + assertThat(response.diagnostics).isEqualTo(diagnostics) + } +} diff --git a/quickbuild/protocol/src/testFixtures/kotlin/org/appdevforall/cotg/quickbuild/testfixtures/OfflineGuard.kt b/quickbuild/protocol/src/testFixtures/kotlin/org/appdevforall/cotg/quickbuild/testfixtures/OfflineGuard.kt new file mode 100644 index 0000000000..ea505e3fbb --- /dev/null +++ b/quickbuild/protocol/src/testFixtures/kotlin/org/appdevforall/cotg/quickbuild/testfixtures/OfflineGuard.kt @@ -0,0 +1,113 @@ +package org.appdevforall.cotg.quickbuild.testfixtures + +import java.io.File + +/** + * Scanner behind the Quick Build offline guard tests. Locates the calling module's build + * dir from that test's own code source (no hardcoded absolute paths), enumerates + * production class dirs (main / non-test variants, JVM and Android layouts), and searches + * raw `.class` bytes for banned network-API constants. + * + * It lives in `:quickbuild:protocol` because that is the only module every guarded module + * already depends on; the scanner itself has nothing to do with the wire protocol. Each + * module keeps its own guard test -- the allowed exceptions differ per module and belong + * next to the assertions that encode them. + * + * The max-line-length suppression below is for a phantom lint: no line here + * exceeds 140, but ktlint under spotless reports L1 max-line-length on this file + * regardless (formatter interaction bug) -- suppressed narrowly, not repo-wide. + */ +@Suppress("ktlint:standard:max-line-length") +object OfflineGuard { + /** Constant-pool / UTF8 substrings of network APIs that must never appear. */ + val BANNED: List = + listOf( + "java/net/Socket", + "java/net/ServerSocket", + "java/net/HttpURLConnection", + "java/net/InetAddress", + "javax/net/ssl", + "okhttp3/", + "java/nio/channels/SocketChannel", + "android/net/ConnectivityManager", + ) + + /** + * Walks up from the test's code-source location to the module `build` dir. An optional + * `quickbuild.offlineGuard.buildDir` system property overrides it for unusual layouts. + */ + fun moduleBuildDir(fromClass: Class<*>): File { + System.getProperty("quickbuild.offlineGuard.buildDir")?.let { return File(it) } + val location = + File( + fromClass.protectionDomain.codeSource.location + .toURI(), + ) + var dir: File? = location + while (dir != null && dir.name != "build") dir = dir.parentFile + requireNotNull(dir) { "could not locate module build dir from test location $location" } + return dir + } + + fun productionClassFiles(buildDir: File): List = + buildDir + .walkTopDown() + .filter { it.isFile && it.extension == "class" } + .filter { isProductionClassPath(it.relativeTo(buildDir).invariantSeparatorsPath.split("/")) } + .toList() + + fun scanForBannedReferences( + buildDir: File, + classFiles: List, + ): List { + val violations = mutableListOf() + for (file in classFiles) { + val bytes = file.readBytes() + val rel = file.relativeTo(buildDir).invariantSeparatorsPath + for (banned in BANNED) { + if (containsAscii(bytes, banned)) violations += "$rel references $banned" + } + } + return violations + } + + private fun isProductionClassPath(segments: List): Boolean { + if (segments.contains(".cache")) return false // jacoco/expanded-zip agent classes + return when { + // JVM module: build/classes/{kotlin,java}/main/... + segments.size >= 3 && segments[0] == "classes" && segments[2] == "main" -> { + true + } + + // Android Kotlin: build/tmp/kotlin-classes//... + segments.size >= 3 && segments[0] == "tmp" && segments[1] == "kotlin-classes" -> { + !isTestVariant(segments[2]) + } + + // Android Java: build/intermediates/javac//.../classes/... + segments.size >= 3 && segments[0] == "intermediates" && segments[1] == "javac" -> { + !isTestVariant(segments[2]) + } + + else -> { + false + } + } + } + + private fun isTestVariant(variant: String): Boolean = + variant == "test" || variant.endsWith("UnitTest") || variant.endsWith("AndroidTest") + + fun containsAscii( + haystack: ByteArray, + needle: String, + ): Boolean { + val pattern = needle.toByteArray(Charsets.US_ASCII) + if (pattern.isEmpty() || haystack.size < pattern.size) return pattern.isEmpty() + outer@ for (i in 0..haystack.size - pattern.size) { + for (j in pattern.indices) if (haystack[i + j] != pattern[j]) continue@outer + return true + } + return false + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 7ce1b50938..c80d9f78fc 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -139,6 +139,7 @@ include( ":lsp:kotlin", ":lsp:xml", ":profiler", + ":quickbuild:protocol", ":subprojects:aapt2-proto", ":subprojects:aaptcompiler", ":subprojects:builder-model-impl", From a94afb013dafd61069d5410c4713914660deb20c Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 21 Aug 2026 22:40:05 -0700 Subject: [PATCH 2/3] =?UTF-8?q?ADFA-4128:=20qb=2003=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20align=20tool-path=20DTO=20test=20with=20contract?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding (Important, pr03-review.md): DaemonProtocolDtoTest's first test was named "configure without optional toolchain paths means self-discovery" with a comment calling null the "discover from ANDROID_HOME" signal — the opposite of the contract in the same commit (DaemonProtocol.kt KDoc: "required, as the daemon never guesses a tool path"; README: configure answers ok:false with one diagnostic per missing field) and of the daemon's actual behavior at stack tip (DaemonService.configure rejects null/blank aapt2/d8Jar/androidJar). Fix: renamed the test and rewrote the comment so null reads as "not supplied, and configure rejects it", per the documented contract. The assertions were already correct (null IS the DTO default) and are unchanged; the rejection behavior itself is asserted in the daemon module's DaemonServiceTest at stack tip, so no rejection assertion is duplicated here. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W --- .../cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt b/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt index 3bed586b89..f7122f5078 100644 --- a/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt +++ b/quickbuild/protocol/src/test/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocolDtoTest.kt @@ -12,10 +12,11 @@ import org.junit.jupiter.api.Test */ class DaemonProtocolDtoTest { @Test - fun `configure without optional toolchain paths means self-discovery and the v1 minApi floor`() { + fun `configure defaults unsupplied toolchain paths to null - which the daemon rejects - and the v1 minApi floor`() { val request = ConfigureRequest(1, "/p", listOf("/a.jar"), "/out") - // Null here is the "discover from ANDROID_HOME" signal, not an error. + // Null means "not supplied": the daemon never discovers tool paths, so configure + // answers ok:false with one diagnostic per null/blank path (see the KDoc and README). assertThat(request.aapt2).isNull() assertThat(request.d8Jar).isNull() assertThat(request.androidJar).isNull() From e48995abb69c849d2b3575395390ec918efcad51 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Wed, 26 Aug 2026 23:43:44 -0700 Subject: [PATCH 3/3] ADFA-4128 (3/11): address CodeRabbit review - F1715-1 stop the response contract claiming values are never arrays Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FstXxJ5cwWPcvmhZ9vJgJ7 --- quickbuild/protocol/README.md | 3 ++- .../cotg/quickbuild/protocol/DaemonProtocol.kt | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/quickbuild/protocol/README.md b/quickbuild/protocol/README.md index 31a6457618..02571e8a9c 100644 --- a/quickbuild/protocol/README.md +++ b/quickbuild/protocol/README.md @@ -57,7 +57,8 @@ Six ops: `configure`, `compile`, `dex`, `relink`, `ping`, `shutdown`. ### Responses -`{"id", "ok", , "diagnostics"}`. Values are flat scalars, never nested. +`{"id", "ok", , "diagnostics"}`. Values are flat - never a nested object. +They are JSON scalars apart from `classesChanged`, which is an array of strings. - `diagnostics` **can appear on success** - a build can succeed with warnings. - **`line` and `column` are JSON numbers, and they stop here.** CoGo keeps error positions for diff --git a/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt b/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt index 3058cf3423..6e31ddde73 100644 --- a/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt +++ b/quickbuild/protocol/src/main/kotlin/org/appdevforall/cotg/quickbuild/protocol/DaemonProtocol.kt @@ -456,8 +456,9 @@ data class DexStats( } /** - * The daemon's answer to one request. [values] holds the op-specific scalars (`classesDir`, - * `dexFile`, `resourcesArsc`, `durationMillis`, ...), serialized flat into the response object. + * The daemon's answer to one request. [values] holds the op-specific values (`classesDir`, + * `dexFile`, `resourcesArsc`, `durationMillis`, ...), serialized flat into the response object - + * never a nested object. `classesChanged` is the one array. * * Adding a response key must not bump [PROTOCOL_VERSION]: the version is a hard gate that aborts * the session on mismatch, and a staged daemon jar can lag the client, so bumping it for an @@ -465,8 +466,9 @@ data class DexStats( * * @property id the [DaemonRequest.id] this answers; the client's only correlation handle. * @property ok whether the op succeeded, false implying at least one ERROR in [diagnostics]. - * @property values op-specific scalars, flat and JSON-scalar-only, keyed by the `KEY_*` constants - * and [ResponseKeys], so a client may read one key and ignore the rest. + * @property values op-specific values, flat - never a nested object - and JSON scalars apart + * from `classesChanged`, which is an array of strings. Keyed by the `KEY_*` constants and + * [ResponseKeys], so a client may read one key and ignore the rest. * @property diagnostics compiler and linker messages, present on success too since a build can * succeed with warnings. */