Move circle definitions into the Circle table

docs/connection-defaults-checklist.md

@@ -57,6 +57,7 @@ The evaluator and the `BETWEEN 0 AND callerLevel` range query are untouched.
 | 3.8 | Owner-console per-app toggle + settings storage (existing per-tenant store, no schema) | L36-37 |
 | 3.9 | The toggle is seeded by the install-time registration consent | L136 |
 | 3.10 | Toggling an app off affects future connections only; already-granted identities keep their grants | L39-41 |
+| 3.10a | **Decided 2026-08-21: future connections only.** Installing or enabling an app never enrols existing connections, and no prompt is offered. See *Decisions* below | L235-237, OQ1 |
 | 3.11 | Bulk-revoke as a separate explicit action on the circle's member list | L40-41 |
 | 3.12 | The whole mechanism sits under the existing global auto-accept settings | L41-43 |
 | 3.13 | Apps may also create circle definitions at runtime (feed's per-channel AUDIENCE circle) | L225-228 |
@@ -112,14 +113,48 @@ assuming, since the two give opposite access outcomes.
 
 *(Inference from reading the three passages together — none of the docs flags it as a conflict.)*
 
-## Open questions that become work
+## Decisions
 
-1. Does enabling an app later offer enrollment of *existing* connections (prompt once, default
-   off), or future connections only? — L235-237, L338-341
-2. Do `Review` circles carry permission keys (`AllowIntroductions`, `ReadWhoIFollow`), or do those
-   leave circles and become per-connection settings? — L342-347
+**Existing connections are never back-enrolled (2026-08-21).** Closes open question 1. Installing or
+enabling an app enrols *future* connections only; no one-time prompt is built.
 
-Neither is in a category above because neither is decided.
+The doc had already settled the important half — enrolment is "offered, never automatic"
+(L235-237) — leaving only whether to offer a prompt at all. Future-only wins for the first pass
+because the manual path already exists and costs nothing to build:
+
+- **Owner console → circle detail → Add Members** takes a multi-select of identities and calls
+  `provideGrants` in one action (`odin-js` `templates/Circles/CircleDetails`, "Add Members to
+  {circle}").
+- **Owner console → contact → circle membership dialog** edits one contact's circles via
+  `CircleSelector` (`components/Circles/CircleMembershipDialog`).
+- Both land on `POST circles/add` → `CircleNetworkService.GrantCircleAsync`.
+
+So an owner who wants their existing contacts in a new app's default circle opens that circle and
+adds them. Slower than a prompt, nothing unreachable. The prompt stays easy to add later, once
+someone has actually felt its absence.
+
+One dependency: `GrantCircleAsync` currently throws `CannotGrantAutoConnectedMoreCircles` (3010)
+for an auto-connected identity, which is exactly the population an owner would want to back-enrol.
+Cat 3 deletes that lockout, so the manual path only fully works from this phase onward.
+
+**Permission keys stay on circles (2026-08-21).** Closes open question 2. Identity-wide permission
+keys are carried by `Review` circles; they do not leave circles to become per-connection settings
+toggled at review.
+
+Consequences:
+
+- No new schema, and no new settings surface — permission keys already live in circle definitions.
+- The deposit-only invariant (3.5) still forbids them on `Connect` and `OwnFlowConnect` circles.
+  Keys are only ever mintable at the review, which is where the key ceremony lives.
+- The validator therefore has two rules, not one: a grant-on-connect circle may carry neither read
+  grants nor permission keys.
+- Part 4's review dialog already surfaces these as per-app toggles, so the client shape is unchanged
+  by this decision.
+
+One refinement Cat 2 forces: part 4's proposed default-circle table puts `AllowIntroductions` *and*
+`ReadConnections` / `ReadWhoIFollow` on the Contacts app's `Review` circle. The latter two are now
+driven by the Reviewed tier instead (Cat 2, item 2.5), so putting them on a circle as well would
+give them two sources. Only `AllowIntroductions` should be circle-borne.
 
 ## Not in scope here
 

src/apps/Odin.Hosting/TenantServices.cs

@@ -81,6 +81,7 @@
 using Odin.Services.Configuration.VersionUpgrade.Version10tov11;
 using Odin.Services.Configuration.VersionUpgrade.Version11tov12;
 using Odin.Services.Configuration.VersionUpgrade.Version12tov13;
+using Odin.Services.Configuration.VersionUpgrade.Version13tov14;
 using Odin.Services.Security.Email;
 using Odin.Services.Security.Health;
 using Odin.Services.Security.PasswordRecovery.RecoveryPhrase;
@@ -399,6 +400,7 @@ internal static ContainerBuilder ConfigureTenantServices(
         cb.RegisterType<V10ToV11VersionMigrationService>().InstancePerLifetimeScope();
         cb.RegisterType<V11ToV12VersionMigrationService>().InstancePerLifetimeScope();
         cb.RegisterType<V12ToV13VersionMigrationService>().InstancePerLifetimeScope();
+        cb.RegisterType<V13ToV14VersionMigrationService>().InstancePerLifetimeScope();
 
         cb.RegisterType<VersionUpgradeService>().InstancePerLifetimeScope();
         cb.RegisterType<VersionUpgradeScheduler>().InstancePerLifetimeScope();

src/core/Odin.Core.Storage/Database/Identity/Table/TableCircle.cs

@@ -22,11 +22,36 @@ internal async Task<CircleRecord> GetAsync(Guid circleId)
         return await base.InsertAsync(item);
     }
 
+    internal new async Task<int> UpsertAsync(CircleRecord item)
+    {
+        item.identityId = odinIdentity;
+        return await base.UpsertAsync(item);
+    }
+
     internal async Task<int> DeleteAsync(Guid circleId)
     {
         return await base.DeleteAsync(odinIdentity, circleId);
     }
 
+    /// <summary>
+    /// Every circle for this identity.  Circles number in the tens, so this is a single read rather
+    /// than a paged one -- the paging overload is still there for callers that want it.
+    /// </summary>
+    internal async Task<List<CircleRecord>> GetAllAsync()
+    {
+        var results = new List<CircleRecord>();
+        Guid? cursor = null;
+
+        do
+        {
+            var (page, next) = await PagingByCircleIdAsync(256, cursor);
+            results.AddRange(page);
+            cursor = next;
+        } while (cursor != null);
+
+        return results;
+    }
+
     internal async Task<(List<CircleRecord>, Guid? nextCursor)> PagingByCircleIdAsync(int count, Guid? inCursor)
     {
         return await PagingByCircleIdAsync(count, odinIdentity, inCursor);

src/core/Odin.Core.Storage/Database/Identity/Table/TableCircleCached.cs

@@ -12,6 +12,7 @@ public class TableCircleCached(TableCircle table, IIdentityTransactionalCacheFac
     AbstractTableCaching(cacheFactory, table.GetType().Name, table.GetType().Name)
 {
     private static readonly List<string> PagingByCircleIdTags = ["PagingByCircleId"];
+    private const string CacheKeyAll = "GetAll";
 
     //
 
@@ -40,6 +41,7 @@ private Task InvalidateAsync(Guid circleId)
     {
         return Cache.InvalidateAsync([
             Cache.CreateRemoveByKeyAction(GetCacheKey(circleId)),
+            Cache.CreateRemoveByKeyAction(CacheKeyAll),
             Cache.CreateRemoveByTagsAction(PagingByCircleIdTags)
         ]);
     }
@@ -69,6 +71,28 @@ public async Task<int> InsertAsync(CircleRecord item)
 
     //
 
+    public async Task<int> UpsertAsync(CircleRecord item)
+    {
+        var result = await table.UpsertAsync(item);
+
+        await InvalidateAsync(item);
+
+        return result;
+    }
+
+    //
+
+    public async Task<List<CircleRecord>> GetAllAsync(TimeSpan? ttl = null)
+    {
+        var result = await Cache.GetOrSetListAsync(
+            CacheKeyAll,
+            _ => table.GetAllAsync(),
+            ttl ?? DefaultTtl);
+        return result;
+    }
+
+    //
+
     public async Task<int> DeleteAsync(Guid circleId)
     {
         var result = await table.DeleteAsync(circleId);

src/services/Odin.Services/Configuration/VersionUpgrade/Version13tov14/V13ToV14VersionMigrationService.cs

@@ -0,0 +1,121 @@
+using System;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
+using Odin.Core;
+using Odin.Core.Exceptions;
+using Odin.Core.Storage.Database.Identity;
+using Odin.Core.Storage.Database.Identity.Table;
+using Odin.Core.Storage.Database.Identity.Wrappers;
+using Odin.Services.Base;
+using Odin.Services.Membership.Circles;
+
+namespace Odin.Services.Configuration.VersionUpgrade.Version13tov14
+{
+    /// <summary>
+    /// v13 → v14: moves circle definitions out of the shared key-three-value blob and into the
+    /// <c>Circle</c> table.
+    ///
+    /// <para>
+    /// Definitions were stored as opaque blob rows, which is why <c>AppId</c> and <c>GrantOn</c> could
+    /// never be queried or constrained. The enrollment pipeline has to answer "which circles enrol on
+    /// connect?" on the hot path, and that is a <c>WHERE GrantOn = ?</c> against an indexed column.
+    /// <see cref="CircleDefinitionService"/> now reads and writes the table; this copies what is already
+    /// there so nothing is lost on the way.
+    /// </para>
+    ///
+    /// <para>
+    /// Idempotent, and additive: a definition already present in the table is left alone rather than
+    /// overwritten, so a partial run can be repeated safely. The blob rows are deliberately <b>not</b>
+    /// deleted -- if this turns out to have gone wrong, the source data is still sitting there. Cleaning
+    /// them up is a later, separate job.
+    /// </para>
+    /// </summary>
+    public class V13ToV14VersionMigrationService(
+        ILogger<V13ToV14VersionMigrationService> logger,
+        IdentityDatabase db,
+        TableKeyThreeValueCached tblKeyThreeValue)
+    {
+        // The context and category keys CircleDefinitionService used while definitions lived in the blob.
+        private const string LegacyCircleValueContextKey = "dc1c198c-c280-4b9c-93ce-d417d0a58491";
+
+        private static readonly ThreeKeyValueStorage LegacyCircleStorage =
+            TenantSystemStorage.CreateThreeKeyValueStorage(Guid.Parse(LegacyCircleValueContextKey));
+
+        private static readonly byte[] LegacyCircleDataType =
+            Guid.Parse("2a915ab8-412e-42d8-b157-a123f107f224").ToByteArray();
+
+        public async Task UpgradeAsync(IOdinContext odinContext, CancellationToken cancellationToken)
+        {
+            odinContext.Caller.AssertHasMasterKey();
+            cancellationToken.ThrowIfCancellationRequested();
+
+            var legacy = (await LegacyCircleStorage
+                .GetByCategoryAsync<CircleDefinition>(tblKeyThreeValue, LegacyCircleDataType) ?? []).ToList();
+
+            if (legacy.Count == 0)
+            {
+                logger.LogInformation("v13->v14: no circle definitions in blob storage; nothing to move");
+                return;
+            }
+
+            var copied = 0;
+            var skipped = 0;
+
+            foreach (var definition in legacy)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+
+                if (definition?.Id == null)
+                {
+                    logger.LogWarning("v13->v14: skipping a blob circle definition with no id");
+                    continue;
+                }
+
+                if (await db.CircleCached.GetAsync(definition.Id) != null)
+                {
+                    skipped++;
+                    continue;
+                }
+
+                // The four promoted fields were never in the blob, so they take their defaults here:
+                // AppId null (an owner circle), GrantOn None (manual membership only), Designation
+                // Personal, Emoji null. That is exactly what every pre-existing circle is.
+                await db.CircleCached.UpsertAsync(CircleDefinitionService.ToRecord(definition));
+                copied++;
+
+                logger.LogDebug("v13->v14: moved circle definition [{name}] {id} into the Circle table",
+                    definition.Name, definition.Id);
+            }
+
+            logger.LogInformation(
+                "v13->v14: moved {copied} circle definition(s) into the Circle table; {skipped} already present",
+                copied, skipped);
+        }
+
+        public async Task ValidateUpgradeAsync(IOdinContext odinContext, CancellationToken cancellationToken)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+
+            var legacy = (await LegacyCircleStorage
+                .GetByCategoryAsync<CircleDefinition>(tblKeyThreeValue, LegacyCircleDataType) ?? []).ToList();
+
+            foreach (var definition in legacy)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+
+                if (definition?.Id == null)
+                {
+                    continue;
+                }
+
+                if (await db.CircleCached.GetAsync(definition.Id) == null)
+                {
+                    throw new OdinSystemException(
+                        $"Validation failed: circle definition {definition.Id} is in blob storage but not in the Circle table");
+                }
+            }
+        }
+    }
+}

src/services/Odin.Services/Configuration/VersionUpgrade/VersionUpgradeService.cs

@@ -20,6 +20,7 @@
 using Odin.Services.Configuration.VersionUpgrade.Version10tov11;
 using Odin.Services.Configuration.VersionUpgrade.Version11tov12;
 using Odin.Services.Configuration.VersionUpgrade.Version12tov13;
+using Odin.Services.Configuration.VersionUpgrade.Version13tov14;
 using Odin.Services.Membership.Connections;
 
 namespace Odin.Services.Configuration.VersionUpgrade;
@@ -40,6 +41,7 @@ public class VersionUpgradeService(
     V10ToV11VersionMigrationService v11,
     V11ToV12VersionMigrationService v12,
     V12ToV13VersionMigrationService v13,
+    V13ToV14VersionMigrationService v14,
     IdentityDatabase db,
     OwnerAuthenticationService authService,
     CircleNetworkService circleNetworkService,
@@ -411,6 +413,29 @@ public async Task UpgradeAsync(VersionUpgradeJobData data, CancellationToken can
                 return;
             }
 
+            if (currentVersion == 13)
+            {
+                await using var tx = await db.BeginStackedTransactionAsync(cancellationToken: cancellationToken);
+
+                _isRunning = true;
+                logger.LogInformation(LogTag + " Upgrading from v{currentVersion}", currentVersion);
+
+                await v14.UpgradeAsync(odinContext, cancellationToken);
+
+                await v14.ValidateUpgradeAsync(odinContext, cancellationToken);
+
+                currentVersion = (await tenantConfigService.IncrementVersionAsync()).DataVersionNumber;
+
+                tx.Commit();
+                logger.LogInformation(LogTag + " Upgrading to v{currentVersion} successful", currentVersion);
+            }
+
+            // do this after each version upgrade
+            if (cancellationToken.IsCancellationRequested)
+            {
+                return;
+            }
+
             // Yey! we made it
             await tenantConfigService.DeleteFailureInfo();