Ask the scope table by name whether an install matters (#931)

A scope row names a package; the map the injector reads is keyed by uid.
Uninstalling a target and installing it again crosses that gap. The app
returns under a new uid, the entry under the old one went with
ACTION_UID_REMOVED, and the install handler looked a target up by uid in
that same cache — so it matched nothing, asked for no rebuild, and the
row it is still configured under sat unread.

That leaves the configuration correct and inert. The manager reads the
table, so it goes on showing the target ticked; draft and saved agree, so
no apply bar appears and nothing on that screen can put it right. It
takes a scope edit elsewhere, or the next boot.

Asking the scope table by name answers it. The uid test stays behind the
new one for the rows no table holds: a module in its own scope, and the
self-scope derived for a legacy one. Removal asks for its own rebuild
too, that cleanup having only ever been incidental to a uid being
retired.

The lifetime this restores was never written down. The README, the
schema, ConfigCache and the scope editor now say it.

Author: JingMatrix

daemon/README.md

@@ -30,6 +30,20 @@ To handle concurrent IPC requests without starving Android Binder thread pools,
 * Atomic Swaps: When the underlying SQLite database changes, the daemon triggers a conflated channel request. A background coroutine queries the database, computes the new module topology, instantiates a new `DaemonState`, and atomically swaps the reference in `ConfigCache`.
 * Preference Isolation: High-frequency module preference reads and writes are decoupled from the core state. Managed by `PreferenceStore`, preferences are serialized as binary blobs and pushed as differential updates to modules, preventing unnecessary cache rebuilds.
 
+## Scope Configuration
+
+The `scope` table records which applications a module may modify. A row is identified by package name and user id (`PRIMARY KEY (mid, app_pkg_name, user_id)`) and stores no uid. `ConfigCache` resolves those names into the `(process name, uid)` keys the injection path matches on, and re-resolves them on every rebuild. The table is the configuration — what was asked for; the cache is the realisation — what can be loaded right now.
+
+That separation is what gives a scope row its lifetime.
+
+* Rows outlive their target. Uninstalling an application that is in scope removes nothing from the table, and installing it again puts the module back once the cache is rebuilt. This is intended: an update shipped as an uninstall and a reinstall, a ROM migration, or a restore would otherwise drop the configuration without saying anything. Scope backups are lists of package names for the same reason, and restore rows for applications that are not installed yet.
+* The package name is the whole identity. A different build that later claims the name inherits the scope, so a row is a statement about a name rather than about a signed application.
+* While its target is absent, a row is invisible in the manager, which lists installed applications only. It survives an apply regardless: the scope editor writes the difference the reader made rather than replacing the table, so a row it cannot draw is neither shown nor dropped.
+* Four paths delete a row, and nothing else does — a module's scope being replaced wholesale (a manager apply, the CLI, a restore), an explicit removal (`scope remove`, or a module withdrawing its own request), pruning a module to the scope its `module.prop` fixes, and the foreign key cascade when the module itself is uninstalled.
+* Package events decide by name. A package that has just been installed arrives under a uid the cache has never seen — a reinstalled target is not the uid it left as — so `VectorService` asks the scope table whether the package is a target before requesting a rebuild. Matching the arriving uid against the cache answers "no" for precisely the case that needs the rebuild.
+
+Module configuration follows the opposite rule: a module whose package no user holds any more is deleted from the database along with its scope and its preferences. A module that is not installed cannot be loaded into anything, while a target that is not installed is only a target that is not running.
+
 ## IPC Architecture
 
 The daemon implements a multi-layered IPC design utilizing Android's Binder mechanism and UNIX domain sockets. It avoids registering standard AIDL services with `ServiceManager`, relying instead on intercepting Binder transactions via the Zygisk module and actively pushing Binder references to target processes.

daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt

@@ -312,6 +312,16 @@ object VectorService : IVectorDaemon.Stub() {
           if (isRemovedForAllUsers && ModuleDatabase.removeModule(moduleName)) {
             // If it was in our DB and we successfully removed it, we treat it as an Xposed module.
             isXposedModule = true
+          } else if (ModuleDatabase.isEnabledScopeTarget(moduleName)) {
+            // A target's scope rows outlive it deliberately — they are what puts the module back
+            // when the app returns — but the entry the cache derived from them must not, and this
+            // branch asked for nothing. What cleaned it up was ACTION_UID_REMOVED, further down,
+            // and only ever incidentally: it fires when the *uid* is retired, which is neither
+            // this event nor guaranteed to follow it. A package that shares a uid with another
+            // retires none, and a daemon not running to hear the one that is sent never learns of
+            // it. The entry left behind is keyed by a uid that no longer names this app, and
+            // Android does hand a freed app id to the next installer that asks.
+            ConfigCache.requestCacheUpdate()
           }
         }
       }
@@ -323,9 +333,23 @@ object VectorService : IVectorDaemon.Stub() {
               ModuleDatabase.updateModuleApkPath(
                   moduleName, ConfigCache.getModuleApkPath(appInfo), false)
         } else {
-          if (ConfigCache.state.scopes.keys.any { it.uid == uid }) {
-            // If not a module, but it's an app that was previously a "scope" (target)
-            // for a module, we need to refresh the cache.
+          // If not a module, but it's an app some module targets, the cache has to be rebuilt so
+          // that the target is resolved again.
+          //
+          // The configuration is asked first, and by name, because the cache alone gets the one
+          // case that matters most wrong. Its scopes are keyed by uid: an app that is uninstalled
+          // and installed again comes back under a *new* uid, and the entry under the old one went
+          // when ACTION_UID_REMOVED rebuilt the cache — so no key matched, no rebuild was asked
+          // for, and the app was left out of the scope map it is still configured to be in. The
+          // row was never deleted, so the manager went on showing the target ticked, correctly,
+          // beside an app nothing was being loaded into; there was no difference to apply and
+          // therefore no way to put it right from the manager at all. It stayed that way until
+          // something unrelated rebuilt the cache — any scope edit, or the next boot.
+          //
+          // The uid test is kept behind it for the rows no scope table holds: a module in its own
+          // scope, and the self-scope a legacy module gets derived rather than stored.
+          if ((moduleName != null && ModuleDatabase.isEnabledScopeTarget(moduleName)) ||
+              ConfigCache.state.scopes.keys.any { it.uid == uid }) {
             ConfigCache.requestCacheUpdate()
           }
 

daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt

@@ -226,6 +226,11 @@ object ConfigCache {
 
         // Gone, not broken. No user has this package any more, so the configuration for it is
         // meaningless and is cleaned up. This is the only case that deletes anything.
+        //
+        // The rule belongs to modules alone, and the asymmetry with a *target* that no user holds
+        // is the point: a module that is not installed cannot be loaded into anything, while a
+        // target that is not installed is only a target that is not running. Its scope rows stay,
+        // and are what puts the module back when it returns — see the `scope` table's own note.
         if (pkgInfo?.applicationInfo == null) {
           Log.w(TAG, "Failed to find package info of $pkgName")
           obsoleteModules.add(pkgName)

daemon/src/main/kotlin/org/matrix/vector/daemon/data/Database.kt

@@ -45,6 +45,22 @@ class Database(context: Context? = FakeContext()) :
         """
         CREATE TABLE IF NOT EXISTS scope (
             mid integer,
+            -- A target is named, never numbered: a package and a user, and no uid. What the
+            -- injection path matches on is a uid, but that is derived in `ConfigCache` and rebuilt
+            -- from these rows, so a target that comes back under a new uid is found again by name.
+            --
+            -- Which is what gives a row its lifetime: it outlives the application it names.
+            -- Uninstalling a target deletes nothing here, and installing it again puts the module
+            -- back as soon as the cache is rebuilt. Deliberately so — an update shipped as an
+            -- uninstall and a reinstall, a ROM migration, or a restore from a backup would
+            -- otherwise drop the configuration without saying anything. The cost of it is that the
+            -- name is the whole identity, so a different build that later claims the name inherits
+            -- the scope: a row is a statement about a name, not about a signed application.
+            --
+            -- Four things delete one, and nothing else does: a module's scope being replaced
+            -- wholesale (a manager apply, the CLI, a restore), an explicit removal (`scope remove`,
+            -- or a module withdrawing its own request), pruning a module to the scope its
+            -- module.prop fixes, and the cascade below when the module itself is uninstalled.
             app_pkg_name text NOT NULL,
             user_id integer NOT NULL,
             PRIMARY KEY (mid, app_pkg_name, user_id),

daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt

@@ -143,6 +143,28 @@ object ModuleDatabase {
     return rows
   }
 
+  /**
+   * Whether any enabled module names [appPackage] as a target, in any user.
+   *
+   * Asked of the configuration rather than of [ConfigCache], because the cache cannot answer it.
+   * Its scope map is keyed by uid, and a package that was uninstalled and installed again comes
+   * back under a *new* one — while the row here, keyed by name, has been waiting for it the whole
+   * time. Matching the cache by uid therefore said "not a target" about the one case that most
+   * needs a rebuild, and the app stayed unhooked until something unrelated rebuilt the cache.
+   */
+  fun isEnabledScopeTarget(appPackage: String): Boolean =
+      dbHelper.readableDatabase
+          .query(
+              "scope INNER JOIN modules ON scope.mid = modules.mid",
+              arrayOf("1"),
+              "app_pkg_name = ? AND enabled = 1",
+              arrayOf(appPackage),
+              null,
+              null,
+              null,
+              "1")
+          .use { it.moveToFirst() }
+
   /** Enabled modules scoped to the system framework, with the path last resolved for each. */
   fun systemServerModuleRows(): List<EnabledModuleRow> {
     val rows = mutableListOf<EnabledModuleRow>()

manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeViewModel.kt

@@ -490,6 +490,12 @@ class ScopeViewModel(
                 // A scope the daemon will not hand over shows as none rather than keeping the
                 // screen shut; [readSavedScope] logs why, and keeps that case apart from the empty
                 // list a module with nothing ticked legitimately has.
+                // Everything the daemon holds, and not only what the list above can draw. A scope
+                // row outlives the app it names — uninstalling a target deletes nothing, so that
+                // installing it again puts the module back — and such a row has no app to appear
+                // as, since the list is built from installed packages. Seeding the draft with the
+                // whole saved set is what carries it through: the apply writes the difference this
+                // screen made, so a row nothing here can see is neither shown nor dropped.
                 val saved = readSavedScope() ?: emptySet()
                 savedScope.value = saved
                 draftScope.value = saved