Cache-refresh membership rule, honest @Suppress, diagnostics glossary

lsposed/AGENTS.md

@@ -27,6 +27,27 @@ What stays in the root package is the shared vocabulary both sides use —
 `DashboardData`, the agent bridge. Moving those buys import churn and nothing
 else; they belong to no single feature.
 
+## "Diagnostic" names seven different things
+
+The word got attached to every layer that answers "what is going on", so the
+name alone will not tell you which one you are looking at. Nothing here is
+misplaced — they genuinely are all diagnostics — but know which is which before
+you add to any of them:
+
+| name | what it actually is |
+|---|---|
+| `DiagnosticsScreen`, *Detailed diagnostics* | the user-facing check suite |
+| `DiagnosticsCache` | the **run state** of that suite (NotRun / Running / Failed) |
+| `DiagnosticReport`, `buildDiagnosticReport`, `DiagnosticCheck` | the **canonical model** the screen and the bundle both render — see `docs/diagnostics.md` |
+| `DiagnosticGate`, `RoutingGateCache`, `resolveDiagnosticGate` | the **precondition** for a meaningful run (VPN up, this app routed) — not a check |
+| `HookDiagnostics`, `ConnectivityAttachDiagnostics`, `KpmDiagnostics` | attach/telemetry for the hooks themselves; **not part of the suite** |
+| `writeDiagnosticZip`, `debug/` | the export — the artifact is the *debug bundle* (`docs/debug-bundle.md`) |
+| `app_scan_diagnostics`, `kmod_diag` | section names inside that bundle |
+
+The pair worth keeping straight: `DiagnosticsCache` holds a **run**,
+`RoutingGateCache` holds a **precondition**, and only the second is derived from
+the canonical config (so only the second is refreshed after a write).
+
 ## Two processes, one APK
 
 The single most important thing about this module: `hook/` is loaded by LSPosed
@@ -56,7 +77,10 @@ directions, since `internal` is module-wide and the compiler will not.
 
 - **`StateCache<T>`** — base for every app-scoped, lazily-loaded cache
   (loading/error/value flows + single-flight job). A new cache **extends this**;
-  never hand-roll `inflight`/`loading` again.
+  never hand-roll `inflight`/`loading` again. If its value is derived from the
+  canonical config, also add it to `CanonicalConfigRepository.derivedCaches` —
+  that list, and only that list, is what a config write refreshes. Membership
+  rule and the deliberate non-members are documented on it.
 - **`RootSnapshotCache`** — the single batched root read. Need new system state
   on the Dashboard/Hiding path? Add a section to its shell snapshot; don't
   add an ad-hoc `suExec` that races the snapshot.

lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/CanonicalConfigRepository.kt

@@ -63,17 +63,42 @@ internal object CanonicalConfigRepository {
             CanonicalWriteResult(exit, output)
         }
 
-    // Reload each derived cache in place — swap old→new, so no observer sees a null blank
-    // between the write and the reload (the toggle-flicker fix). Root first: the others
-    // derive from it and reuse it via force=false. Failures keep the stale value.
+    /**
+     * The caches whose value is *derived from the canonical config*, and which a
+     * write therefore leaves stale. That is the whole membership rule — add a cache
+     * here if and only if its `load` reads the config (directly, or via the root
+     * snapshot's config-bearing sections).
+     *
+     * Deliberately NOT members: `AppListCache` (an `AppSummary` carries no config
+     * state — the picker merges target flags in reactively), `UpdateCheckCache` and
+     * `DiagnosticsCache` (unrelated to the config), and `SystemServerConfigCache`
+     * (lives in the system_server process, unreachable from here — its own
+     * `SystemDataFileWatcher` invalidates it).
+     *
+     * `RootSnapshotCache` is the shared upstream rather than a member; see
+     * [refreshDerivedCaches].
+     */
+    private val derivedCaches: List<StateCache<*>> =
+        listOf(TargetsCache, DashboardCache, StatisticsCache, RoutingGateCache)
+
+    /**
+     * Reload every config-derived cache in place — swap old→new, so no observer sees
+     * a null blank between the write and the reload (the toggle-flicker fix).
+     *
+     * The root snapshot goes first and alone: the others all derive from it, so they
+     * follow with `force = false` to reuse it. Passing `force = true` here would be a
+     * silent, invisible cost — each cache would invalidate the snapshot and re-run the
+     * whole root shell for itself, once per member. Iterating a list instead of
+     * open-coding the calls is what keeps the order and the flag structural rather
+     * than a comment someone has to notice.
+     *
+     * A failure keeps the stale value (the cache records its own error), so one
+     * unhappy cache can't abort the rest.
+     */
     internal suspend fun refreshDerivedCaches() {
         runCatching { RootSnapshotCache.refresh() }
-        runCatching { TargetsCache.refreshInPlace(force = false) }
-        runCatching { DashboardCache.refreshInPlace(force = false) }
-        runCatching { StatisticsCache.refreshInPlace(force = false) }
-        // Tolerates RoutingGateCache not being initialized yet (load() throws on a
-        // null appContext before any screen has called ensureLoaded/refresh) — a
-        // config write racing app startup should not crash the write itself.
-        runCatching { RoutingGateCache.refreshInPlace(force = false) }
+        derivedCaches.forEach { cache ->
+            if (!cache.pristine) runCatching { cache.refreshInPlace(force = false) }
+        }
     }
 }

lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/StateCache.kt

@@ -78,6 +78,19 @@ internal abstract class StateCache<T>(
         reload(force)
     }
 
+    /**
+     * True while the cache has never held anything — no value, no failed load.
+     *
+     * A pristine cache has nothing that can go stale, so a config write can skip
+     * it: the next [ensure] loads it fresh anyway. Skipping also matters because
+     * [refreshInPlace] bypasses the concrete cache's `ensureLoaded`, so calling it
+     * first would run [load] without the inputs that method stashes — the load
+     * fails, [reload] records the error, and [ensure] then early-returns on that
+     * error forever. Kept in the error case on purpose: that one is worth retrying.
+     */
+    val pristine: Boolean
+        get() = _value.value == null && _error.value == null
+
     /** Drop the cached value/error so the next [ensure] reloads. */
     open fun invalidate() {
         _value.value = null

lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/diagnostics/JavaChecks.kt

@@ -1,32 +1,22 @@
+// The deprecated ConnectivityManager surface is the point of this file, not an
+// oversight: `allNetworks`, `getNetworkInfo(type)`, `getNetworkInfo(network)`,
+// `activeNetworkInfo` and the network-handle calls are exactly what a VPN-probing
+// app reaches for, so the checks that prove we hid the tunnel have to reach for the
+// same ones. Migrating them to the modern equivalents would silently drop detection
+// coverage — see docs/detection-vectors.md for the vector each one stands in for.
+//
+// Hence file-level rather than the six per-function suppressions this replaces: the
+// whole file is deliberately-legacy probe code, and there is no non-probe code here
+// for the blanket to hide a genuine deprecation warning in.
+@file:Suppress("DEPRECATION")
+
 package dev.okhsunrog.vpnhide.diagnostics
 
 import android.net.ConnectivityManager
 import android.net.LinkProperties
 import android.net.Network
 import android.net.NetworkCapabilities
-import android.net.NetworkInfo
-import android.net.Uri
 import android.os.Build
-import androidx.activity.compose.rememberLauncherForActivityResult
-import androidx.activity.result.contract.ActivityResultContracts
-import androidx.compose.foundation.isSystemInDarkTheme
-import androidx.compose.foundation.layout.*
-import androidx.compose.foundation.rememberScrollState
-import androidx.compose.foundation.shape.RoundedCornerShape
-import androidx.compose.foundation.verticalScroll
-import androidx.compose.material.icons.Icons
-import androidx.compose.material.icons.filled.FiberManualRecord
-import androidx.compose.material.icons.filled.Stop
-import androidx.compose.material3.*
-import androidx.compose.runtime.*
-import androidx.compose.ui.Alignment
-import androidx.compose.ui.Modifier
-import androidx.compose.ui.platform.LocalContext
-import androidx.compose.ui.res.stringResource
-import androidx.compose.ui.text.font.FontFamily
-import androidx.compose.ui.text.font.FontWeight
-import androidx.compose.ui.unit.dp
-import androidx.compose.ui.unit.sp
 import dev.okhsunrog.vpnhide.LogTags
 import dev.okhsunrog.vpnhide.R
 import dev.okhsunrog.vpnhide.VpnHideLog
@@ -35,22 +25,10 @@ import dev.okhsunrog.vpnhide.checks.CheckStatus
 import dev.okhsunrog.vpnhide.checks.NativeProbe
 import dev.okhsunrog.vpnhide.generated.IfaceLists
 import dev.okhsunrog.vpnhide.next
-import dev.okhsunrog.vpnhide.ui.components.EnhancedButton
-import dev.okhsunrog.vpnhide.ui.components.EnhancedCard
-import dev.okhsunrog.vpnhide.ui.components.GroupedCard
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.launch
-import kotlinx.coroutines.withContext
-import java.io.File
 import java.net.NetworkInterface
-import java.text.SimpleDateFormat
-import java.util.Date
-import java.util.Locale
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicReference
-import java.util.zip.ZipEntry
-import java.util.zip.ZipOutputStream
 
 private const val TAG = LogTags.TEST
 
@@ -327,7 +305,6 @@ private fun checkNetworkInterfaceEnum(name: String): CheckResult =
         javaCheck(name, null, "${e.message}")
     }
 
-@Suppress("DEPRECATION")
 internal fun checkAllNetworksVpn(
     cm: ConnectivityManager,
     name: String,
@@ -377,7 +354,6 @@ private data class NetworkForTypeResult(
     val error: String? = null,
 )
 
-@Suppress("DEPRECATION")
 private fun queryNetworkForType(
     cm: ConnectivityManager,
     type: Int,
@@ -392,7 +368,6 @@ private fun queryNetworkForType(
         NetworkForTypeResult(error = t.cause?.message ?: t.message ?: t.javaClass.simpleName)
     }
 
-@Suppress("DEPRECATION")
 private fun checkNetworkForTypeVpn(
     cm: ConnectivityManager,
     name: String,
@@ -415,7 +390,6 @@ private fun checkNetworkForTypeVpn(
     return javaCheck(name, false, detail)
 }
 
-@Suppress("DEPRECATION")
 private fun checkActiveNetworkHandle(
     cm: ConnectivityManager,
     name: String,
@@ -436,7 +410,6 @@ private fun checkActiveNetworkHandle(
     return javaCheck(name, !leaksVpnHandle, detail)
 }
 
-@Suppress("DEPRECATION")
 private fun checkAllNetworksHandles(
     cm: ConnectivityManager,
     name: String,
@@ -553,7 +526,6 @@ private fun checkLinkPropertiesRoutes(
 // leak. (Its companion getActiveNetworkInfo() was dropped: .type reports the
 // underlying transport (WIFI/mobile) for an active VPN, not TYPE_VPN, so it never
 // surfaced the leak.)
-@Suppress("DEPRECATION")
 private fun checkNetworkInfoVpn(
     cm: ConnectivityManager,
     name: String,

lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/HookEntry.kt

@@ -199,6 +199,9 @@ class HookEntry : IXposedHookLoadPackage {
     /** Remove routes whose interface is a VPN tunnel. Returns true if any went. */
     private fun sanitizeLinkRoutes(copy: LinkProperties): Boolean {
         try {
+            // Unchecked only in the element type: AOSP declares LinkProperties.mRoutes as
+            // ArrayList<RouteInfo>, and the `as?` still checks List-ness at runtime, so a
+            // ROM that reshaped the field falls out as null rather than crashing here.
             @Suppress("UNCHECKED_CAST")
             val routesField = XposedHelpers.getObjectField(copy, "mRoutes") as? MutableList<RouteInfo> ?: return false
             val filtered =
@@ -223,6 +226,8 @@ class HookEntry : IXposedHookLoadPackage {
     private fun sanitizeStackedLinks(copy: LinkProperties): Boolean {
         var modified = false
         try {
+            // Same shape as mRoutes above: AOSP declares mStackedLinks as
+            // Hashtable<String, LinkProperties>; Map-ness is still checked at runtime.
             @Suppress("UNCHECKED_CAST")
             val stacked = XposedHelpers.getObjectField(copy, "mStackedLinks") as? MutableMap<String, LinkProperties>
             if (stacked != null && stacked.isNotEmpty()) {

lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/PackageVisibilityHooks.kt

@@ -186,6 +186,11 @@ internal object PackageVisibilityHooks {
                 val pls = parceledListSliceClass ?: return
                 if (!pls.isInstance(result)) return
 
+                // T is whatever element type the caller hooked for (PackageInfo,
+                // ApplicationInfo, ResolveInfo…). ParceledListSlice.getList() is
+                // declared List<T> on the framework side; List-ness is checked at
+                // runtime, the element type is taken on trust — pkgOf below is the
+                // only thing that touches an element, and it is null-tolerant.
                 @Suppress("UNCHECKED_CAST")
                 val original = XposedHelpers.callMethod(result, "getList") as? List<T> ?: return
                 val filtered =
@@ -336,6 +341,9 @@ internal object PackageVisibilityHooks {
         object : XC_MethodHook() {
             override fun afterHookedMethod(param: MethodHookParam) {
                 if (param.hasThrowable()) return
+                // Unchecked only in element nullability — getNamesForUids returns
+                // String[] with null holes for unresolved uids. The array class itself
+                // is still checked, so a ROM returning something else bails out.
                 @Suppress("UNCHECKED_CAST")
                 val names = param.result as? Array<String?> ?: return
                 val caller = observerCaller() ?: return

lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ui/theme/Motion.kt

@@ -60,6 +60,21 @@ object AppMotion {
     fun <T> slowEffects(): AnimationSpec<T> = tween(durationMillis = 500, easing = AppEasing.FancyTransition)
 }
 
+/**
+ * Hand a spec cached as `Any` back out as one for [T].
+ *
+ * [MotionScheme] asks for a fresh `FiniteAnimationSpec<T>` per call site, but the
+ * specs are built once and shared, so the erased element type has to be restored on
+ * the way out. Sound for what is cached here: every spec below is a `tween` or a
+ * `spring` with no visibility threshold, and neither ever touches a value of `T` —
+ * they vectorize through the converter supplied at animation time. Do not route a
+ * spec that carries `T`-typed data (a threshold, a keyframe) through this.
+ *
+ * One suppression, stated once, instead of six identical ones down the object.
+ */
+@Suppress("UNCHECKED_CAST")
+private fun <T> AnimationSpec<Any>.retyped(): FiniteAnimationSpec<T> = this as FiniteAnimationSpec<T>
+
 /**
  * Custom [MotionScheme] driving every expressive Material 3 component animation.
  *
@@ -78,21 +93,15 @@ val AppMotionScheme: MotionScheme =
         private val fastEffects = AppMotion.fastEffects<Any>()
         private val slowEffects = AppMotion.slowEffects<Any>()
 
-        @Suppress("UNCHECKED_CAST")
-        override fun <T> defaultSpatialSpec(): FiniteAnimationSpec<T> = defaultSpatial as FiniteAnimationSpec<T>
+        override fun <T> defaultSpatialSpec(): FiniteAnimationSpec<T> = defaultSpatial.retyped()
 
-        @Suppress("UNCHECKED_CAST")
-        override fun <T> fastSpatialSpec(): FiniteAnimationSpec<T> = fastSpatial as FiniteAnimationSpec<T>
+        override fun <T> fastSpatialSpec(): FiniteAnimationSpec<T> = fastSpatial.retyped()
 
-        @Suppress("UNCHECKED_CAST")
-        override fun <T> slowSpatialSpec(): FiniteAnimationSpec<T> = slowSpatial as FiniteAnimationSpec<T>
+        override fun <T> slowSpatialSpec(): FiniteAnimationSpec<T> = slowSpatial.retyped()
 
-        @Suppress("UNCHECKED_CAST")
-        override fun <T> defaultEffectsSpec(): FiniteAnimationSpec<T> = defaultEffects as FiniteAnimationSpec<T>
+        override fun <T> defaultEffectsSpec(): FiniteAnimationSpec<T> = defaultEffects.retyped()
 
-        @Suppress("UNCHECKED_CAST")
-        override fun <T> fastEffectsSpec(): FiniteAnimationSpec<T> = fastEffects as FiniteAnimationSpec<T>
+        override fun <T> fastEffectsSpec(): FiniteAnimationSpec<T> = fastEffects.retyped()
 
-        @Suppress("UNCHECKED_CAST")
-        override fun <T> slowEffectsSpec(): FiniteAnimationSpec<T> = slowEffects as FiniteAnimationSpec<T>
+        override fun <T> slowEffectsSpec(): FiniteAnimationSpec<T> = slowEffects.retyped()
     }