Skip to content

Cat 3: apps declare default circles, and connections enrol from them - #1663

Closed
toddmitchell wants to merge 20 commits into
app-registrations-to-tablefrom
app-default-circles
Closed

Cat 3: apps declare default circles, and connections enrol from them#1663
toddmitchell wants to merge 20 commits into
app-registrations-to-tablefrom
app-default-circles

Conversation

@toddmitchell

@toddmitchell toddmitchell commented Aug 21, 2026

Copy link
Copy Markdown
Member

All of Cat 3 — declaration and enrolment, in two commits.

Originally scoped as just the declaration half (3.1–3.5). The enrolment half landed on the same branch and the branch is force-push protected, so the two ship together. They are pointless apart anyway: the pipeline queries GrantOn = Connect and finds nothing until an app declares, and a declaration sits inert until something queries it.

Commit 1 — declaration (3.1–3.5). Stacked on app-registrations-to-table (#1662)circle-definitions-to-table (#1661) → review-stamp-cat1 (#1649).

This is PR (2) of three. PR (1) was the plumbing (#1661, #1662); PR (3) is the enrolment pipeline and the owner toggle.

What an app can now declare

"defaultCircles": [{
  "id": "",                    // app-supplied; what an update matches on
  "name": "Chat-only",
  "driveGrants": [ /* ChatDrive Write|React */ ],
  "grantOn": "connect",         // none | connect | ownFlowConnect | review
  "designation": "personal",
  "emoji": "💬"
}]

It belongs in the registration payload rather than a later call for the two reasons part 2 gives: a grant-on-connect circle enrols ambiently, so the first connection after install must already know about it; and the install consent screen is where the owner agrees to it.

Create-or-update, matched on circle id

Replaying a registration updates the circle rather than duplicating it (3.3). An app may only touch circles it owns — another app's, or an owner circle, is refused with CircleNotOwnedByApp (3015) rather than taken over.

Circles an app stops declaring are deliberately left alone. People may be in them, and a changed registration payload is no reason to silently revoke membership.

The deposit-only invariant (3.5)

Enforced at definition-write time and re-run on every update, so it holds whenever GrantOn changes. Two rules, following the decision that permission keys stay on circles:

A Connect / OwnFlowConnect circle
permission keys rejected — CannotGrantKeysOnAmbientCircle (3013)
read grants rejected — CannotGrantReadOnAmbientCircle (3014)
read on an AllowAnonymousReads drive allowed — a member gains nothing a stranger didn't have

Checked at write, not at grant, for the same confused-deputy reason as part 1's drives-it-can-already-read rule: an app can plant a definition, and the next owner-driven grant would mint it with the master key in scope.

This is what turns "an unreviewed connection holds zero read keys" from a convention into an enforced property.

The [JsonIgnore] fix has moved

This PR originally carried the fix for a bug #1661 introduced — [JsonIgnore] on the promoted circle fields made every client-side circle update silently reset GrantOn. That fix now lives in #1661 itself, so #1661 is safe to land on its own rather than only as part of this stack. It arrives here by merge.

Testing

5 integration tests in AppDefaultCircleTests: declared circles become real rows owned by the app; re-registering updates rather than duplicates; an ambient circle is rejected for permission keys and for a read grant; a Review circle accepts both.

Two of them failed on the first run and are what surfaced the [JsonIgnore] bug.

Full solution suite green: 2,547 passed, 0 failed (68 skipped, all pre-existing [Explicit]).

Commit 2 — enrolment (3.6–3.13) and the system apps declaring

The pipeline. ResolveEnrollmentCirclesAsync runs wherever a connection's circles are resolved: system circles exactly as before, plus every grant-on-connect circle belonging to an app the owner has left enabled. TableCircle.GetByGrantOnAsync hits Idx1Circle (identityId, GrantOn) — that query is the whole reason GrantOn is a column, and until #1661 moved definitions into the table it could not have been written.

The toggle. A Dictionary<Guid,bool> on TenantSettings, set via POST register/connect-enrollment. An app absent from the map counts as enabled — which is exactly the install-consent seeding the spec asks for, and it sidesteps a circular dependency, since TenantConfigService already depends on IAppRegistrationService. Turning an app off affects future connections only.

The declarations. Chat, mail and feed each declare a deposit-only Connect circle in SystemAppConstants, carrying that app's slice of what the system circles hand out today. v15 → v16 backfills existing installs and validates through the pipeline's own query path rather than a direct read.

Transitional by design

The system circles still enrol every connection, so one formed now lands in both and the same drive grants arrive twice. Redundant, not harmful.

It also means "an unreviewed connection holds zero read keys" is still false — the system circles still carry read grants. That becomes true when they retire, which is the next phase. Worth stating plainly rather than leaving implied.

Retiring them is a bigger job than it looks: feed distribution, AllowIntroductions and ShardRecovery all key off the Confirmed circle and have no replacement built, and odin-js still posts the two hardcoded circle GUIDs.

A design bug the new circles exposed

UnreviewConnectionAsync started rejecting every contact, because they all now hold an app default circle. But being in one is not evidence the owner reviewed anyone — it happened automatically. The rule now ignores any circle with GrantOn != None, so only a circle the owner deliberately chose stands in the way. Both directions are pinned by test.

Two existing tests changed

SystemInitializeConfigTests and CircleDefinitionTests asserted exact circle counts. They now assert presence. The set of circles after init genuinely grew by three, so the old assertions were checking a number that no longer means anything.

Still not here

AppRegistrationRequest has no AppSlug field — an app declaring its own address arrives with drive addressing. And 3.7 (an app-context accept bypassing the default set) needs the V2 explicit-accept path from #1599.

🤖 Generated with Claude Code

toddmitchell and others added 6 commits August 20, 2026 19:15
Cat 2 of docs/connection-defaults-checklist.md. Cat 1 gave the review a
record; this makes it mean something.

The 777 tier has always been broken: the evaluator folds Connected and
AutoConnected into one case and no caller is ever stamped AutoConnected, so
a `connected` ACL admits every connection including one the owner has never
looked at. The recut makes the promise true rather than removing the tier,
because "who can see my connections list" wants reviewed people and cannot
be expressed any other way.

- Assignment, not evaluation. ReviewedAt != null -> 777, else 444, at the
  three sites that build a connected caller: transit, guest/YouAuth, home.
  CreateTransitPermissionContextAsync now returns the review state, since it
  was the one site with no ICR to read it from.
- AutoConnected (555) deleted.
- Connected renamed to Reviewed, keeping the serialized value via
  [JsonStringEnumMemberName("connected")]. Enums persist as camelCase
  strings, so without this every stored ACL would be orphaned.
- Caller.IsConnected no longer derives from SecurityLevel. It could not: an
  unreviewed connection now ranks 444, and PeerIncomingDriveUpload/Update
  gate on IsConnected, so deriving it would close the perimeter on exactly
  the caller the deposit path exists for. It becomes a flag set at context
  construction -- the wire and the read audience are different questions.
- IdentityHasPermissionAsync's Connected case now tests IsReviewed() rather
  than IsConnected(). This is a second evaluator the spec's "assignment, not
  evaluation" note does not cover; it drives outbound feed distribution, and
  left alone unreviewed connections would keep receiving content ACL'd to
  the reviewed tier.
- The ConnectedIdentitiesCanView{Connections,WhoIFollow} keys are gated on
  the review instead of granted to any connection. They were implemented as
  permission keys on the Confirmed circle, which is the system already
  treating reviewed as the operative tier without a name for it.

Behavior change, deliberate: existing bare-`connected` ACLs -- today's
"Vetted" profile fields -- become reviewed-only. Unreviewed connections lose
access they were never meant to have. Release-note it.

Tests are not updated in this commit; 29 test files still reference
SecurityGroupType.Connected and the test projects will not build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mechanical only, so the solution builds and the suite can run:

  SecurityGroupType.Connected     -> SecurityGroupType.Reviewed
  SecurityGroupType.AutoConnected -> SecurityGroupType.Reviewed
  AccessControlList.Connected     -> AccessControlList.Reviewed

71 files, 132 lines, no other edits. AutoConnected folds into Reviewed
because the evaluator already treated the two as one case and no caller was
ever stamped with it, so every test using it was really testing the 777
path.

No assertions or expectations were adjusted. Whatever fails now is a real
signal about the ladder recut, not a rename artifact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting it was a data-compatibility break. The value was never assigned to
a caller, but clients do set it on files -- the chat app ACLs messages with
it -- so stored file headers name it, and JsonStringEnumConverter throws on
a string with no matching member. Every existing header carrying
"autoconnected" would fail to deserialize.

The value stays 555 so the enum agrees with the indexed
requiredSecurityGroup column, which already holds 555 for those rows. It is
documented as read-only legacy: no caller is stamped with it, the evaluator
no longer has a case for it, and it retires for real with the system
circles.

Note this is storage compatibility only, not an access change. Those files
were never peer-readable: chat is a deposit medium, and read grants carry
the storage key that a deposit-only connection does not have.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restoring AutoConnected to the enum was not enough: CallerHasPermission had
no case for it, so a legacy 555 file fell through to false and was denied to
every non-owner caller -- a reviewed one included. Before the recut the two
shared a case, so any connected caller passed.

It also disagreed with the index. requiredSecurityGroup BETWEEN 0 AND
callerLevel admits 555 to a 777 caller, so QueryBatch would list the file
and a direct get would then refuse it.

555 now evaluates at the same threshold as 777, which is what it always did.
The intended tightening still applies: an unreviewed caller is 444, so
555 <= 444 fails and they lose access -- correct, and moot in practice for
encrypted files, where a deposit-only connection holds no storage key.

IdentityHasPermissionAsync needs nothing; it never had a 555 case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cat 3.1-3.5. An app names the circles it wants to exist and when their
members are enrolled; the server materialises them as real rows and enforces
the deposit-only invariant when the definition is written.

- AppRegistrationRequest gains DefaultCircles: name, grants, GrantOn,
  Designation, Emoji, keyed by an app-supplied circle id. It belongs in the
  registration payload rather than a later call because a grant-on-connect
  circle enrols ambiently -- the first connection after install must already
  know about it -- and because the install consent screen is where the owner
  agrees to it.
- CreateOrUpdateAppCircleAsync matches on circle id, so replaying a
  registration updates the circle instead of duplicating it. An app may only
  touch circles it owns; another app's, or an owner circle, is refused
  rather than taken over. Circles an app stops declaring are left alone --
  people may be in them, and a changed registration payload is no reason to
  silently revoke membership.
- The declaration is stored on detailsJson, the column part 1 reserved for
  it. Consent display and repair only; the Circle rows are the truth.
- AssertDepositOnlyIfAmbientAsync runs at definition-write time and again on
  every update, so the invariant holds whenever GrantOn changes. A
  Connect/OwnFlowConnect circle may carry no permission keys (3013) and no
  read grant except on drives that already allow anonymous reads (3014).
  Checked at write rather than at grant for the same confused-deputy reason
  the drives-it-can-already-read rule is: an app can plant a definition, and
  the next owner-driven grant would mint it with the master key in scope.

Fixes a bug introduced by moving definitions into the table:
CircleDefinitionControllerBase serves CircleDefinition directly on the wire,
including an update that takes one as the request body -- so [JsonIgnore] on
the four promoted fields meant any client-side circle update sent no GrantOn
and silently reset it. The fields are back on the wire, and the blob copy is
cleared inside ToRecord instead, which is the trick ToConnectionsRecord
already uses for the grant collections. AppId is no longer taken from an
update request either: ownership is set at creation and must not be
reassignable by anyone who can PUT a definition.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…are them

Cat 3.6-3.13 plus the declarations that make them do anything. The two
halves are pointless apart: the pipeline queries GrantOn = Connect and finds
nothing until an app declares, and a declaration sits inert until something
queries it.

The pipeline:

- ResolveEnrollmentCirclesAsync runs wherever a connection's circles are
  resolved. It returns the system circles exactly as before, plus every
  grant-on-connect circle belonging to an app the owner has left enabled.
  This is the frozen union in CircleConstants replaced by one computed at
  connect time from what is actually registered.
- TableCircle.GetByGrantOnAsync hits Idx1Circle (identityId, GrantOn). That
  query is the whole reason GrantOn is a column, and until circle
  definitions moved into the table it could not have been written.
- The owner's per-app toggle lives in TenantSettings as a map. An app absent
  from it counts as enabled, which is exactly the install-consent seeding
  the spec asks for -- and it avoids a circular dependency, since
  TenantConfigService already depends on IAppRegistrationService. Turning an
  app off affects future connections only; identities already enrolled keep
  their grants.

The declarations:

- Chat, mail and feed each declare a deposit-only Connect circle in
  SystemAppConstants, carrying that app's slice of what the system circles
  hand out today. v15 -> v16 backfills installs that predate them, and
  validates through the pipeline's own query path rather than a direct read.

Transitional by design. The system circles still enrol every connection, so
one formed now lands in both and the same drive grants arrive twice --
redundant, not harmful. That also means "an unreviewed connection holds zero
read keys" is still false, because the system circles still carry read
grants. It becomes true when they retire, which is the next phase.

Fixes a bug the new circles exposed: UnreviewConnectionAsync rejected every
contact, because they all now hold an app default circle. Being in one is
not evidence the owner reviewed anyone -- it happened automatically. The
rule now ignores any circle with GrantOn != None, so only a circle the owner
deliberately chose stands in the way.

SystemInitializeConfigTests and CircleDefinitionTests asserted exact circle
counts; they now assert presence. The set of circles after init genuinely
grew by three, so the old assertions were testing a number that is no longer
meaningful.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@toddmitchell toddmitchell changed the title Let apps declare their default circles at registration Cat 3: apps declare default circles, and connections enrol from them Aug 21, 2026
toddmitchell and others added 3 commits August 21, 2026 19:50
#1655 shipped with no tests, and its green suite proved almost nothing:
every test connects by owner-driven accept, so every caller is reviewed and
behaves exactly as before. Nothing exercised an unreviewed one.

Serialization, as unit tests against hardcoded strings. A round-trip cannot
catch a broken [JsonStringEnumMemberName]: the value would serialize as
"reviewed", deserialize back to Reviewed, and look perfectly consistent
while orphaning every ACL written before the rename. Reviewed must emit
"connected"; "autoconnected" must still parse to 555; the numeric slots are
pinned because the DB range query depends on their ordering.

Tier and ACL, over the real peer path: a reviewed connection reads a
connected-ACL file and an unreviewed one does not, the tier assigned over
transit follows the stamp, and review and un-review both take effect at once
rather than when the peer context cache happens to expire.

One of those failed, and the bug is real. Granting a circle stamps the
review -- membership implies review -- but nothing reset the peer's cached
transit context, so the owner performed a deliberate act and the contact
kept reading at the old level for up to an hour. GrantCircleAsync now resets
the cache when the grant is what caused the promotion, which is what
ConfirmConnectionAsync already did for its own path.

Mutation-checked. Reverting the tier assignment to always-777 fails five of
six; re-deriving Caller.IsConnected from SecurityLevel fails the peer-send
test with exactly the message that describes the regression.

That last one is worth recording: the first perimeter test I wrote passed
under that mutation and was therefore worthless -- GetRemoteDotYouContext is
not gated on Caller.IsConnected, the upload controllers are. It was replaced
with a real peer file send.

Still uncovered: the feed-distribution path through IdentityHasPermissionAsync,
and the tenant-settings gating.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/services/Odin.Services/Membership/Circles/CircleDefinition.cs
The two remaining Cat 2 changes had no coverage, and writing the tests found
that one of them was not actually done.

Feed distribution, through IdentityHasPermissionAsync: that evaluator is
keyed on the identity rather than a caller context, so the assignment change
never reached it and it had to start testing IsReviewed(). An unreviewed
follower must not receive a post ACL'd to the reviewed tier, and a reviewed
one still must. Both pinned.

The connections-list keys: this one was broken. Gating
GetAdditionalPermissionKeysForConnectedIdentities on the review closed only
one of two paths -- UpdateTenantSettingsFlag still wrote ReadConnections
onto the Confirmed circle, so anyone in that circle held it regardless of
the tier. Earlier notes called the circle copy redundant but harmless. It
was neither: it is a second source, and it defeated the gate entirely.

The flag no longer writes the key. The method that did is now a one-way
strip, so a tenant that touches either setting is cleaned up, and one that
never does is cleaned by EnsureSystemCirclesExistAsync reconciling the
definition against the constant. No migration, which also avoids colliding
with the version bump on the sibling branch.

SystemCircleUpdatedWhenConnectedFlagChanges asserted the old mechanism
directly -- enable the flag, expect the key on the circle. It had been
passing all along, quietly certifying the hole. Inverted rather than
deleted: it now asserts the flag does not write the key, and says why a copy
there is not redundant. Whether the setting works is covered by the new
distribution tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
toddmitchell and others added 11 commits August 24, 2026 18:42
The review already records the owner's act on the connection registration, so the
auto-to-confirmed swap it was wrapped around has nothing left to do. ReviewConnectionAsync
stamps first and enrols straight away -- no confirm hop, and no master key on the path, so
an auto-connected contact can be reviewed from an app context like any other.

The 3010 lockout goes with it. It existed to force that confirm before an auto-connection
could gain circles; GrantCircleAsync stamps the review as it grants, which makes the grant
itself the owner's act it was standing in for.

Shard readiness follows the same move: confirmed-ness becomes reviewed-ness.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isCallerConfirmed asked whether the caller sat in the Confirmed Connections circle, which
was only ever a stand-in for "the owner reviewed them". The recut ladder assigns Reviewed
from ReviewedAt, so ask the level directly.

Nothing downstream changes: the preflight payload, the log fields and the refusal label all
consume the bool rather than recomputing it, and the two sets coincide on existing data --
v12->v13 stamped every confirmed connection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The review dialog closes when the call returns, so an empty body means the
client can only learn what took effect on a later status read -- by which
point the screen it needed to render is gone.

Three outcomes, not two:

- granted: minted as a live circle grant; the contact holds it now.
- deposited: sealed to the connection's write-only key, awaiting conversion.
  An app holds no master key, so even a circle it owns is deposited rather
  than minted; it becomes real the next time the connection's key store key
  is in scope, via peer CAT auth or the owner's next grant touch.
- pending: recorded but not acted on, because the circle belongs to an app
  whose keys the reviewing client does not hold. Always empty here -- the
  cross-app queue fills it -- but the shape is defined now so the client
  does not have to change when it lands.

The distinction matters: a client showing "done" for a deposit would be
claiming access that does not exist yet, which is the exact failure the
pending state exists to prevent. If a UI has only two states, deposited
belongs with pending, not with granted.

The outcome is read back from the resulting state rather than inferred from
which branch was taken. That an app deposits even its own circles is a
property of GrantCircleAsync, not of the caller, and reading the result
keeps this honest if that changes.

Additive for clients: nothing reads the body today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Friends, Family, Work and Acquaintances existed only because the owner
console's setup wizard created them: an identity that ran it got four
owner circles, and an identity set up any other way got none. The app
that presents them owns them now.

Two halves:

- New identities: RegisterChatAppAsync provisions them alongside the
  chat app, from SystemAppConstants.ChatRelationshipCircles. Created if
  missing and never overwritten -- deliberately not declared as the
  app's DefaultCircles, which a re-registration reapplies and would use
  to reset a circle the owner has since renamed. EnsureBuiltInApps now
  runs before the setup request's own circles, so the wizard's copies at
  the same ids lose the race instead of winning it.

- Existing identities: the v16 -> v17 migration rebinds what is already
  there. Ownership only -- membership, grants, names and GrantOn are the
  owner's and are left alone. It runs whether or not chat is installed,
  so an identity that installs it later cannot hit CircleNotOwnedByApp
  the first time the app tries to manage one.

The ids are md5(name) on both sides, which is what the wizard assigns
(odin-js toGuidId), so both halves land on the same circles and a rename
does not hide one from the migration. A test pins that derivation.

Reassignment needed a door: CircleDefinitionService.UpdateAsync refuses
to touch AppId at all, and CreateOrUpdateAppCircleAsync refuses to take
over a circle it does not already own. SetOwningAppAsync is internal and
does ownership and nothing else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CallerMayIntroduce asked for the AllowIntroductions key, which only the
Confirmed Connections circle grants. That circle is retiring, and an
ambient circle cannot carry a permission key to inherit it -- so the
permission had nowhere to go.

It does not need to go anywhere. The owner reviewing a connection is the
act the key was ever standing in for, and SecurityGroupType.Reviewed is
where that act is recorded, so ask the level. A held key is still
honoured for an owner who put it on a circle of their own making, and
the auto-accept branch is untouched.

Nobody loses anything: v12->v13 stamped every confirmed connection, so
the two sets coincide on existing data. And unlike a grant, a policy
check takes effect for existing connections without re-issuing every
member's stored circle grant under the master key -- the same reasoning
the auto-accept branch already carries.

One consequence, in the tests: revoking the circle no longer withdraws
introductions, because revoking a circle does not un-review anyone.
Withdrawing is un-reviewing. Today that takes both steps, since the
circle still carries a real key; when it retires, the un-review is the
whole of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the Reviewed tier onto the app-circle line: retiring the Confirmed
Connections circle needs both halves in one place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Confirmed Connections circle is the only thing granting
AllowIntroductions; retiring it without this would break introductions
outright.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retiring the circles needs the code to stop branching on them: cat1 is
what removes the 3010 lockout (which reads the Auto circle's grant) and
the confirm hop inside ReviewConnectionAsync.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first circle to use CircleGrantOn.Review, which until now was an enum
value nothing resolved. Stamping ReviewedAt and joining the contact to the
Review-tier circles are the same event; un-reviewing removes them.

Every path that stamps enrols, not just the review endpoint: an owner
accepting a request stamps at accept time, and granting any circle stamps
because putting someone in a circle is itself the owner's act. Wiring only
the endpoint would have left an owner-accepted contact reviewed on the
ladder and missing the grant that reviewing carries.

The circle holds one grant: write on the shard recovery drive. That is
what is left of the Confirmed Connections circle once the apps'
grant-on-connect circles take the chat, mail and feed drives and
introductions become a policy check on the reviewed level. A contact
writes their recovery shards into that drive, and only someone the owner
has actually looked at should be able to. The grant is write-only, so an
ambient circle could legally hold it -- but ambient reaches every
connection, which is broader than the rule being expressed.

A new id rather than a repurposed one: bb2683fa means "every connection I
confirmed" in stored grants, in fifteen test files and in the clients,
and this means something narrower.

Symmetry matters more than it looks. Without revoke-on-unreview the stamp
clears while the grants that membership carries stay live, and
un-reviewed is true of the ladder and false of the keys the contact holds
-- the same split that made withdrawing introductions a two-step
operation.

A chat-only review now joins them to this circle and nothing else, so the
test that asserted it grants nothing at all says that instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Membership is written by id, not resolved through an enrollment tier. The
tier lookup was indirection nothing needed: one circle, one id, and the
code that writes the review stamp writes the membership beside it.

GrantOn goes to None to match, since nothing resolves Review any more --
which means the reviewed circle would otherwise be counted a deliberate
circle by UnreviewConnectionAsync and block the very un-review that is
about to remove it. Excluded there alongside its two siblings.

The v16->v17 upgrade gains the backfill. Stamping and joining are one
event from here on, but every contact reviewed before this was stamped
when no such circle existed; without it they read as reviewed and still
cannot write their recovery shards. Membership only where the stamp
already says so -- it never stamps anyone, and skips contacts already in
the circle, so a re-run is a no-op.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Membership of those two circles was standing in for two facts that now
have real homes: how the connection formed (ConnectionRequestOrigin) and
whether the owner has looked at it (ReviewedAt). Nothing here deletes the
circles; it stops seven places from asking them questions they were never
the right answer to.

An owner-driven request now puts the contact in the reviewed circle at
the same moment it stamps them, from EnsureSystemCircles, where Confirmed
has always been granted. That is the only place the key store key is in
hand: finalization runs on the sender's side as an incoming peer
callback, which holds neither the master key for a live grant nor the
drive permissions a deposit would need.

- IsConfirmedConnection is gone; the two migrations that derived the
  stamp from that circle carry their own frozen id, since they are about
  the circle as it was and should not follow a constant on its way out.
- CallerContext's system-circle flag is removed. Nothing read it.
- Feed distribution asks for connections instead of that circle's
  members, which is what it wanted all along.
- AutoFixConnections no longer skips auto-connected identities: it was
  dodging the 3010 lockout, and that is already gone.
- Chat and mail authorize their own grant-on-connect circles, which carry
  the same drives.
- Introductions read "connected but not reviewed" from the security level
  rather than from Auto-connected membership.

That last one widens the auto-accept branch: it used to reach only
contacts that auto-connected, and now reaches anyone unreviewed,
including someone reviewed and then un-reviewed. Deliberate -- auto-accept
means "I will connect with anyone", so there is nothing left to withhold.
Un-reviewing withdraws introductions only where auto-accept is off, and
both halves are pinned by tests. A refusal now reports the state rather
than its history: nothing distinguishes never-reviewed from
reviewed-then-un-reviewed once the stamp is the only record.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@toddmitchell

Copy link
Copy Markdown
Member Author

Closing with the branch: the connection-defaults line is being rebuilt on top of #1688, which replays the Circle/AppRegistrations table moves onto main. Commits remain reachable locally at a7ded35.

@toddmitchell
toddmitchell deleted the app-default-circles branch August 26, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant