Enforce :sharingan/:sharingan-noop public API parity in CI (#12)

.github/workflows/api-check.yml

@@ -1,9 +1,14 @@
 name: Public API check
 
 # Guards the committed public-API dumps (sharingan/api, sharingan-noop/api).
-# apiCheck fails the build when the public surface drifts from the dumps, so an
-# accidental signature change cannot slip in between releases — the "swap
-# sharingan-noop in release" safety story depends on the API staying stable.
+#
+# Two complementary gates live here:
+#   - api-check  : apiCheck fails when a module's surface drifts from its own
+#     committed dump (an accidental signature change between releases).
+#   - api-parity : checkApiParity fails when :sharingan and :sharingan-noop drift
+#     from EACH OTHER, i.e. they no longer expose an identical public contract
+#     (issue #12). This is what makes the "swap sharingan-noop in release" safety
+#     story enforceable rather than dependent on human discipline.
 # Regenerate intentional changes locally with `./gradlew apiDump` and commit.
 on:
   pull_request:
@@ -28,3 +33,21 @@ jobs:
 
       - name: Check public API against committed dumps
         run: ./gradlew apiCheck
+
+  api-parity:
+    # Cross-module parity is a pure text comparison of the four committed dumps —
+    # no Android/iOS SDK, no compilation, no Kotlin/Native link — so it runs on
+    # cheap ubuntu for fast, independent feedback (it does NOT need the macOS host
+    # the api-check job above requires).
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v6
+
+      - name: Set up JDK 17
+        uses: actions/setup-java@v5
+        with:
+          distribution: temurin
+          java-version: "17"
+
+      - name: Assert :sharingan and :sharingan-noop expose an identical public contract
+        run: ./gradlew checkApiParity

README.md

@@ -97,6 +97,13 @@ git clone https://github.com/mibrahimdev/Sharingan && cd Sharingan
 ./gradlew publishToMavenLocal
 ```
 
+Contributors: `:sharingan` and `:sharingan-noop` must expose an **identical
+public API** so the debug→release swap is safe. `./gradlew checkApiParity` (run in
+CI) enforces this and fails on any drift — see
+[`docs/api-parity.md`](docs/api-parity.md) for the contract and why a few debug-only
+symbols are excluded. After an intentional API change, run `./gradlew apiDump` and
+commit the regenerated dumps.
+
 Maintainers: cutting a Maven Central release is a two-step,
 stage-then-manually-release flow — see [`docs/RELEASING.md`](docs/RELEASING.md).
 

build.gradle.kts

@@ -24,3 +24,173 @@ apiValidation {
         enabled = true
     }
 }
+
+// ---------------------------------------------------------------------------
+// Cross-module API-parity gate (issue #12).
+//
+// The "swap :sharingan-noop in release" safety story depends on the two modules
+// exposing a SIGNATURE-FOR-SIGNATURE identical public CONTRACT. They are compiled
+// independently, so the only thing keeping them in lockstep is human discipline:
+// a drifted default or a forgotten method would surface only as a *consumer's
+// release-build compile error* — the worst place to find it.
+//
+// BCV (#11) already dumps each module's full public surface to api/*.api (JVM) and
+// api/*.klib.api (native/iOS). This task compares those committed dumps and FAILS
+// the build on any divergence. It is pure text processing over the four files —
+// no compilation, no iOS link — so it runs in seconds and needs no Android/iOS SDK.
+//
+// It is NOT a whole-file diff: :sharingan legitimately carries debug-only symbols
+// the noop must not (and cannot) expose. Those are filtered out before comparing —
+// see the local helpers and docs/api-parity.md for the rationale behind each
+// exclusion. (The helpers are local to `doLast` on purpose: Gradle Kotlin DSL drops
+// top-level statements placed after top-level `fun` declarations, so the comparison
+// logic lives inside the task action rather than as script-level functions.)
+// ---------------------------------------------------------------------------
+
+tasks.register("checkApiParity") {
+    group = "verification"
+    description =
+        "Asserts :sharingan and :sharingan-noop expose an identical public API contract (issue #12)."
+
+    val realApi = layout.projectDirectory.file("sharingan/api/sharingan.api").asFile
+    val noopApi = layout.projectDirectory.file("sharingan-noop/api/sharingan-noop.api").asFile
+    val realKlib = layout.projectDirectory.file("sharingan/api/sharingan.klib.api").asFile
+    val noopKlib = layout.projectDirectory.file("sharingan-noop/api/sharingan-noop.klib.api").asFile
+
+    // Re-run only when one of the committed dumps changes.
+    inputs.files(realApi, noopApi, realKlib, noopKlib)
+
+    doLast {
+        // Debug-only JVM classes the real module ships but a consumer never references
+        // directly, so :sharingan-noop correctly omits them. Excluded from parity:
+        //  - dev/sharingan/ui/**             : the in-app Compose viewer screens
+        //  - dev/sharingan/internal/**       : init ContentProvider + notification receiver
+        //  - dev/sharingan/SharinganActivity : the Android viewer Activity
+        //  - any ComposableSingletons$*      : Compose-compiler lambda synthetics
+        fun isExcludedJvmClass(name: String): Boolean =
+            name.startsWith("dev/sharingan/ui/") ||
+                name.startsWith("dev/sharingan/internal/") ||
+                name == "dev/sharingan/SharinganActivity" ||
+                name.contains("ComposableSingletons\$")
+
+        // Reduce a JVM `.api` dump to the consumer-facing contract as a set of
+        // qualified signatures. Drops the excluded classes wholesale and strips the
+        // Compose-compiler `public static final field $stable I` line that only the
+        // real module carries (Compose ships only in :sharingan). Each surviving
+        // member is keyed by its enclosing class so an identical signature in a
+        // different class cannot mask a divergence.
+        fun jvmContractEntries(apiFile: File): Set<String> {
+            val lines = apiFile.readLines()
+            val entries = linkedSetOf<String>()
+            var i = 0
+            while (i < lines.size) {
+                val line = lines[i]
+                // Class/interface headers sit at column 0; members are tab-indented
+                // and the block closes with a lone "}". Skip non-header lines.
+                if (line.isBlank() || line.first().isWhitespace() || line == "}") {
+                    i++
+                    continue
+                }
+                val className =
+                    if (" class " in line) line.substringAfter(" class ").substringBefore(" ") else ""
+                val block = mutableListOf(line)
+                var j = i + 1
+                while (j < lines.size && lines[j] != "}") {
+                    block.add(lines[j])
+                    j++
+                }
+                i = j + 1
+                if (className.isEmpty() || isExcludedJvmClass(className)) continue
+                // Keep the header verbatim (sans trailing " {") so a changed supertype is caught.
+                entries.add("CLASS " + line.removeSuffix(" {").trim())
+                for (member in block.drop(1)) {
+                    val m = member.trim()
+                    if (m.isEmpty() || "\$stable" in m) continue
+                    entries.add("$className :: $m")
+                }
+            }
+            return entries
+        }
+
+        // Reduce a native `.klib.api` dump to the consumer-facing contract. Each BCV
+        // declaration carries a globally-unique signature-id trailing comment, so the
+        // trimmed line is unique and used as-is. Dropped:
+        //  - the `//` header block, incl. `// Library unique name: <…:sharingan[-noop]>`
+        //    which differs by module name and would always false-fail a raw diff;
+        //  - `$stableprop` / `$stableprop_getter`: the native equivalent of the JVM
+        //    `$stable` field, present only in the Compose-carrying real module;
+        //  - `dev.sharingan.ui/SharinganScreen`: the top-level UI composable (ui/**).
+        fun nativeContractEntries(klibFile: File): Set<String> {
+            val entries = linkedSetOf<String>()
+            for (raw in klibFile.readLines()) {
+                val t = raw.trim()
+                if (t.isEmpty() || t == "}" || t.startsWith("//")) continue
+                if ("\$stableprop" in raw || "dev.sharingan.ui/" in raw) continue
+                entries.add(t)
+            }
+            return entries
+        }
+
+        // Human-readable mismatch report for one surface, or null when the two
+        // contracts are identical. Compared both directions so a symbol the noop is
+        // MISSING and a symbol it EXPOSES EXTRA are both reported.
+        fun diffSurface(
+            surface: String,
+            realPath: String,
+            noopPath: String,
+            realEntries: Set<String>,
+            noopEntries: Set<String>,
+        ): String? {
+            val missingFromNoop = (realEntries - noopEntries).sorted()
+            val extraInNoop = (noopEntries - realEntries).sorted()
+            if (missingFromNoop.isEmpty() && extraInNoop.isEmpty()) return null
+            return buildString {
+                appendLine("API PARITY MISMATCH on the $surface surface")
+                appendLine("  :sharingan      dump: $realPath")
+                appendLine("  :sharingan-noop dump: $noopPath")
+                if (missingFromNoop.isNotEmpty()) {
+                    appendLine("  In :sharingan but MISSING from :sharingan-noop")
+                    appendLine("  (consumer code compiled against debug would FAIL to compile after a release swap):")
+                    missingFromNoop.forEach { appendLine("    - $it") }
+                }
+                if (extraInNoop.isNotEmpty()) {
+                    appendLine("  In :sharingan-noop but NOT in :sharingan")
+                    appendLine("  (the noop over-exposes surface the real module lacks — equally a contract break):")
+                    extraInNoop.forEach { appendLine("    + $it") }
+                }
+            }
+        }
+
+        val problems = listOfNotNull(
+            diffSurface(
+                "Android/JVM (.api)",
+                "sharingan/api/sharingan.api",
+                "sharingan-noop/api/sharingan-noop.api",
+                jvmContractEntries(realApi),
+                jvmContractEntries(noopApi),
+            ),
+            diffSurface(
+                "native/iOS (.klib.api)",
+                "sharingan/api/sharingan.klib.api",
+                "sharingan-noop/api/sharingan-noop.klib.api",
+                nativeContractEntries(realKlib),
+                nativeContractEntries(noopKlib),
+            ),
+        )
+        if (problems.isNotEmpty()) {
+            throw GradleException(
+                problems.joinToString("\n\n") + "\n\n" +
+                    "The release-swap safety story requires :sharingan and :sharingan-noop to\n" +
+                    "expose a signature-for-signature identical public contract. Reconcile the\n" +
+                    "lagging module (add/remove the symbol), run `./gradlew apiDump`, and commit\n" +
+                    "the regenerated dumps. See docs/api-parity.md for the contract and rationale.",
+            )
+        }
+        logger.lifecycle(
+            "API parity OK: :sharingan and :sharingan-noop expose an identical public contract (JVM + native).",
+        )
+    }
+}
+
+// Make the standard `check` lifecycle run the parity gate locally when present.
+tasks.matching { it.name == "check" }.configureEach { dependsOn("checkApiParity") }

docs/api-parity.md

@@ -0,0 +1,132 @@
+# Cross-module API parity
+
+`:sharingan` (the real debug library) and `:sharingan-noop` (the inert release
+replacement) **must expose a signature-for-signature identical public contract.**
+This document explains why, how the `checkApiParity` gate enforces it, and — most
+importantly for contributors — *why a handful of symbols are deliberately excluded
+from the comparison* so you don't "fix" a false positive by breaking the contract.
+
+## Why parity matters
+
+The whole point of the two-module design is the **release swap**: a consumer
+compiles against `:sharingan` in debug builds and substitutes `:sharingan-noop` in
+release builds (typically `debugImplementation` / `releaseImplementation`). For
+that swap to be safe, every symbol the consumer can reference in debug must also
+exist — with an identical signature — in the noop.
+
+The two modules are compiled **independently** from separate sources. Nothing but
+human discipline keeps them in lockstep. A drifted default value, a renamed
+parameter, or a method added to one module and forgotten in the other does not
+fail either module's own build. It surfaces only later, as a **consumer's
+release-build compile error** — the worst possible place to discover it, on
+someone else's machine, at release time.
+
+`checkApiParity` turns that latent, human-dependent invariant into an automated
+gate that fails fast in CI on every PR.
+
+## How the check works
+
+[Binary Compatibility Validator](https://github.com/Kotlin/binary-compatibility-validator)
+(BCV, wired up in #11) dumps each module's full public surface to committed files:
+
+| Surface          | `:sharingan`                     | `:sharingan-noop`                          |
+| ---------------- | -------------------------------- | ------------------------------------------ |
+| Android / JVM    | `sharingan/api/sharingan.api`     | `sharingan-noop/api/sharingan-noop.api`     |
+| Native / iOS     | `sharingan/api/sharingan.klib.api`| `sharingan-noop/api/sharingan-noop.klib.api`|
+
+The `checkApiParity` Gradle task (defined in the root `build.gradle.kts`) reads
+those four committed dumps, reduces each to its **consumer-facing contract** (by
+filtering — see below), and asserts the real and noop contracts are identical for
+*both* surfaces. The comparison is **bidirectional**: it fails if the noop is
+*missing* a symbol the real module has, *and* if the noop *exposes* a symbol the
+real module lacks. Both are contract breaks.
+
+It is pure text processing over already-committed files — no compilation, no
+Kotlin/Native link, no Android or iOS SDK — so it runs in a couple of seconds.
+
+```bash
+./gradlew checkApiParity
+```
+
+In CI it runs as the `api-parity` job in
+[`.github/workflows/api-check.yml`](../.github/workflows/api-check.yml), on a cheap
+`ubuntu` runner (it needs no macOS host).
+
+## Why it is NOT a whole-file diff
+
+A naïve `diff` of the committed dumps would fail constantly, because `:sharingan`
+legitimately carries symbols the noop neither has nor should have. The check
+filters these out before comparing. **If you hit a parity failure, first confirm
+it is a real contract change and not one of the categories below.**
+
+### 1. Debug-only UI / platform classes (real module only)
+
+`:sharingan` ships an in-app log viewer and the Android plumbing to launch it.
+None of it is part of the API a consumer calls directly, so the noop correctly
+omits it. Excluded from the JVM comparison:
+
+- `dev/sharingan/ui/**` — the Compose viewer screens (`SharinganScreenKt`, etc.).
+- `dev/sharingan/internal/**` — `SharinganInitProvider` (the auto-init
+  `ContentProvider`) and `SharinganNotificationReceiver`.
+- `dev/sharingan/SharinganActivity` — the Android Activity that hosts the viewer.
+- any `ComposableSingletons$*` class — synthetic lambda holders generated by the
+  Compose compiler.
+
+On the native surface the only counterpart is the top-level
+`dev.sharingan.ui/SharinganScreen` composable, which is excluded for the same
+reason. (`SharinganViewController` and `presentSharingan` are **not** excluded —
+those *are* the iOS entry points the noop must mirror, and it does.)
+
+### 2. The Compose `$stable` / `$stableprop` synthetics
+
+The Compose compiler adds stability metadata to every class it sees. Because
+Compose ships **only in `:sharingan`**, the real dumps carry symbols the noop
+never will, even for classes that are otherwise part of the shared contract
+(e.g. `HttpEvent`):
+
+- JVM: a `public static final field $stable I` line inside each class.
+- Native: top-level `…$stableprop` values and `…$stableprop_getter` functions.
+
+These are compiler bookkeeping, not API a consumer references, so they are
+stripped before comparing. **This is the subtle one** — it lives *inside* classes
+that are otherwise shared, so a naïve diff would false-fail on every event type.
+
+### 3. The klib header line
+
+Each `.klib.api` begins with a comment block including
+`// Library unique name: <io.github.mibrahimdev:sharingan>` vs `…:sharingan-noop`.
+That line differs by construction. All `//` header comments are dropped before
+comparing.
+
+## The contract that MUST stay in lockstep
+
+After filtering, both modules expose the same 19 JVM contract classes (plus the
+native top-level `SharinganViewController` / `presentSharingan` functions):
+
+`Sharingan`, `HttpLogger` (+`Companion`), `MqttLogger`, `BleLogger`,
+`SharinganStore` (+`Companion`), `SharinganEvent` (+`DefaultImpls`), `HttpEvent`,
+`MqttEvent`, `BleEvent`, `TimingPhase`, `BleOperation`, `MqttDirection`,
+`SharinganExport`, `SharinganAndroidKt`, `ktor.SharinganKtorConfig`,
+`ktor.SharinganKtorKt`.
+
+Anything a consumer can reach lives here, and any change to it must be mirrored in
+**both** modules.
+
+## What to do when `checkApiParity` fails
+
+1. Read the failure. It lists each diverging symbol, qualified by class, under
+   "MISSING from :sharingan-noop" (real has it, noop doesn't) or "NOT in
+   :sharingan" (noop over-exposes it).
+2. Decide which module is correct. Usually you changed `:sharingan`'s public API
+   and forgot to make the same change in `:sharingan-noop` (or vice versa). Make
+   the matching change to the lagging module's source.
+3. Regenerate the dumps and commit them:
+   ```bash
+   ./gradlew apiDump
+   ```
+4. Re-run `./gradlew checkApiParity` to confirm it passes.
+
+If the failure is genuinely one of the excluded categories above leaking through
+(e.g. a new debug-only package), update the filter in the `checkApiParity` task
+*and* document the new exclusion here — never widen the noop to match a debug-only
+symbol.