Keep superseded accounts out of numberless account discovery

Record numberless accounts explicitly instead of treating every retained ACI
as a discoverable account. Resolve older index entries against saved account
state, preserving ACI lookup and retrying unavailable state on later reads.

The optional index field remains compatible with version 2 readers. Update
native-image reflection metadata and cover replacement, legacy migration,
locked and missing state, and mismatched ACIs in regression tests.

doc/numberless-accounts.md

@@ -48,8 +48,9 @@ account has no phone number, and `Signal.getSelfACI` returns the ACI.
 
 Run `./gradlew test installDist` with JDK 25. Regression tests cover the linking
 request (including ACI authentication and omission of PNI keys), numbered-account
-linking, missing group credential salts, persisted account reload, account discovery,
-group membership lookup, recipient-number formatting, and JSON account identifiers.
+linking, missing group credential salts, persisted account reload, account discovery
+(including superseded accounts and legacy indexes), group membership lookup,
+recipient-number formatting, and JSON account identifiers.
 
 D-Bus transport tests require a session bus. To run them on an isolated bus, use
 `dbus-run-session -- ./gradlew --no-daemon :test --tests '*NumberlessDbusTest*' --rerun-tasks`.

lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStorage.java

@@ -4,5 +4,6 @@ import java.util.List;
 
 public record AccountsStorage(List<Account> accounts, Integer version) {
 
-    public record Account(String path, String environment, String number, String uuid) {}
+    // Older indexes omit this flag; entries with only a UUID must then be checked against their account state.
+    public record Account(String path, String environment, String number, String uuid, Boolean numberless) {}
 }

lib/src/main/java/org/asamk/signal/manager/storage/accounts/AccountsStore.java

@@ -63,7 +63,7 @@ public class AccountsStore {
     public synchronized Set<AccountsStorage.Account> getAllAccounts() throws IOException {
         return readAccounts().stream()
                 .filter(a -> a.environment() == null || serviceEnvironment.equals(a.environment()))
-                .filter(a -> a.number() != null || a.uuid() != null)
+                .filter(a -> a.number() != null || (a.uuid() != null && Boolean.TRUE.equals(a.numberless())))
                 .collect(Collectors.toSet());
     }
 
@@ -98,14 +98,15 @@ public class AccountsStore {
                 return new AccountsStorage.Account(a.path(),
                         serviceEnvironment,
                         number,
-                        aci == null ? null : aci.toString());
+                        aci == null ? null : aci.toString(),
+                        number == null && aci != null);
             }
 
             if (number != null && number.equals(a.number())) {
-                return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid());
+                return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid(), false);
             }
             if (aci != null && aci.toString().equals(a.uuid())) {
-                return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null);
+                return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null, false);
             }
 
             return a;
@@ -117,7 +118,8 @@ public class AccountsStore {
         final var account = new AccountsStorage.Account(accountPath,
                 serviceEnvironment,
                 number,
-                aci == null ? null : aci.toString());
+                aci == null ? null : aci.toString(),
+                number == null && aci != null);
         updateAccounts(accounts -> {
             final var existingAccounts = accounts.stream().map(a -> {
                 if (a.environment() != null && !serviceEnvironment.equals(a.environment())) {
@@ -125,10 +127,10 @@ public class AccountsStore {
                 }
 
                 if (number != null && number.equals(a.number())) {
-                    return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid());
+                    return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid(), false);
                 }
                 if (aci != null && aci.toString().equals(a.uuid())) {
-                    return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null);
+                    return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null, false);
                 }
 
                 return a;
@@ -159,7 +161,7 @@ public class AccountsStore {
     private void createInitialAccounts() throws IOException {
         final var legacyAccountPaths = getLegacyAccountPaths();
         final var accountsStorage = new AccountsStorage(legacyAccountPaths.stream()
-                .map(number -> new AccountsStorage.Account(number, null, number, null))
+                .map(number -> new AccountsStorage.Account(number, null, number, null, false))
                 .toList(), CURRENT_STORAGE_VERSION);
 
         IOUtils.createPrivateDirectories(dataPath);
@@ -191,7 +193,7 @@ public class AccountsStore {
     private List<AccountsStorage.Account> readAccounts() throws IOException {
         final var pair = openFileChannel(getAccountsFile());
         try (final var fileChannel = pair.first(); final var lock = pair.second()) {
-            final var storage = readAccountsLocked(fileChannel);
+            var storage = readAccountsLocked(fileChannel);
 
             var accountsVersion = storage.version() == null ? 1 : storage.version();
             if (accountsVersion > CURRENT_STORAGE_VERSION) {
@@ -200,12 +202,36 @@ public class AccountsStore {
                 throw new IOException("Accounts file was created by a no longer supported older version: "
                         + accountsVersion);
             } else if (accountsVersion < CURRENT_STORAGE_VERSION) {
-                return upgradeAccountsFile(fileChannel, storage, accountsVersion).accounts();
+                storage = upgradeAccountsFile(fileChannel, storage, accountsVersion);
             }
-            return storage.accounts();
+            final var accounts = storage.accounts().stream().map(this::resolveNumberlessAccount).toList();
+            if (!accounts.equals(storage.accounts())) {
+                saveAccountsLocked(fileChannel, new AccountsStorage(accounts, storage.version()));
+            }
+            return accounts;
         }
     }
 
+    private AccountsStorage.Account resolveNumberlessAccount(final AccountsStorage.Account entry) {
+        if (entry.numberless() != null) {
+            return entry;
+        }
+
+        var numberless = false;
+        if (entry.number() == null && entry.uuid() != null) {
+            // Older entries also lose their number when another account takes it over.
+            try (final var account = accountLoader.loadAccountOrNull(entry.path())) {
+                if (account == null) {
+                    // Keep the entry unresolved so a locked or unavailable account can be checked later.
+                    return entry;
+                }
+                numberless = account.getNumber() == null && account.getAci() != null && entry.uuid()
+                        .equals(account.getAci().toString());
+            }
+        }
+        return new AccountsStorage.Account(entry.path(), entry.environment(), entry.number(), entry.uuid(), numberless);
+    }
+
     private AccountsStorage upgradeAccountsFile(
             final FileChannel fileChannel,
             final AccountsStorage storage,
@@ -226,7 +252,8 @@ public class AccountsStore {
                         return new AccountsStorage.Account(a.path(),
                                 getServiceEnvironmentString(account.getServiceEnvironment()),
                                 a.number(),
-                                a.uuid());
+                                a.uuid(),
+                                a.numberless());
                     }
                 }).toList();
             }

lib/src/test/java/org/asamk/signal/manager/storage/accounts/AccountsStoreTest.java

@@ -1,13 +1,22 @@
 package org.asamk.signal.manager.storage.accounts;
 
+import org.asamk.signal.manager.Settings;
 import org.asamk.signal.manager.api.ServiceEnvironment;
+import org.asamk.signal.manager.storage.SignalAccount;
+import org.asamk.signal.manager.storage.Utils;
+import org.asamk.signal.manager.util.KeyUtils;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.signal.core.models.ServiceId.ACI;
 
+import java.io.IOException;
+import java.nio.channels.OverlappingFileLockException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -15,6 +24,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class AccountsStoreTest {
 
+    private static final ACI OLD_ACI = ACI.parseOrThrow("11111111-1111-4111-8111-111111111111");
+    private static final ACI NEW_ACI = ACI.parseOrThrow("22222222-2222-4222-8222-222222222222");
+
     @TempDir
     Path directory;
 
@@ -46,4 +58,151 @@ class AccountsStoreTest {
         assertEquals(newPath, store.getPathByAci(aci));
         assertEquals(1, store.getAllAccounts().size());
     }
+
+    @ParameterizedTest
+    @ValueSource(booleans = {false, true})
+    void replacingANumberDoesNotRediscoverTheOldAccountAsNumberless(final boolean update) throws Exception {
+        final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> null);
+        final var oldPath = store.addAccount("+12025550123", OLD_ACI);
+        Files.createFile(directory.resolve(oldPath));
+        final String newPath;
+        if (update) {
+            newPath = store.addAccount("+12025550124", NEW_ACI);
+            store.updateAccount(newPath, "+12025550123", NEW_ACI);
+        } else {
+            newPath = store.addAccount("+12025550123", NEW_ACI);
+        }
+
+        final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> {
+            throw new AssertionError("New entries should not require loading account state for discovery");
+        });
+        assertEquals(Set.of(newPath), getAccountPaths(reopened));
+        assertEquals(newPath, reopened.getPathByNumber("+12025550123"));
+        assertEquals(oldPath, reopened.getPathByAci(OLD_ACI));
+
+        // Only an explicit account update should turn the old entry into a numberless account.
+        reopened.updateAccount(oldPath, null, OLD_ACI);
+        assertEquals(Set.of(oldPath, newPath), getAccountPaths(reopened));
+    }
+
+    @ParameterizedTest
+    @ValueSource(ints = {1, 2})
+    void legacyIndexesDistinguishNumberlessAccountsFromSupersededNumberedAccounts(final int version) throws Exception {
+        createSavedAccount("old", "+12025550123", OLD_ACI);
+        createSavedAccount("numberless", null, NEW_ACI);
+        final var legacyIndex = """
+                                {
+                                  "version": %d,
+                                  "accounts": [
+                                    {"path": "old", "environment": %s, "number": null, "uuid": "%s"},
+                                    {"path": "numberless", "environment": %s, "number": null, "uuid": "%s"},
+                                    {"path": "current", "environment": "STAGING", "number": "+12025550123", "uuid": null}
+                                  ]
+                                }
+                                """.formatted(version,
+                version == 1 ? "null" : "\"STAGING\"",
+                OLD_ACI,
+                version == 1 ? "null" : "\"STAGING\"",
+                NEW_ACI);
+        Files.writeString(directory.resolve("accounts.json"), legacyIndex);
+
+        final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
+        assertEquals(Set.of("numberless", "current"), getAccountPaths(store));
+        assertEquals("old", store.getPathByAci(OLD_ACI));
+        assertEquals("numberless", store.getPathByAci(NEW_ACI));
+        assertEquals(Set.of("+12025550123"), store.getAllNumbers());
+
+        final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> {
+            throw new AssertionError("Resolved entries should not require loading account state again");
+        });
+        assertEquals(Set.of("numberless", "current"), getAccountPaths(reopened));
+        assertTrue(new AccountsStore(directory.toFile(),
+                ServiceEnvironment.LIVE,
+                this::loadAccountOrNull).getAllAccounts().isEmpty());
+    }
+
+    @Test
+    void retriesLockedLegacyAccountsAndPersistsSuccessfulClassification() throws Exception {
+        createSavedAccount("numberless", null, NEW_ACI);
+        writeLegacyNumberlessEntry(NEW_ACI);
+        final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
+
+        try (final var account = SignalAccount.load(directory.toFile(), "numberless", false, Settings.DEFAULT)) {
+            assertTrue(store.getAllAccounts().isEmpty());
+            assertEquals("numberless", store.getPathByAci(NEW_ACI));
+            final var storage = Utils.createStorageObjectMapper()
+                    .readValue(directory.resolve("accounts.json").toFile(), AccountsStorage.class);
+            assertNull(storage.accounts().getFirst().numberless());
+        }
+
+        assertEquals(Set.of("numberless"), getAccountPaths(store));
+        try (final var account = SignalAccount.load(directory.toFile(), "numberless", false, Settings.DEFAULT)) {
+            final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> {
+                throw new AssertionError("A classified account should remain discoverable while locked");
+            });
+            assertEquals(Set.of("numberless"), getAccountPaths(reopened));
+        }
+    }
+
+    @Test
+    void retriesLegacyAccountsAfterMissingStateIsRestored() throws Exception {
+        writeLegacyNumberlessEntry(NEW_ACI);
+        final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
+        assertTrue(store.getAllAccounts().isEmpty());
+        assertEquals("numberless", store.getPathByAci(NEW_ACI));
+
+        createSavedAccount("numberless", null, NEW_ACI);
+        assertEquals(Set.of("numberless"), getAccountPaths(store));
+    }
+
+    @Test
+    void doesNotDiscoverLegacyEntriesWithAMismatchedAci() throws Exception {
+        createSavedAccount("numberless", null, NEW_ACI);
+        writeLegacyNumberlessEntry(OLD_ACI);
+        final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
+        assertTrue(store.getAllAccounts().isEmpty());
+        assertEquals("numberless", store.getPathByAci(OLD_ACI));
+    }
+
+    private Set<String> getAccountPaths(final AccountsStore store) throws IOException {
+        return store.getAllAccounts().stream().map(AccountsStorage.Account::path).collect(Collectors.toSet());
+    }
+
+    private void writeLegacyNumberlessEntry(final ACI aci) throws IOException {
+        final var legacyIndex = """
+                                {
+                                  "version": 2,
+                                  "accounts": [
+                                    {"path": "numberless", "environment": "STAGING", "number": null, "uuid": "%s"}
+                                  ]
+                                }
+                                """.formatted(aci);
+        Files.writeString(directory.resolve("accounts.json"), legacyIndex);
+    }
+
+    private void createSavedAccount(final String path, final String number, final ACI aci) throws IOException {
+        // Account creation needs a number until the ACI has been assigned.
+        try (final var account = SignalAccount.create(directory.toFile(),
+                path,
+                "+12025550123",
+                ServiceEnvironment.STAGING,
+                KeyUtils.generateIdentityKeyPair(),
+                KeyUtils.generateIdentityKeyPair(),
+                KeyUtils.createProfileKey(),
+                Settings.DEFAULT)) {
+            account.setAci(aci);
+            account.setNumber(number);
+        }
+    }
+
+    private SignalAccount loadAccountOrNull(final String path) {
+        if (!SignalAccount.accountFileExists(directory.toFile(), path)) {
+            return null;
+        }
+        try {
+            return SignalAccount.load(directory.toFile(), path, false, Settings.DEFAULT);
+        } catch (IOException | OverlappingFileLockException e) {
+            return null;
+        }
+    }
 }

src/main/resources/META-INF/native-image/org.asamk/signal-cli/reachability-metadata.json

@@ -4012,7 +4012,8 @@
             "java.lang.String",
             "java.lang.String",
             "java.lang.String",
-            "java.lang.String"
+            "java.lang.String",
+            "java.lang.Boolean"
           ]
         },
         {
@@ -4023,6 +4024,10 @@
           "name": "number",
           "parameterTypes": []
         },
+        {
+          "name": "numberless",
+          "parameterTypes": []
+        },
         {
           "name": "path",
           "parameterTypes": []