Move app registrations into the AppRegistrations table - #1662
Closed
toddmitchell wants to merge 2 commits into
Closed
Move app registrations into the AppRegistrations table#1662toddmitchell wants to merge 2 commits into
toddmitchell wants to merge 2 commits into
Conversation
The other half of the drive-addressing groundwork. The table shipped with part 1's schema work and had no caller; AppRegistrationService still wrote to the shared key-three-value blob, where UNIQUE(identityId, AppSlug) cannot be expressed at all. Since the slug is a wire address other identities resolve against, a best-effort uniqueness check over opaque rows is not good enough. The table also gives Circle.AppId and Drives.AppId a real target. Slugs have to be coined, because no registration has one and the column is NOT NULL: - Known system apps get their obvious name -- chat, feed, mail, photo, owner. These are the addresses drive addressing assumes. - Everything else derives from the registration's display name, the only human-meaningful thing on the record. - The whole set is resolved and checked before anything is written. GenerateAll orders known apps first, so an app called "Chat" cannot take the chat app's address, disambiguates collisions with a numeric suffix, and throws rather than returning a duplicate. The migration re-checks distinctness on top of that. A half-migrated app table with a slug collision is much worse than a migration that refuses to start. - A name that slugifies to nothing falls back to the app id. Unreadable, always available, and better than refusing to migrate. AppId, AppSlug, Name and CorsHostName become columns and are [JsonIgnore]d out of grantJson, so a query on a column cannot disagree with the hydrated object. Everything else still rides the JSON. Registering a new app derives a slug the same way, so registration and migration land on the same value; the request has no slug field yet, that arrives with drive addressing. Updates carry the existing slug forward -- it is immutable, and other identities may already hold it. Dedupe seeds from stored slugs rather than re-derived ones, so an app holding "acme-2" still holds it whatever its name would slugify to today. v14 -> v15 moves the rows. Idempotent and additive: an app already present is skipped so its slug is never reassigned, and the blob rows are left in place as a fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
toddmitchell
added a commit
that referenced
this pull request
Sep 4, 2026
* Move circle definitions into the Circle table
Definitions lived in the shared key-three-value blob, which is why AppId and
GrantOn could never be queried or constrained -- the columns shipped dormant
with the drive-addressing schema work and nothing was ever wired to them.
TableCircle had no caller at all.
Putting the definitions in the table is what makes those columns usable: asking
"which circles enrol on connect?" is a WHERE GrantOn = ? against an indexed
column, not a load-all-deserialize-filter over opaque rows.
- CircleDefinition gains AppId, GrantOn, Designation and Emoji. CircleDefinition
is both the stored shape and the wire shape -- CircleDefinitionControllerBase
serves it directly and takes one as an update body -- so the fields stay on the
wire and the blob copy is cleared inside ToRecord instead, the same
clear-before-serialize trick ToConnectionsRecord uses for the grant
collections. Nothing in the blob can drift from the column, because
deserializing the blob alone yields defaults.
- Equality and GetHashCode account for the four, since EnsureSystemCirclesExist
reconciles definitions by comparing them.
- CircleGrantOn and CircleDesignation are new enums matching the column values.
Every existing circle is None/Personal, so nothing changes behaviour until
something sets them.
- AppId is not taken from an update request. Ownership is set when the circle is
created and must not be reassignable by anyone who can PUT a definition.
- TableCircle grows UpsertAsync and GetAllAsync; TableCircleCached wraps both and
invalidates the all-key alongside the per-circle key.
- CircleDefinitionService moves off ThreeKeyValueStorage onto db.CircleCached
with ToRecord/FromRecord doing the column-vs-blob split.
- v12 -> v13 copies existing definitions across. Idempotent and additive: a
definition already in the table is left alone, so a partial run repeats safely.
The blob rows are deliberately left in place -- if this goes wrong the source
data is still there. Cleaning them up is a separate job.
Tests pin both directions: no promoted value survives into the blob, the
caller's object is intact afterwards, the fields round-trip through the record,
they are visible on the wire, and an update body echoed back does not reset
GrantOn.
No behaviour change. The four fields take their defaults for every existing
circle, which is what they already were.
Squashed from PR #1661 (commits 07952208f, 3ea043d15) onto main; the migration is
renumbered v13->v14 to v12->v13 because the review-stamp work it originally sat
on is not here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Move app registrations into the AppRegistrations table
The other half of the drive-addressing groundwork. The table shipped with the
schema work and had no caller; AppRegistrationService still wrote to the shared
key-three-value blob, where UNIQUE(identityId, AppSlug) cannot be expressed at
all. Since the slug is a wire address other identities resolve against, a
best-effort uniqueness check over opaque rows is not good enough. The table also
gives Circle.AppId and Drives.AppId a real target.
Slugs have to be coined, because no registration has one and the column is
NOT NULL:
- Known system apps get their obvious name -- chat, feed, mail, photo, owner.
These are the addresses drive addressing assumes.
- Everything else derives from the registration's display name, the only
human-meaningful thing on the record.
- The whole set is resolved and checked before anything is written. GenerateAll
orders known apps first, so an app called "Chat" cannot take the chat app's
address, disambiguates collisions with a numeric suffix, and throws rather than
returning a duplicate. The migration re-checks distinctness on top of that. A
half-migrated app table with a slug collision is much worse than a migration
that refuses to start.
- A name that slugifies to nothing falls back to the app id. Unreadable, always
available, and better than refusing to migrate.
AppId, AppSlug, Name and CorsHostName become columns and are [JsonIgnore]d out of
grantJson, so a query on a column cannot disagree with the hydrated object.
Everything else still rides the JSON.
Registering a new app derives a slug the same way, so registration and migration
land on the same value; the request has no slug field yet, that arrives with
drive addressing. Updates carry the existing slug forward -- it is immutable, and
other identities may already hold it. Dedupe seeds from stored slugs rather than
re-derived ones, so an app holding "acme-2" still holds it whatever its name
would slugify to today.
The rows move in v12 -> v13, the same version step that moves the circle
definitions -- both are the same job, they ship together, and a tenant is either
on the tables or on the blob. Idempotent and additive: an app already present is
skipped so its slug is never reassigned, and the blob rows are left in place as a
fallback.
Cherry-picked from PR #1662 onto main; its v14 -> v15 migration is folded into
this branch's v12 -> v13 rather than burning a second version number.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Make the promoted circle columns settable, and queryable by GrantOn
The columns landed with nowhere to be set from and nothing to read them, which
left the tables moved but not actually in use. Both halves are lifted from the
Cat 3 branch, and both are inert until something declares a circle that enrols.
- TableCircle.GetByGrantOnAsync is a WHERE GrantOn = ? against Idx1Circle. That
query is the entire reason GrantOn is a column, and until the definitions moved
into the table it could not have been written. TableCircleCached caches it under
a ByGrantOn tag and invalidates that tag alongside the per-circle and all keys.
No caller yet -- the auto-connect pipeline is Cat 3.
- CreateCircleRequest gains AppId, GrantOn, Designation and Emoji, and the create
path writes them. Until now nothing could set AppId at all: create ignored it
and update refuses it by design, so an app-owned circle was unrepresentable.
Omitting all four yields null/None/Personal/null, which is what every existing
circle already is.
- AssertDepositOnlyIfAmbientAsync enforces the invariant the moment GrantOn
becomes settable: a circle that enrols without the owner present may hand out
write/react and read on already-anonymous drives, and nothing else. Checked at
definition-write time rather than grant-mint time, because an app can plant a
definition and the next owner-driven grant would mint it with the master key in
scope. Run on create and on every update, since an update is how a circle
becomes ambient.
Error codes 3013 and 3014 are new. 3010 is untouched here -- retiring it belongs
to the review work.
Tests pin the guard before there is a caller that can trip it: an ambient circle
carrying a permission key is refused, and a manual-membership circle carrying the
same key is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Build clean under --warnaserror
CI builds with --warnaserror; my local runs did not, so five warnings I read
past were six hard errors there. None of them were noise.
- TableCircle captured the primary-constructor parameter it also passes to the
base (CS9107). TableCircleMember already keeps an explicit field for exactly
this; TableCircle does the same now.
- RevokeApp/RemoveAppRevocation passed a possibly-null registration to SaveAsync
(CS8604). Under the old blob store that wrote a row whose payload was the
literal "null"; ToRecord would throw instead. Neither is wanted, so a missing
app now returns early, which is what the null check was always shaped like.
- FromRecord dereferenced a deserialize that can return null (CS8602). An
unreadable grantJson is a corrupt row, so it throws with the app id rather
than a bare NullReferenceException.
- Two tests dereferenced a nullable deserialize.
Verified with the same command the sqlite/debug job runs: 0 warnings, 0 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Drop the global UNIQUE on Circle.circleId
Regenerated CRUD and migration from the generator.
Circle declared circleId BYTEA NOT NULL UNIQUE alongside the correct
UNIQUE(identityId, circleId). The column-level constraint is global, and system
circle ids are fixed constants shared by every identity, so on Postgres -- where
all tenants share one database -- the second identity to run
config/system/initialize collided with the first:
23505: duplicate key value violates unique constraint
circlemigrationsv202608040942_circleid_key
The constraint dates to Postgres support (#854) and has been harmless until now
only because TableCircle had no caller: the table was empty everywhere. This
branch is the first code to write to it. SQLite never sees it because each tenant
gets its own file, which is why sqlite/debug and sqlite/release both passed while
postgres/release failed 337 tests.
Drives is the model: DriveId carries no column-level UNIQUE, only
UNIQUE(identityId, DriveId).
v202608261644 rebuilds the table without it, keeping the composite unique and
both indexes. Existing deployments already carry the constraint, so the DDL edit
alone would not have reached them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Create AppRegistrations on databases that never got it
Regenerated from the generator, with UpAsync/DownAsync hand-edited.
AppRegistrations shipped with the drive-addressing DDL carrying a version-0
migration only. The identity database keeps one migration version, and going up
the migrator runs only the groups above it -- so version 0 was unreachable on
every database already past it, and the table was simply never created there.
Fresh databases start at -1, run the version-0 group, and get it; which is every
CI database, so all three legs stayed green while a real deployment threw
42P01: relation "appregistrations" does not exist
as soon as AppRegistrationService queried it. main got away with it because
nothing read the table; this branch is its first reader.
v202608271000 restamps it above every released version, so the migrator reaches
it. Two populations have to arrive there, hence the branch in UpAsync:
- Table absent: create at this version and rename into place. No CopyDataAsync
and no rename of a table that is not there.
- Table present at version 0: the generated rebuild. Restamping requires it --
on SQLite the version marker lives inside the stored CREATE TABLE text.
DownAsync mirrors it: with no AppRegistrationsMigrationsV0 to restore, Up must
have created the table outright, so undoing means dropping it.
Uses cn.TableExistsAsync rather than GetTableVersionAsync, which on Postgres goes
through obj_description('AppRegistrations'::regclass) and throws 42P01 on a
missing relation before it can report anything.
The generator needs the same branch for any table introduced after the first
release, or the next regen reverts this. Noted in the file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Set the upgrade run state the way main now does
main's #1686 moved the flag off a private _isRunning field onto the injected
VersionUpgradeRunState. The v12 -> v13 step this branch adds was still assigning
the field, which merges cleanly and then does not compile.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Read the legacy app blob with the shape it was written in
The v12 -> v13 app move read nothing, silently. It deserialized blob rows into
AppRegistration, which [JsonIgnore]s AppId, AppSlug, Name and CorsHostName --
correct for writing, since those are columns now and a second copy in grantJson
could drift from them, and fatal for reading the blob, where that JSON is the
only place the values exist.
Every legacy row came back with a null AppId, the Where filtered the lot, and the
migration logged "no app registrations in blob storage; nothing to move" and
committed. The tenant reached v13 with an empty AppRegistrations table while the
blob still held every app -- and AppRegistrationService reads the table only, so
the identity presents as having no apps at all. Seen on the demo box.
The migration now holds LegacyAppRegistration: the blob shape frozen as it was
before the columns were promoted. A migration reads history, so it owns a copy of
the shape history was written in rather than borrowing a type that has moved on --
the same reasoning as the frozen context and category keys above it.
Note the asymmetry that caused this: CircleDefinition solves the same
blob-versus-column problem by clearing the fields inside ToRecord and keeping them
serializable, so the circle half of this migration was never affected.
Tests seed a legacy row in the old JSON shape and assert the app lands with its
real name, its CorsHostName and a slug derived from that name -- a slug of the app
id would mean Name came back null. Plus idempotency: a second pass must not mint a
new slug, since it is an address other identities may already hold.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Create the shared drive once instead of racing 20 threads to make it
Overwrite_Encrypted_PayloadManyTimes_Concurrently_MultipleThreads runs 20
threads against one target drive, and each thread called CreateDrive for that
same drive from inside PrepareEncryptedFile. Only one create can win; a loser's
first upload could land before the winner's drive was visible, so the setup
assertion on IsSuccessStatusCode failed. Seen on the ubuntu/postgres job of run
33076953471 (3 of 20 threads), where sqlite and windows passed on the same
commit.
Create the drive once in the test method, before the threads start. The
concurrency under test - 20 threads overwriting their own file 50 times - is
unchanged.
Also make the two counters Interlocked: all 20 threads increment them and the
final assertion reads them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Wire the drive addressing columns through
AppId, DriveSlug and DriveTypeSlug shipped with the drive-addressing DDL and were
invisible above the SQL layer: DriveManager.ToRecord never wrote them,
ToStorageDriveData never read them, and StorageDrive had nowhere to put them. The
columns could not be populated even by hand.
- StorageDriveData and StorageDrive gain the three. They stay out of
StorageDriveDetails on purpose -- UNIQUE(identityId, AppId, DriveSlug)
constrains the columns, and a copy inside detailsJson could disagree with what
the constraint is enforcing. Same discipline the circle work used.
- ToRecord writes them, ToStorageDriveData reads them, and the create path
persists what the request carried.
- CreateDriveRequest accepts them, all optional. Omitting them leaves a drive
addressed by Guid exactly as before, which is every drive today.
- OwnerClientDriveData carries them, so a client can read what a drive holds.
- OdinSlug validates the format from docs/drive-addressing.md: lowercase, digits,
internal hyphens, 1-12 characters. Validate and reject, never coerce -- the
value ends up in other identities' URLs, so lowercasing 'Chat' would hand back
an address the caller did not ask for. The reserved-segment list is empty and
deliberately present: /apps roots the slug tree so neither position has a
literal sibling today, and it must grow when one appears.
WriteOnlyKeyPair is deliberately not plumbed. It is key material for write-only
deposits with escrow, rotation and deleted-drive questions still open in the doc,
and it must never reach a client shape.
Nothing derives a slug or assigns ownership. Every drive still carries null for
all three until the mapping is settled; this only makes the columns reachable.
Tests: the columns round-trip through TableDrives and null stays null; the slug
rule accepts what the doc allows and rejects encoding, path-separator, case and
length violations.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add the addressing go-live checklist
Derived working checklist across the two table moves, the chat circle ownership
change and the drive addressing columns: what is done, what is blocked on a
decision only Todd can make, what backfill and enforcement is owed, and the
deploy-safety items the demo box taught us.
Follows docs/connection-defaults-checklist.md: a derived list, not a spec. The
two design docs remain the source of truth.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Carry the drive addressing fields on the app-route drive shape
OwnerClientDriveData got AppId, DriveSlug and DriveTypeSlug; ClientDriveData did
not, so a client reading drives over the app or V2 route could not see them.
- V2DriveMetadataController maps all three.
- ClientTokenDriveMetadataController redacts them for third parties exactly as it
already redacts Name and Attributes. The slug is designed to be a
remote-resolvable address, but resolution happens on the recipient side
(drive-addressing.md, "Slugs are resolved by the recipient"), so a guest does
not need the list to use one. One-line change if we decide otherwise.
The peer route is deliberately untouched: PeerQueryControllerBase maps
PerimeterDriveData, the cross-identity wire shape, which carries only TargetDrive
and Attributes. Publishing slugs to another identity is a separate decision about
what an identity discloses, not a mapping change.
Null on every drive today, so nothing changes for any caller.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let an app ask for its own slug at registration
drive-addressing.md has the app choosing its slug: it is a package name, not a
role -- "a second chat implementation does not get to call itself chat; it picks
its own slug (chatty)" -- and "registration is first-come", which only makes
sense if the app is asking and can be refused. The field never existed; the
server coined one from the display name because pre-existing registrations had
none and the column is NOT NULL. That was the migration's stopgap, not the design.
AppRegistrationRequest.AppSlug is optional and unenforced:
- Omitted: derived from Name exactly as before, so nothing that registers today
starts failing.
- Supplied: validated for format and taken verbatim, or refused. Never quietly
replaced with a derived one -- it is an address other identities resolve
against, and handing back a different one is worse than saying no.
- Already held by another app: refused with a clear client error rather than a
UNIQUE(identityId, AppSlug) constraint violation from the database.
Immutability is unchanged: updates carry the stored slug forward, and no update
request carries a slug field at all.
Tests cover all four paths. Fixed my own invented expectation while writing them:
"Acme Receipts" derives to "acme-receipt", not "acme-receipts" -- the
generator truncates at the 12-character cap. The migration test written earlier
had the same wrong value and had never run, since port 4444 was busy; it would
have failed in CI.
Not done here: protecting the system slugs (chat, mail, feed, photo, owner) from
a caller claiming them. AppSlugGenerator orders known apps first when deriving,
but nothing stops a supplied slug taking one on an identity where that app is not
yet registered -- checklist 3.9.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Update the checklist for the app slug work
3.1 is done: AppRegistrationRequest carries an optional AppSlug, validated and
taken verbatim or refused. Records what shipped (0.10, 0.11), and the two things
that decision leaves behind:
- 3.3a: slug derivation now lives in both the registration service and the
migration, deliberately, rather than making the column nullable.
- 3.9 changes character. It used to be theoretical; a caller can now supply
'chat' on an identity where the chat app is not yet registered and take it
first-come. Same question as drive-addressing.md OQ2.
Plus 3.3b, the 12-character truncation, and 7.5 for clients that want to name
themselves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give every drive an owning app and a slug
The drive addressing columns have been in place but nothing ever put a value
in them, so every drive carried a null AppId, DriveSlug and DriveTypeSlug.
This fills them in from a fixed mapping.
Move the thirteen app-owned drives out of SystemDriveConstants into
WellKnownAppDrives, leaving only TransientTempDrive behind. Add CommunityDrive
there too. The rename is mechanical -- same namespace, so it is an identifier
swap the compiler checks -- and touches 451 call sites.
Coin app ids for the apps that own a drive but have no registration yet:
Community, Contacts, Email, HomePage, Lists, Location, Moments, Recovery,
Vault, and a System app for the transient drive. An app id is permanent, since
it is what Drives.AppId points at and what a drive slug is unique within.
DriveSlugGenerator mirrors AppSlugGenerator: fixed slugs for the known drives,
derived from the drive name otherwise, whole set resolved up front. It is
wired into drive creation as a fallback -- a caller-supplied slug is taken
verbatim or refused, never quietly replaced.
Two details worth calling out:
Slugs are deduped per owning app, not per identity. The constraint is
UNIQUE(identityId, AppId, DriveSlug), so feed/news and chat/news may coexist;
deduping identity-wide would hand the second one "news-2", a permanent address
nobody asked for, for a collision the schema allows.
Nothing is derived for a drive with no owning app. AppId and DriveSlug are set
together or both NULL: NULLs are distinct in a unique index in both dialects,
so a slug on an AppId-less row is unconstrained and two drives could claim it.
Raise OdinSlug.MaxLength from 12 to 14 so "shard-recovery" fits. The database
caps these at 64, so there is room. Existing tests that asserted 12-character
truncation are updated to the new cap.
Whitespace-only is now treated as "not set" for app and drive slugs alike.
Clients serialize an unset field as "" or " " routinely, and the three
spellings had diverged: null and "" derived a slug while " " failed
validation and threw. A value with real content is still validated and
rejected, so " chat " is an error rather than being trimmed to "chat".
Seeding is deliberately unchanged: EnsureSystemDrivesExist still creates all
fourteen drives, now via WellKnownAppDrives. Which of them a new identity
should get is a separate decision.
Known gaps, to be addressed next:
- Profile, Wallet and HomePageConfig share one drive type but are given the
type slugs profile, wallet and profile; Moments and Lists share a type
across two different apps. Both break the one-slug-per-type and
one-app-per-type rules in docs/drive-addressing.md. Type slugs are keyed by
drive alias for now so the mapping is stored as given rather than resolved
by guesswork.
- Photo Library and Vault are named by the mapping but have no Guids yet.
- The WellKnownAppDrives header still says its drives are absent from
SystemDrives and never server-created. Both are untrue until seeding is
settled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Register the built-in apps and their circles at identity setup
Every drive already had an owning app; this gives every app its circles, and
decides which apps a new identity starts with.
Nine apps are built-in and get registered: Chat, Contacts, Email, Feed,
HomePage, Location, Mail, Recovery and System. Contacts, Email, HomePage,
Location, Recovery and System had no registration before -- each is granted
ReadWrite on the drives it owns, with no permission keys and no authorized
circles, since none were specified. The three near-identical Register*App
helpers collapse into one RegisterAppIfNotExistsAsync.
WellKnownAppCircles holds the sixteen app-owned circles. Ten belong to
built-in apps and are provisioned by EnsureBuiltInAppCirclesExistAsync; the
other six arrive only with their app. Friends, Family, Work and Acquaintances
are in code for the first time -- until now the owner console's setup wizard
created them client-side. The two system circles are deliberately untouched:
they are retired in a later step, and until then they still have to work.
Chat is the only GrantOn=Connect circle, so it is granted ambiently to
auto-connections with no owner review. It carries write/react only, which is
what the deposit-only invariant requires.
Seeded drives and SystemDrives are now the same set, which is the point of
this change and not a coincidence: SystemDrives is what makes a drive immutable
(DriveManager refuses to rename, re-mode or archive anything in it), so a
seeded drive missing from it is one the owner can archive out from under the
system. WalletDrive leaves both -- Vault is not built-in -- and EmailAppDrive
joins both, because Email is built-in and its registration is granted the
drive, and a grant cannot be issued for a drive that does not exist.
ListsDrive and MomentsDrive are seeded even though Lists and Moments are not
built-in. The system circles grant them, and issuing those grants throws if the
drive is absent. Both go when those circles do.
Move ChannelDriveType into WellKnownAppDrives to break a static-initializer
cycle. SystemDrives lists drives declared in WellKnownAppDrives, and
PublicPostsChannelDrive read SystemDriveConstants.ChannelDriveType, so touching
WellKnownAppDrives first ran SystemDriveConstants mid-initialization and built
SystemDrives out of fields that were still null. It resolved by declaration
order until adding EmailAppDrive to the list changed which type was touched
first; the failure is a NullReferenceException far from the cause, never an
error at the source. WellKnownAppDrives now reads nothing from
SystemDriveConstants, so the dependency runs one way.
WellKnownAppDrivesTests guarded that EmailAppDrive was never auto-created.
That stopped being true, and the test kept passing because it only checked list
membership. It now guards what replaced it: anything seeded must be immutable.
Not done: the conversion for the six apps that are not built-in, which has to
stamp ownership and slugs onto what existing identities already hold.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Declare the default system as one tree
Six lists currently state which app owns which drives and circles, all keyed by
app id and never joined: EnsureSystemDrivesExist, EnsureBuiltInApps,
SystemDrives, BuiltInAppIds, the slug tables in DriveSlugGenerator, and the
circle constants. Answering "what is Chat?" means grepping four files, and
nothing stops two of those lists disagreeing -- which is how the seeded drives
and SystemDrives drifted apart earlier on this branch.
Odin.Services.Apps.Builtin declares it once instead:
SystemApp.cs the SystemApp and AppDriveGrant records
BuiltinDrives.cs 18 drives -- identity, address, settings
BuiltinCircles.cs 17 circles -- id, grants, GrantOn
BuiltinAppDriveGrants.cs 25 cross-app grants, flat
BuiltinApps.cs 15 apps, and the projections over them
Ownership is a tree, so drives and circles nest under the app that owns them.
Grants are not: eleven of the eighteen supplied rows cross app boundaries --
Chat holds ReadWrite on ContactDrive, which Contacts owns -- so nesting them
would mean one app's node referencing another's, which is the static
initializer cycle that already bit us once. They sit in a flat sibling list
that references the drive constants directly.
Circle drive-grants are the opposite and do nest: every circle grants only
drives its own app owns, with no exceptions. The two system circles are the one
thing that does not fit -- owned by no app, granting across six drives -- so
they stay in SystemCircleConstants until they retire.
Nothing reads any of this yet. The values are copied, not moved, and
SystemDriveConstants, WellKnownAppCircles and BuiltInCircleConstants are still
what runs. Both copies were diffed field by field: drive settings match on
name, anonymous reads, owner-only, subscriptions, CDN, target drive and app id;
circles match on id, owning app, GrantOn and drive grants.
Slugs are stated here rather than derived. Each drive carries its own, so the
36 entries in DriveSlugGenerator's lookup tables stop being a second place for
them to live. Apps carry one too: only five were fixed before, so ten would
have been derived from their display name, and the one for the app formerly
called Owner still read "owner" after the rename.
Name, AppSlug and Permissions have no reader yet. They exist to build an
AppRegistrationRequest, which cannot be derived until AuthorizedCircles has
somewhere to point -- today Chat and Mail aim theirs at the system circles.
Verified against the four supplied mappings: 15 apps and their built-in flags,
18 drives with owning app and both slugs, 17 circles with owning app and
GrantOn, and 18 drive grants.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Provision an identity from the app tree
BuiltinProvisioner reads the tree and creates what an identity starts with, so
the set provisioned is a projection rather than a list restated in
TenantConfigService. EnsureInitialOwnerSetupAsync calls EnsureAllAsync; the
version ladder keeps calling EnsureDrivesAsync on its own, since
VersionUpgradeService does one up-front pass so migrations can assume every
drive exists. TenantConfigService keeps EnsureSystemDrivesExist and
EnsureBuiltInApps as forwarders so those callers are untouched.
Verified the same 14 drives, 9 apps and 13 circles as before -- diffed against
the previous commit, not assumed.
Provisioning order is drives, then circles, then apps, which is a change.
Circles used to come first, and that made a check misreport: the deposit-only
guard runs whether or not validation is skipped, and for an ambient circle with
a read grant it reads the drive to see if it allows anonymous reads. With no
drive there yet the lookup returned nothing and the error blamed the read grant
rather than the ordering. Nothing needed circles first -- HandleDriveAdded only
touches the two system circles, and those are created by the caller before the
provisioner runs.
Within drives, non-anonymous first. Creating an anonymous-read drive makes
HandleDriveAdded grant read on it to the system circles, so every drive those
circles already grant has to exist by then. All six are non-anonymous, so
ordering on that flag satisfies the constraint by construction. It used to be a
comment asking the next person to keep ListsDrive above the anonymous ones.
The tree now separates the two groups instead of flagging them: Builtin is the
nine an identity is configured with, Wellknown the six that arrive only when
the owner installs them. The BuiltIn property is gone -- list membership is the
fact, and holding both invites an app in one list claiming the other.
SystemAppConstants.BuiltInAppIds and IsBuiltInApp are deleted for the same
reason; their only caller moved into the provisioner.
Three of the six own a drive every identity already has, seeded long before
ownership existed: ListsDrive, MomentsDrive and WalletDrive. Those need
stamping by the conversion, which is still to write. The other three own
nothing that exists.
Also record why drive creation must not check that the owning app exists. It
does not today, but only by omission, and the dependency runs the other way: a
registration is granted drives, and a grant cannot be issued for a drive that
is absent. Validating the app at drive creation would make the two constraints
unsatisfiable for every built-in app.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Bring existing identities onto the app tree
The addressing columns landed several commits ago and nothing ever filled them,
so on a tenant that predates this every drive carries AppId, DriveSlug and
DriveTypeSlug as NULL, and the circles the owner console's setup wizard created
carry no AppId. v13 -> v14 fills them in from BuiltinApps.
Two halves, and they are different jobs. Stamping fills rows that already
exist. Provisioning then creates what is missing, and is the same
BuiltinProvisioner a new identity gets -- so an upgraded identity converges on
what a fresh one has, rather than the two paths drifting. Stamping runs first,
so provisioning sees a drive that is already owned rather than trying to create
one that is there.
It walks the whole tree, not just the built-in apps. ListsDrive, MomentsDrive
and WalletDrive belong to apps that are not built-in, yet sit on every identity
because they were seeded long before ownership existed. Those three are the
reason the Wellknown list is not simply inert.
Additive throughout: anything that already has an owner is skipped rather than
reassigned, so a partial run repeats safely and a value set by hand is never
overwritten. Nothing is deleted -- WalletDrive stops being seeded for new
identities, but the ones that have it keep it, stamped like the rest.
This needs two setters that deliberately did not exist. Ownership is not
reassignable through the normal write paths -- CircleDefinitionService.UpdateAsync
refuses to take AppId from a request precisely so nobody who can PUT a
definition can hand a circle to an app, and nothing updates a drive's slug
because it is a wire address other identities resolve against. Both new methods
are internal, refuse an item that already has an owner, and exist only to give
a row the values it would have been created with today. The drive one also
skips the system-drive guard every other setter has: all fourteen are system
drives, so guarding would make it useless for its one job.
Version.DataVersionNumber goes to 14. That constant is the gate --
RequiresUpgradeAsync compares against it -- so without the bump the rung would
exist and never run.
Supersedes #1691, now closed. That branch numbered a different v13 -> v14 which
gave the relationship circles to chat; the mapping puts them under Contacts, so
it was contradicted rather than merely renumbered. It was never deployed, so no
identity is recorded at a v14 that meant something else.
Untested. Nothing here has been exercised: the migration path is hosting
integration territory, and the checklist already notes that every migration
test starts from an empty database, which is the case that works.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Move the drive constants into the tree
The 15 CreateDriveRequest constants were duplicated by BuiltinDrives and read
by nothing but one test file, so they are deleted and AllowCdnTests points at
the tree instead. That last part matters more than the line count: those tests
pin CDN settings, and aimed at the copy the provisioner no longer uses they
would have kept passing while the real values drifted.
SystemDrives becomes BuiltinDrives.Protected. It was never a list of system
drives -- ListsDrive and MomentsDrive are in it and belong to apps that are not
even built-in, provisioned only because the system circles grant them and a
grant for an absent drive throws. What the list actually decides is whether the
owner may rename, re-mode or archive a drive, which is its only use: three
guards in DriveManager and the flag the owner console renders. It is named for
that now, and says out loud that protected means "we provisioned it" rather
than "it is systemic".
Verified the new list identical to the old, same entries in the same order,
before repointing any of the twelve usages.
SystemDriveConstants drops from 218 lines to 33: the transient drive's
identity, which still has around ninety references, and a forwarder for the
channel type. Moving those two would retire the file, but it is a couple of
hundred call sites and better done deliberately.
WellKnownAppCircles is gone, deleted separately; BuiltinCircles is now the only
declaration of the app-owned circles, and its comment no longer claims
otherwise.
Protected is still hand-listed rather than derived from what is actually
provisioned. The two drifted once already -- WalletDrive left the seeded set
and EmailAppDrive joined it, and neither was reflected -- so deriving it is the
real fix. It needs the provisioned set named first, which is currently computed
inline in EnsureDrivesAsync, and the static initializer cycle between these
types has bitten once, so that is worth doing on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Make the upgrade enforce the app tree, not just fill it in
The tree is the source of truth for the drives, circles and apps it declares,
so the upgrade now makes them match rather than filling in whatever is empty.
Each stamp used to skip anything that already had a value, which meant a row
written by an earlier build kept that value forever.
That is not hypothetical. An earlier branch briefly gave the relationship
circles to chat before the mapping put them under Contacts, and any identity
that ran it holds Chat-owned Friends, Family, Work and Acquaintances that a
fill-only stamp would never correct. Same shape for app slugs: registrations
built before the tree was authoritative derived the slug from the display name,
so "Homebase - Location" was registered as homebase-locat -- and a slug is
immutable through every normal path, since other identities resolve against it,
so nothing else would ever fix it.
Three methods, all internal and migration-only, all returning whether they
changed anything so the log records corrections rather than visits:
ApplyTreeAddressAsync drive AppId, DriveSlug, DriveTypeSlug
ApplyTreeDefinitionAsync circle AppId, GrantOn, Designation
ApplyTreeSlugAsync app AppSlug -- new; there was no way in at all
Each one is an exception to a rule that exists for a reason. UpdateAsync
refuses to take AppId from a request so that nobody who can PUT a definition
can hand a circle to an app; an update carries the stored app slug forward
because it is a wire address. The remarks on each say why the exception is
warranted, so the rule is not quietly weakened.
Validation is stronger to match: it used to check a drive or circle had an
owner, and now checks the value equals what the tree says. A correction that
silently fails now fails the upgrade instead of recording success.
The app slug path checks uniqueness before writing, since UNIQUE(identityId,
AppSlug) would otherwise surface as a constraint violation. It throws naming
the conflicting app. One limitation: two apps that need to swap slugs cannot,
because whichever is corrected second still finds the first holding its target.
No mapping we have does that, and failing loudly beats half-applying a rename.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give user-created channel drives an owner and an address
A user makes channel drives at will, so there are arbitrarily many and none has
a fixed alias -- they are the one kind of drive the tree cannot list, and the
reason DriveSlugGenerator keeps its derivation at all. The upgrade now hands
them to the feed app with a slug derived from the drive name and a type slug of
"channel".
Fills rather than corrects, unlike the drives the tree declares. The tree is
authoritative for what it declares and it does not declare these: their slug
comes from a name the owner chose and may since have changed, so re-deriving on
every upgrade would move an address other identities resolve against. A channel
drive that already has one is left alone.
Ordering is load-bearing. It runs after the tree drives are stamped, so
FeedDrive and PublicPostsChannelDrive already hold "feed" and "posts" in
storage and the taken set read back is complete -- otherwise a channel named
Posts could take "posts" first. Uniqueness is scoped to the feed app, matching
UNIQUE(identityId, AppId, DriveSlug), and the taken set accumulates inside the
loop so two channels named News become news and news-2.
The type slug is stated rather than looked up. These drives came from a query
on ChannelDriveType so the type is already known, and TypeSlugFor can return
null -- which AssertValidOrNull permits, so a miss would have been stored
silently. Validation now checks the type slug too; it previously asserted only
the owning app and slug, so a null would have survived the upgrade unnoticed.
That matters because DriveTypeSlug is what ?type=channel will filter on once
those routes exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let the tree name every app's slug, not a second list
KnownAppSlugs listed five apps by hand while the tree named far more, so any
app it missed had its slug coined from the display name instead: "Homebase -
Recovery" became homebase-recov where the tree said recovery. The v13->v14
stamp then had to correct an address other identities may already hold.
Reading BuiltinApps.All removes the second list rather than lengthening it.
The tree is validated as it is indexed -- a duplicate app id or a malformed
slug is a mistake worth naming before it reaches a registration.
This settles one disagreement in the tree's favour: the System app derives as
'system' now, not 'owner', which is what the v14 stamp was already forcing.
EveryAppInTheTreeGetsTheSlugTheTreeNames is the test that would have caught
this, and cannot drift as the tree grows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give Emergency Location Access one definition and one creator
The circle was declared twice. EnsureBuiltInCirclesExistAsync created it from
BuiltInCircleConstants' own copy, whose CreateCircleRequest left out AppId,
GrantOn and Designation -- and because that runs before provisioning in
EnsureInitialOwnerSetupAsync, EnsureCircleExistsAsync then found the circle
present and skipped it. Every identity ended up with the unowned row, new ones
included, so the owner console showed the circle as belonging to no app while
the tree named Location as its owner.
UpdateAsync will not set AppId by design, so the reconcile branch could not
repair it either: it rewrote everything except the field that was wrong.
The definition now lives only in the tree and the row is only ever written by
EnsureCircleExistsAsync, which carries all three fields. v9 is where this
circle arrives, so that migration asks for it by name instead of relying on a
side effect of reconciling the system circles.
Existing identities still need the v13->v14 stamp, which corrects rather than
fills and already covers this circle.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add the Webdrop app and give every remaining drive a slug
Webdrop joins the tree as a built-in: its app id, drive, drive slug, circle and
registration request, and provisioning registers it like the rest.
The v13->v14 upgrade also gains a final pass over drives nothing declares --
created through the owner console or the setup wizard's own request.Drives, so
neither the tree nor the channel query reaches them. They would otherwise come
out of the upgrade with no address. Slug only: the owning app stays as it was,
which means the unique index cannot police these rows, so the pass dedupes them
itself and validation fails the upgrade if any drive is left without a slug or
two unowned drives share one.
ApplyTreeAddressAsync becomes ApplyAddressAsync, with a nullable app id and type
slug, since it now serves drives that have neither.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Webdrop is known, not shipped with every identity
It was in the built-in list, so provisioning created its drive, its circle and
its registration on every identity that upgraded -- and the drive allows
anonymous reads, which makes HandleDriveAdded grant read on it to both system
circles. A new public drive on every identity is a product decision, not a
side effect of a tree entry.
Wellknown is the list for this, and Lists, Moments and Vault are the
precedent: nothing is created for anyone, while the v13->v14 stamp still gives
the drive an owner and an address on identities that already have one. The
drive is made by chat-kmp, so those identities exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Answer from the blob until an identity has moved to the tables
The services read the Circle and AppRegistrations tables and nothing else, but
the rows only arrive in v12 -> v13, and that upgrade runs when the owner logs
in. Between a deploy and that login an identity has no apps and no circles: app
tokens resolve to a null registration, and IsEnabledAsync reports every circle
disabled, because a null definition is not a disabled one. A dormant identity
could sit there indefinitely.
So the four read paths fall back to the blob. LegacyDefinitionStore holds the
legacy keys and the frozen registration shape, which the migration now shares
rather than keeping its own copy.
The gate is the tenant's data version, not an empty table. The move
deliberately leaves the blob rows in place, so after it a missing row means the
owner deleted something -- falling back on a miss would resurrect it. The list
reads are a union, table first, since anything written during the window is
already there and is the newer of the two.
AssignSlugAsync now reserves the legacy slugs too: a slug free in the table
could still be one the move is about to coin, and the move would then fail on
UNIQUE(identityId, AppSlug).
Delete all of it once every environment reports v13 or later.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Keep the tests in step with what the tree provisions
Twenty-three tests were failing on the branch, in three groups.
Eighteen were mail tests that could not get past setup. They name the Email
drive in their DriveSpec and the fixture created it outright, but Email joined
the tree, so the system gets there first and CreateDrive answers with "Drive by
alias and type already exists". Naming a drive in a spec states a precondition
rather than a claim to have made it, so the fixture now calls EnsureDrive, which
creates only what is missing. CreateDrive is unchanged and still means create,
so a test about drive creation still fails loudly on a duplicate.
Three counted circles and drives by hand. They count from BuiltinApps.SeededCircles
now, so adding an app's circle cannot break an unrelated assertion, and a fresh
identity is no longer expected to have a WalletDrive -- that stopped being seeded.
Two iterated BuiltinDrives.Protected, which still listed WebDropDrive after
Webdrop moved to Wellknown and stopped being provisioned. That list's own
comment asks for exactly this: protected is the same set as provisioned, and
they drifted once before.
Odin.Hosting.Tests.V2 495/495, Odin.Hosting.Tests 787/787.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let a client ask what the upgrade is doing while it runs
VersionUpgradeMiddleware refuses every /api call with 503 while an upgrade is
in flight, and data-version-info is under /api like everything else. So the one
question worth asking during an upgrade was the one question that could not be
asked -- the endpoint that reports the version, the failure info and whether an
upgrade is still required answered only once there was nothing to report.
It is exempt now. The UpgradeIsRunning header is still appended, so a client
gets the state and the fact that an upgrade is running in the same response.
Scoped to that one route, named as a constant the way InitialSetupEndpoint
already is: the rest of the controller mutates and stays behind the guard.
The guard only started firing when #1686 moved the flag to a tenant singleton
the middleware can actually see -- before that it was structurally always
false, which is why this never came up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give Community the app id its client actually registers with
The tree said 7802a474-5235-4581-af81-dd1a96d81edf. Nothing in odin-js has ever
used that guid; the community client registers with 77ed6136-6b33-4654-8088-3d89c91e6065
(common-app/src/constants.ts, COMMUNITY_APP_ID), and that one appears nowhere in
odin-core. Two ids invented independently for one app.
So the registered app matched no tree entry. KnownAppSlugs missed it and the
slug came from the display name -- "Homebase - Community" truncated to
homebase-commu, where the tree says community. The community drive and circle
were keyed to the constant too, which means they named an app that does not
exist on any identity.
One literal; the tree entry, drive and circle all read the constant.
Identities not yet converted now get it right. One already on v13 is corrected
by StampAppSlugsAsync when it reaches v14, since that looks up by app id. One
already on v14 has missed the correction and keeps homebase-commu.
Nine other tree ids have no odin-js client to check against -- their clients are
in chat-kmp, and the same comparison is owed there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Retire BuiltInCircleConstants
It described Emergency Location Access before the tree did, and since the tree
took over it has held only the circle's id -- already derived from
BuiltinCircles.EmergencyLocationAccessCircle.Id. A file whose whole content is
an alias for something else is a place for the two to drift.
Its last two callers, both in the v8 -> v9 validation, read the tree directly
now. AllBuiltInCircles and IsBuiltInCircle went with it; nothing called them
once the circle-count assertions started counting from BuiltinApps.SeededCircles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give the Vault drive the guids chat-kmp actually uses
odin-core named 1f513d67.../4a1414da... and chat-kmp names
f47ac10b-58cc-4372-a567-0e02b2c3d479 with the Contact drive's type. Neither
value appeared in the other repo, so the two named different drives: the
v13 -> v14 stamp would have looked for one no identity has, exactly the way the
Community app id missed.
chat-kmp's values win because they are the ones in the wild -- its client makes
the drive. Both are placeholders by its own comment, the alias being the RFC
4122 example uuid, so a doc comment says so here rather than leaving the next
reader to file it as a bug.
The shared type is the part to keep an eye on. Slugs are unaffected --
KnownDriveTypeSlugs is keyed by alias, which is why HomePageConfig, Wallet and
Profile already share one type between them -- but GetDrivesByType on the
contact type now also returns Vault, on the owner, app and peer routes. Only
where chat-kmp made the drive; Vault is not provisioned.
Replace both guids here and in chat-kmp together when the real ones land.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let an app name its own slugs, and refuse a duplicate drive slug
CreateDriveAsync validated the format of a caller-supplied drive slug but
only ever checked the taken set when deriving one. A supplied duplicate
therefore reached the insert and surfaced as a raw UNIQUE(identityId,
AppId, DriveSlug) violation rather than a client error. Read the taken set
whenever the drive has an owning app and use it for both questions: what a
derived slug must avoid, and whether a supplied one is already claimed.
Refuse rather than suffix -- a supplied slug is an address, so handing back
"news-2" would look like success while giving the caller a different one.
YouAuthAppParameters is a closed allowlist: it deserializes
permission_request by JsonPropertyName and re-emits via ToQueryString, so
an app's "as" key was dropped between its YouAuth URL and the /owner/appreg
screen. Carry AppSlug through both directions. Not required in Validate --
omitting it still means "derive from the app name".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Address drives by slug, locally and over peer
The addressing columns landed a while back, but nothing resolved them: no
route named a slug and no lookup could turn one into a drive. This adds
both halves.
The lookups filter the cached all-drives list rather than querying by slug.
Drives number in the tens, the list is already loaded for every other read
on DriveManager, and going through it means slug lookups inherit the same
archived/owner-only/anonymous filtering as their siblings -- a slug must not
surface a drive the guid routes would hide. App slugs resolve through the
AppRegistrations row, with no pre-v13 blob fallback: slugs only exist as
columns, so an un-migrated identity has no slug-addressable apps, which is
the truth rather than a gap.
Locally, GET /api/v2/apps/{appSlug}/drives lists what an app owns and
?type= filters it -- the replacement for /drives/metadata/channel-drives,
which exists only because channel drives had no other way to be named as a
group.
Over peer is the case that matters: it is how a caller addresses another
identity's drive without both hosts sharing hardcoded guid constants. The
slug is resolved by the recipient, exactly as a drive id is -- only the host
holding the drive knows what "chat" means there. So the sender asks
(new perimeter endpoint, guarded like metadata/type and returning 404 for
both unknown and unreadable), caches the answer, and then runs the ordinary
peer operation. No existing peer DTO or perimeter handler changes.
Incidentally more precise than the guid routes: those synthesize
TargetDrive { Alias = driveId, Type = Empty } and let the remote match on
alias alone. Resolution returns the real pair.
Peer *writes* are not covered. V2DrivePeerWriteController takes the remote
drive from the multipart instructions body rather than the route, so
resolve-then-delegate does not reach it; that needs the transfer
instruction set to carry a slug address.
Built, not exercised -- no runtime verification of route registration, the
perimeter round-trip, or the 404 paths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Test the slug-addressed routes, and fix what that turned up
43 tests across the four route groups: the local /apps/{appSlug}/drives
routes, the peer reads, the peer temporal reads, and the peer writes. Every
peer read is compared against its guid twin on the same drive -- a slug form
that merely succeeds is not the bar, it has to name the same drive.
Three things the tests found, none of which compiling had caught.
The perimeter resolver required Read or Write on the drive, and a
ConditionalTemporalRead grant is neither. Every temporal slug route was
therefore unusable by exactly the caller it exists for, the same way the
deposit-only writer was before. Any grant on the drive now resolves: holding
one already proves the caller knows of the drive.
Mail's app left both BuiltinApps lists but CircleConstants still grants
MailDrive from the two system circles, so identity setup threw
invalidGrantNonExistingDrive before finishing. MailDrive joins Lists and
Moments in SystemCircleCarryOverDrives, which exists for this exact case.
Moments and Vault joined Builtin, and a built-in app is registered at setup,
which needs a registration request. Added both.
Test helpers gained what the routes need to be exercised: CreateDrive takes
appId/driveSlug/driveTypeSlug, RegisterApp takes appSlug.
Full V2 project: 539 passed, 0 failed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Stop capturing the peer query service twice
CI builds with --warnaserror, where CS9107 is fatal: each slug-addressed peer
controller took PeerDriveQueryService as a primary-constructor parameter,
passed it to the base, and then also used it in its own body -- capturing it
in both. Locally this is only a warning, which is why it built here and broke
there.
The base now exposes it once as a protected property and the derived
controllers go through that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Put Mail back on the tree, and the three that displaced it back where they were
Rewriting both BuiltinApps lists by hand in "Test the slug-addressed routes"
changed which apps an identity is provisioned with, in ways that commit did not
intend and its V2-only test run could not see.
Mail was dropped from both lists outright. EnsureAppsAsync walks Builtin, so the
Mail app stopped being registered at setup -- "there should be 3 app grants;
mail, chat, and the app created in this test" is that, stated by the test. With
MailAppId off the tree the slug generator no longer recognised it either and
slugged it off its display name as homebase-mail, which is exactly the bug the
generator reads the tree to avoid.
Webdrop, Moments and Vault went the other way, from Wellknown into Builtin. That
seeds WebDropDrive, WalletDrive and VaultDrive for every identity, none of which
are in BuiltinDrives.Protected -- 17 drives provisioned against a Protected of
14. Protected's own remark says to keep the two in step, and the initialize test
says WalletDrive is deliberately absent because it is no longer seeded. Webdrop's
promotion also silently reversed "Webdrop is known, not shipped with every
identity".
Membership returns to what it was before that commit. Lists stays commented out
of Wellknown, which that commit did deliberately and explained.
MailDrive leaves SystemCircleCarryOverDrives with it. It only joined to stop
identity setup throwing invalidGrantNonExistingDrive once its app was gone; with
Mail owning it in Builtin again it is a seeded drive, and the carry-over list is
back to the two whose apps really are not built-in. Provisioned drives are again
the 14 Protected names.
The seven tests that failed on this branch since that commit now pass:
AppSlugGeneratorTests.KnownSystemAppsGetTheirName, the two
SystemInitializeConfigTests drive-count tests, and the five
CircleNetworkServiceAppTests app-grant tests. Odin.Services.Tests: 573 passed,
0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Revert "Put Mail back on the tree, and the three that displaced it back where they were"
This reverts commit 0c4815d6a3a4e42425a48dc21b2a1dfc525ad25b.
* Make the tests say what the tree now says: no Mail app, Moments and Vault built-in
The tree changed in "Test the slug-addressed routes" and the tests were never
brought with it, which is what has been red since. Mail's app is gone for good,
Moments and Vault ship with every identity; these tests still described the world
before that.
Mail is no longer a known system app, so the slug generator has nothing to say
about it -- KnownSystemAppsGetTheirName stops asserting a slug for an app the
tree does not name.
Only Chat and Mail authorised the system circles, so with Mail unregistered a
new connection carries one built-in app grant instead of two. The five
CircleNetworkServiceAppTests counts go 3 -> 2.
Protected regains the drives provisioning actually creates, WebDropDrive and
VaultDrive, and is again the same set: 16 either way. Its remark asks for the
two to be kept in step and they had drifted.
Two things this turned up that were not just test text.
VaultAppRegistrationRequest granted ReadWrite on WalletDrive. Vault is built-in,
so it is registered at setup, and a grant for a drive that does not exist throws
-- with WalletDrive no longer seeded, identity setup failed outright for every
new identity, not only in these tests. The grant goes. WalletDrive is now owned
by nothing, so an identity that already has one keeps it with a derived slug and
no owning app, reachable by guid rather than by an /apps/{appSlug}/drives path.
Both v13->v14 remarks that said otherwise are corrected.
WebDropDrive is anonymous-read and Webdrop is built-in, so HandleDriveAdded
grants the system circles read on it as it is created. The connected circle
holds 10 drive grants now, not 9, and the test says why.
Odin.Services.Tests: 567 passed, 0 failed. The nine tests in the two affected
Hosting.Tests classes all pass, including the two that only failed once
WalletDrive stopped being seeded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let the tenant lock go before the post-commit notification runs
SaveIcrAsync held the tenant lock across a write whose transaction disposed
inside that lock. Disposing a transaction runs its post-commit actions, and one
of those is the deferred DriveDefinitionAddedNotification: its handler goes
HandleDriveAdded -> GrantAnonymousRead -> UpdateCircleDefinitionAsync and calls
SaveIcrAsync again for the same tenant. The lock is keyed per tenant and is not
re-entrant, so the second call waited on a lock the first was still holding, and
neither ever returned.
It presented as a version upgrade that simply stopped. No exception, so nothing
was logged and no failed version recorded; no thread, because both frames were
parked async continuations rather than blocked threads; ~2% CPU with an idle
thread pool; and Postgres sitting idle-in-transaction at ClientRead holding a
connection nobody would ever come back for. Tenants stayed below the release
version and answered 503 to every owner request until something retried them --
which worked only because the retry had no drives left to create, so nothing
re-entered.
Found with dumpasync against a dump of a wedged host: the async stack showed
UpdateCircleDefinitionAsync -> SaveIcrAsync -> NodeLock -> KeyedAsyncLock parked
beneath a second copy of the same chain, one holding what the other wanted.
The transaction now opens outside the lock. Transactions are ref-counted, so the
one CircleNetworkStorage.UpsertAsync opens is no longer outermost and no longer
runs the post-commit actions; they run when this outer one disposes, by which
time the lock is released and the re-entrant call can take it. Ordering changes,
semantics do not: the notification still fires only after the commit, which is
the reason it was deferred.
Not step-2's bug, though that is where it surfaced. This code is identical on
both branches; f.hobbit hangs here on step-1 too. What made it reachable was the
up-front EnsureSystemDrivesExist pre-pass, which creates drives on every upgrade
run regardless of version -- before it, a tenant at v10 or above never created a
drive during an upgrade and so never re-entered.
No regression test yet, deliberately. The one I wrote does not discriminate: the
drive-create-inside-a-transaction shape leaks a ConnectionWrapper whose finalizer
aborts the run whether the fix is present or not, so it proved nothing. The
verification is f.hobbit on the integration host, which reproduces reliably. A
test that guards this needs the harness problem solved first.
Odin.Services.Tests: 567 passed. Release build clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Revert "Let the tenant lock go before the post-commit notification runs"
This reverts commit ce6b7cc05.
The fix was wrong. Releasing the lock before the post-commit notification did
stop the deadlock, and immediately replaced it with an infinite loop: on the
integration host gandalf ran GrantAnonymousRead about sixteen times a second,
1474 calls and climbing, all under one correlation id -- one operation recursing,
not many requests.
The cycle was there all along:
GrantAnonymousRead -> UpdateCircleDefinitionAsync -> SaveIcrAsync
-> transaction disposes -> post-commit -> DriveDefinitionAddedNotification
-> HandleDriveAdded -> GrantAnonymousRead -> ...
The tenant lock was the only thing stopping it, by blocking the second pass
forever. That is what the hang was. Removing the block without removing the cycle
turns a stall into a spin, which is worse on a live host: it burns CPU and
rewrites connection rows rather than sitting still.
So the deadlock diagnosis stands -- dumpasync showed the chain parked on
KeyedAsyncLock beneath a second copy of itself -- but the lock is a symptom of
the cycle, not the defect. The fix belongs where the cycle closes: either
HandleDriveAdded stops when the circle already grants the drive, so the second
pass is a no-op and the chain ends, or SaveIcrAsync's write stops republishing a
drive notification it did not cause. Which of those is right depends on why
updating a circle definition raises a drive-added notification at all, and I do
not understand that yet.
Reverting first so nothing spins on a live host while that is worked out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Run post-transaction actions after the transaction is torn down
The post-commit loop ran inside the dispose, while the action list was
still populated and _transaction was still set, even though the ref
count had already dropped to zero. In that window a post-commit action
that touched the database did not stack onto anything: it started a
fresh outermost transaction, and disposing that one iterated the same
uncleared list again and republished the notification that had just
been handled. With a per-tenant lock already held by the outer frame,
the re-entry deadlocked, which is what stalled the version upgrade
wh…
toddmitchell
added a commit
that referenced
this pull request
Sep 4, 2026
* Move circle definitions into the Circle table
Definitions lived in the shared key-three-value blob, which is why AppId and
GrantOn could never be queried or constrained -- the columns shipped dormant
with the drive-addressing schema work and nothing was ever wired to them.
TableCircle had no caller at all.
Putting the definitions in the table is what makes those columns usable: asking
"which circles enrol on connect?" is a WHERE GrantOn = ? against an indexed
column, not a load-all-deserialize-filter over opaque rows.
- CircleDefinition gains AppId, GrantOn, Designation and Emoji. CircleDefinition
is both the stored shape and the wire shape -- CircleDefinitionControllerBase
serves it directly and takes one as an update body -- so the fields stay on the
wire and the blob copy is cleared inside ToRecord instead, the same
clear-before-serialize trick ToConnectionsRecord uses for the grant
collections. Nothing in the blob can drift from the column, because
deserializing the blob alone yields defaults.
- Equality and GetHashCode account for the four, since EnsureSystemCirclesExist
reconciles definitions by comparing them.
- CircleGrantOn and CircleDesignation are new enums matching the column values.
Every existing circle is None/Personal, so nothing changes behaviour until
something sets them.
- AppId is not taken from an update request. Ownership is set when the circle is
created and must not be reassignable by anyone who can PUT a definition.
- TableCircle grows UpsertAsync and GetAllAsync; TableCircleCached wraps both and
invalidates the all-key alongside the per-circle key.
- CircleDefinitionService moves off ThreeKeyValueStorage onto db.CircleCached
with ToRecord/FromRecord doing the column-vs-blob split.
- v12 -> v13 copies existing definitions across. Idempotent and additive: a
definition already in the table is left alone, so a partial run repeats safely.
The blob rows are deliberately left in place -- if this goes wrong the source
data is still there. Cleaning them up is a separate job.
Tests pin both directions: no promoted value survives into the blob, the
caller's object is intact afterwards, the fields round-trip through the record,
they are visible on the wire, and an update body echoed back does not reset
GrantOn.
No behaviour change. The four fields take their defaults for every existing
circle, which is what they already were.
Squashed from PR #1661 (commits 07952208f, 3ea043d15) onto main; the migration is
renumbered v13->v14 to v12->v13 because the review-stamp work it originally sat
on is not here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Move app registrations into the AppRegistrations table
The other half of the drive-addressing groundwork. The table shipped with the
schema work and had no caller; AppRegistrationService still wrote to the shared
key-three-value blob, where UNIQUE(identityId, AppSlug) cannot be expressed at
all. Since the slug is a wire address other identities resolve against, a
best-effort uniqueness check over opaque rows is not good enough. The table also
gives Circle.AppId and Drives.AppId a real target.
Slugs have to be coined, because no registration has one and the column is
NOT NULL:
- Known system apps get their obvious name -- chat, feed, mail, photo, owner.
These are the addresses drive addressing assumes.
- Everything else derives from the registration's display name, the only
human-meaningful thing on the record.
- The whole set is resolved and checked before anything is written. GenerateAll
orders known apps first, so an app called "Chat" cannot take the chat app's
address, disambiguates collisions with a numeric suffix, and throws rather than
returning a duplicate. The migration re-checks distinctness on top of that. A
half-migrated app table with a slug collision is much worse than a migration
that refuses to start.
- A name that slugifies to nothing falls back to the app id. Unreadable, always
available, and better than refusing to migrate.
AppId, AppSlug, Name and CorsHostName become columns and are [JsonIgnore]d out of
grantJson, so a query on a column cannot disagree with the hydrated object.
Everything else still rides the JSON.
Registering a new app derives a slug the same way, so registration and migration
land on the same value; the request has no slug field yet, that arrives with
drive addressing. Updates carry the existing slug forward -- it is immutable, and
other identities may already hold it. Dedupe seeds from stored slugs rather than
re-derived ones, so an app holding "acme-2" still holds it whatever its name
would slugify to today.
The rows move in v12 -> v13, the same version step that moves the circle
definitions -- both are the same job, they ship together, and a tenant is either
on the tables or on the blob. Idempotent and additive: an app already present is
skipped so its slug is never reassigned, and the blob rows are left in place as a
fallback.
Cherry-picked from PR #1662 onto main; its v14 -> v15 migration is folded into
this branch's v12 -> v13 rather than burning a second version number.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Make the promoted circle columns settable, and queryable by GrantOn
The columns landed with nowhere to be set from and nothing to read them, which
left the tables moved but not actually in use. Both halves are lifted from the
Cat 3 branch, and both are inert until something declares a circle that enrols.
- TableCircle.GetByGrantOnAsync is a WHERE GrantOn = ? against Idx1Circle. That
query is the entire reason GrantOn is a column, and until the definitions moved
into the table it could not have been written. TableCircleCached caches it under
a ByGrantOn tag and invalidates that tag alongside the per-circle and all keys.
No caller yet -- the auto-connect pipeline is Cat 3.
- CreateCircleRequest gains AppId, GrantOn, Designation and Emoji, and the create
path writes them. Until now nothing could set AppId at all: create ignored it
and update refuses it by design, so an app-owned circle was unrepresentable.
Omitting all four yields null/None/Personal/null, which is what every existing
circle already is.
- AssertDepositOnlyIfAmbientAsync enforces the invariant the moment GrantOn
becomes settable: a circle that enrols without the owner present may hand out
write/react and read on already-anonymous drives, and nothing else. Checked at
definition-write time rather than grant-mint time, because an app can plant a
definition and the next owner-driven grant would mint it with the master key in
scope. Run on create and on every update, since an update is how a circle
becomes ambient.
Error codes 3013 and 3014 are new. 3010 is untouched here -- retiring it belongs
to the review work.
Tests pin the guard before there is a caller that can trip it: an ambient circle
carrying a permission key is refused, and a manual-membership circle carrying the
same key is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Build clean under --warnaserror
CI builds with --warnaserror; my local runs did not, so five warnings I read
past were six hard errors there. None of them were noise.
- TableCircle captured the primary-constructor parameter it also passes to the
base (CS9107). TableCircleMember already keeps an explicit field for exactly
this; TableCircle does the same now.
- RevokeApp/RemoveAppRevocation passed a possibly-null registration to SaveAsync
(CS8604). Under the old blob store that wrote a row whose payload was the
literal "null"; ToRecord would throw instead. Neither is wanted, so a missing
app now returns early, which is what the null check was always shaped like.
- FromRecord dereferenced a deserialize that can return null (CS8602). An
unreadable grantJson is a corrupt row, so it throws with the app id rather
than a bare NullReferenceException.
- Two tests dereferenced a nullable deserialize.
Verified with the same command the sqlite/debug job runs: 0 warnings, 0 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Drop the global UNIQUE on Circle.circleId
Regenerated CRUD and migration from the generator.
Circle declared circleId BYTEA NOT NULL UNIQUE alongside the correct
UNIQUE(identityId, circleId). The column-level constraint is global, and system
circle ids are fixed constants shared by every identity, so on Postgres -- where
all tenants share one database -- the second identity to run
config/system/initialize collided with the first:
23505: duplicate key value violates unique constraint
circlemigrationsv202608040942_circleid_key
The constraint dates to Postgres support (#854) and has been harmless until now
only because TableCircle had no caller: the table was empty everywhere. This
branch is the first code to write to it. SQLite never sees it because each tenant
gets its own file, which is why sqlite/debug and sqlite/release both passed while
postgres/release failed 337 tests.
Drives is the model: DriveId carries no column-level UNIQUE, only
UNIQUE(identityId, DriveId).
v202608261644 rebuilds the table without it, keeping the composite unique and
both indexes. Existing deployments already carry the constraint, so the DDL edit
alone would not have reached them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Create AppRegistrations on databases that never got it
Regenerated from the generator, with UpAsync/DownAsync hand-edited.
AppRegistrations shipped with the drive-addressing DDL carrying a version-0
migration only. The identity database keeps one migration version, and going up
the migrator runs only the groups above it -- so version 0 was unreachable on
every database already past it, and the table was simply never created there.
Fresh databases start at -1, run the version-0 group, and get it; which is every
CI database, so all three legs stayed green while a real deployment threw
42P01: relation "appregistrations" does not exist
as soon as AppRegistrationService queried it. main got away with it because
nothing read the table; this branch is its first reader.
v202608271000 restamps it above every released version, so the migrator reaches
it. Two populations have to arrive there, hence the branch in UpAsync:
- Table absent: create at this version and rename into place. No CopyDataAsync
and no rename of a table that is not there.
- Table present at version 0: the generated rebuild. Restamping requires it --
on SQLite the version marker lives inside the stored CREATE TABLE text.
DownAsync mirrors it: with no AppRegistrationsMigrationsV0 to restore, Up must
have created the table outright, so undoing means dropping it.
Uses cn.TableExistsAsync rather than GetTableVersionAsync, which on Postgres goes
through obj_description('AppRegistrations'::regclass) and throws 42P01 on a
missing relation before it can report anything.
The generator needs the same branch for any table introduced after the first
release, or the next regen reverts this. Noted in the file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Set the upgrade run state the way main now does
main's #1686 moved the flag off a private _isRunning field onto the injected
VersionUpgradeRunState. The v12 -> v13 step this branch adds was still assigning
the field, which merges cleanly and then does not compile.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Read the legacy app blob with the shape it was written in
The v12 -> v13 app move read nothing, silently. It deserialized blob rows into
AppRegistration, which [JsonIgnore]s AppId, AppSlug, Name and CorsHostName --
correct for writing, since those are columns now and a second copy in grantJson
could drift from them, and fatal for reading the blob, where that JSON is the
only place the values exist.
Every legacy row came back with a null AppId, the Where filtered the lot, and the
migration logged "no app registrations in blob storage; nothing to move" and
committed. The tenant reached v13 with an empty AppRegistrations table while the
blob still held every app -- and AppRegistrationService reads the table only, so
the identity presents as having no apps at all. Seen on the demo box.
The migration now holds LegacyAppRegistration: the blob shape frozen as it was
before the columns were promoted. A migration reads history, so it owns a copy of
the shape history was written in rather than borrowing a type that has moved on --
the same reasoning as the frozen context and category keys above it.
Note the asymmetry that caused this: CircleDefinition solves the same
blob-versus-column problem by clearing the fields inside ToRecord and keeping them
serializable, so the circle half of this migration was never affected.
Tests seed a legacy row in the old JSON shape and assert the app lands with its
real name, its CorsHostName and a slug derived from that name -- a slug of the app
id would mean Name came back null. Plus idempotency: a second pass must not mint a
new slug, since it is an address other identities may already hold.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Create the shared drive once instead of racing 20 threads to make it
Overwrite_Encrypted_PayloadManyTimes_Concurrently_MultipleThreads runs 20
threads against one target drive, and each thread called CreateDrive for that
same drive from inside PrepareEncryptedFile. Only one create can win; a loser's
first upload could land before the winner's drive was visible, so the setup
assertion on IsSuccessStatusCode failed. Seen on the ubuntu/postgres job of run
33076953471 (3 of 20 threads), where sqlite and windows passed on the same
commit.
Create the drive once in the test method, before the threads start. The
concurrency under test - 20 threads overwriting their own file 50 times - is
unchanged.
Also make the two counters Interlocked: all 20 threads increment them and the
final assertion reads them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Wire the drive addressing columns through
AppId, DriveSlug and DriveTypeSlug shipped with the drive-addressing DDL and were
invisible above the SQL layer: DriveManager.ToRecord never wrote them,
ToStorageDriveData never read them, and StorageDrive had nowhere to put them. The
columns could not be populated even by hand.
- StorageDriveData and StorageDrive gain the three. They stay out of
StorageDriveDetails on purpose -- UNIQUE(identityId, AppId, DriveSlug)
constrains the columns, and a copy inside detailsJson could disagree with what
the constraint is enforcing. Same discipline the circle work used.
- ToRecord writes them, ToStorageDriveData reads them, and the create path
persists what the request carried.
- CreateDriveRequest accepts them, all optional. Omitting them leaves a drive
addressed by Guid exactly as before, which is every drive today.
- OwnerClientDriveData carries them, so a client can read what a drive holds.
- OdinSlug validates the format from docs/drive-addressing.md: lowercase, digits,
internal hyphens, 1-12 characters. Validate and reject, never coerce -- the
value ends up in other identities' URLs, so lowercasing 'Chat' would hand back
an address the caller did not ask for. The reserved-segment list is empty and
deliberately present: /apps roots the slug tree so neither position has a
literal sibling today, and it must grow when one appears.
WriteOnlyKeyPair is deliberately not plumbed. It is key material for write-only
deposits with escrow, rotation and deleted-drive questions still open in the doc,
and it must never reach a client shape.
Nothing derives a slug or assigns ownership. Every drive still carries null for
all three until the mapping is settled; this only makes the columns reachable.
Tests: the columns round-trip through TableDrives and null stays null; the slug
rule accepts what the doc allows and rejects encoding, path-separator, case and
length violations.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add the addressing go-live checklist
Derived working checklist across the two table moves, the chat circle ownership
change and the drive addressing columns: what is done, what is blocked on a
decision only Todd can make, what backfill and enforcement is owed, and the
deploy-safety items the demo box taught us.
Follows docs/connection-defaults-checklist.md: a derived list, not a spec. The
two design docs remain the source of truth.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Carry the drive addressing fields on the app-route drive shape
OwnerClientDriveData got AppId, DriveSlug and DriveTypeSlug; ClientDriveData did
not, so a client reading drives over the app or V2 route could not see them.
- V2DriveMetadataController maps all three.
- ClientTokenDriveMetadataController redacts them for third parties exactly as it
already redacts Name and Attributes. The slug is designed to be a
remote-resolvable address, but resolution happens on the recipient side
(drive-addressing.md, "Slugs are resolved by the recipient"), so a guest does
not need the list to use one. One-line change if we decide otherwise.
The peer route is deliberately untouched: PeerQueryControllerBase maps
PerimeterDriveData, the cross-identity wire shape, which carries only TargetDrive
and Attributes. Publishing slugs to another identity is a separate decision about
what an identity discloses, not a mapping change.
Null on every drive today, so nothing changes for any caller.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let an app ask for its own slug at registration
drive-addressing.md has the app choosing its slug: it is a package name, not a
role -- "a second chat implementation does not get to call itself chat; it picks
its own slug (chatty)" -- and "registration is first-come", which only makes
sense if the app is asking and can be refused. The field never existed; the
server coined one from the display name because pre-existing registrations had
none and the column is NOT NULL. That was the migration's stopgap, not the design.
AppRegistrationRequest.AppSlug is optional and unenforced:
- Omitted: derived from Name exactly as before, so nothing that registers today
starts failing.
- Supplied: validated for format and taken verbatim, or refused. Never quietly
replaced with a derived one -- it is an address other identities resolve
against, and handing back a different one is worse than saying no.
- Already held by another app: refused with a clear client error rather than a
UNIQUE(identityId, AppSlug) constraint violation from the database.
Immutability is unchanged: updates carry the stored slug forward, and no update
request carries a slug field at all.
Tests cover all four paths. Fixed my own invented expectation while writing them:
"Acme Receipts" derives to "acme-receipt", not "acme-receipts" -- the
generator truncates at the 12-character cap. The migration test written earlier
had the same wrong value and had never run, since port 4444 was busy; it would
have failed in CI.
Not done here: protecting the system slugs (chat, mail, feed, photo, owner) from
a caller claiming them. AppSlugGenerator orders known apps first when deriving,
but nothing stops a supplied slug taking one on an identity where that app is not
yet registered -- checklist 3.9.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Update the checklist for the app slug work
3.1 is done: AppRegistrationRequest carries an optional AppSlug, validated and
taken verbatim or refused. Records what shipped (0.10, 0.11), and the two things
that decision leaves behind:
- 3.3a: slug derivation now lives in both the registration service and the
migration, deliberately, rather than making the column nullable.
- 3.9 changes character. It used to be theoretical; a caller can now supply
'chat' on an identity where the chat app is not yet registered and take it
first-come. Same question as drive-addressing.md OQ2.
Plus 3.3b, the 12-character truncation, and 7.5 for clients that want to name
themselves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give every drive an owning app and a slug
The drive addressing columns have been in place but nothing ever put a value
in them, so every drive carried a null AppId, DriveSlug and DriveTypeSlug.
This fills them in from a fixed mapping.
Move the thirteen app-owned drives out of SystemDriveConstants into
WellKnownAppDrives, leaving only TransientTempDrive behind. Add CommunityDrive
there too. The rename is mechanical -- same namespace, so it is an identifier
swap the compiler checks -- and touches 451 call sites.
Coin app ids for the apps that own a drive but have no registration yet:
Community, Contacts, Email, HomePage, Lists, Location, Moments, Recovery,
Vault, and a System app for the transient drive. An app id is permanent, since
it is what Drives.AppId points at and what a drive slug is unique within.
DriveSlugGenerator mirrors AppSlugGenerator: fixed slugs for the known drives,
derived from the drive name otherwise, whole set resolved up front. It is
wired into drive creation as a fallback -- a caller-supplied slug is taken
verbatim or refused, never quietly replaced.
Two details worth calling out:
Slugs are deduped per owning app, not per identity. The constraint is
UNIQUE(identityId, AppId, DriveSlug), so feed/news and chat/news may coexist;
deduping identity-wide would hand the second one "news-2", a permanent address
nobody asked for, for a collision the schema allows.
Nothing is derived for a drive with no owning app. AppId and DriveSlug are set
together or both NULL: NULLs are distinct in a unique index in both dialects,
so a slug on an AppId-less row is unconstrained and two drives could claim it.
Raise OdinSlug.MaxLength from 12 to 14 so "shard-recovery" fits. The database
caps these at 64, so there is room. Existing tests that asserted 12-character
truncation are updated to the new cap.
Whitespace-only is now treated as "not set" for app and drive slugs alike.
Clients serialize an unset field as "" or " " routinely, and the three
spellings had diverged: null and "" derived a slug while " " failed
validation and threw. A value with real content is still validated and
rejected, so " chat " is an error rather than being trimmed to "chat".
Seeding is deliberately unchanged: EnsureSystemDrivesExist still creates all
fourteen drives, now via WellKnownAppDrives. Which of them a new identity
should get is a separate decision.
Known gaps, to be addressed next:
- Profile, Wallet and HomePageConfig share one drive type but are given the
type slugs profile, wallet and profile; Moments and Lists share a type
across two different apps. Both break the one-slug-per-type and
one-app-per-type rules in docs/drive-addressing.md. Type slugs are keyed by
drive alias for now so the mapping is stored as given rather than resolved
by guesswork.
- Photo Library and Vault are named by the mapping but have no Guids yet.
- The WellKnownAppDrives header still says its drives are absent from
SystemDrives and never server-created. Both are untrue until seeding is
settled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Register the built-in apps and their circles at identity setup
Every drive already had an owning app; this gives every app its circles, and
decides which apps a new identity starts with.
Nine apps are built-in and get registered: Chat, Contacts, Email, Feed,
HomePage, Location, Mail, Recovery and System. Contacts, Email, HomePage,
Location, Recovery and System had no registration before -- each is granted
ReadWrite on the drives it owns, with no permission keys and no authorized
circles, since none were specified. The three near-identical Register*App
helpers collapse into one RegisterAppIfNotExistsAsync.
WellKnownAppCircles holds the sixteen app-owned circles. Ten belong to
built-in apps and are provisioned by EnsureBuiltInAppCirclesExistAsync; the
other six arrive only with their app. Friends, Family, Work and Acquaintances
are in code for the first time -- until now the owner console's setup wizard
created them client-side. The two system circles are deliberately untouched:
they are retired in a later step, and until then they still have to work.
Chat is the only GrantOn=Connect circle, so it is granted ambiently to
auto-connections with no owner review. It carries write/react only, which is
what the deposit-only invariant requires.
Seeded drives and SystemDrives are now the same set, which is the point of
this change and not a coincidence: SystemDrives is what makes a drive immutable
(DriveManager refuses to rename, re-mode or archive anything in it), so a
seeded drive missing from it is one the owner can archive out from under the
system. WalletDrive leaves both -- Vault is not built-in -- and EmailAppDrive
joins both, because Email is built-in and its registration is granted the
drive, and a grant cannot be issued for a drive that does not exist.
ListsDrive and MomentsDrive are seeded even though Lists and Moments are not
built-in. The system circles grant them, and issuing those grants throws if the
drive is absent. Both go when those circles do.
Move ChannelDriveType into WellKnownAppDrives to break a static-initializer
cycle. SystemDrives lists drives declared in WellKnownAppDrives, and
PublicPostsChannelDrive read SystemDriveConstants.ChannelDriveType, so touching
WellKnownAppDrives first ran SystemDriveConstants mid-initialization and built
SystemDrives out of fields that were still null. It resolved by declaration
order until adding EmailAppDrive to the list changed which type was touched
first; the failure is a NullReferenceException far from the cause, never an
error at the source. WellKnownAppDrives now reads nothing from
SystemDriveConstants, so the dependency runs one way.
WellKnownAppDrivesTests guarded that EmailAppDrive was never auto-created.
That stopped being true, and the test kept passing because it only checked list
membership. It now guards what replaced it: anything seeded must be immutable.
Not done: the conversion for the six apps that are not built-in, which has to
stamp ownership and slugs onto what existing identities already hold.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Declare the default system as one tree
Six lists currently state which app owns which drives and circles, all keyed by
app id and never joined: EnsureSystemDrivesExist, EnsureBuiltInApps,
SystemDrives, BuiltInAppIds, the slug tables in DriveSlugGenerator, and the
circle constants. Answering "what is Chat?" means grepping four files, and
nothing stops two of those lists disagreeing -- which is how the seeded drives
and SystemDrives drifted apart earlier on this branch.
Odin.Services.Apps.Builtin declares it once instead:
SystemApp.cs the SystemApp and AppDriveGrant records
BuiltinDrives.cs 18 drives -- identity, address, settings
BuiltinCircles.cs 17 circles -- id, grants, GrantOn
BuiltinAppDriveGrants.cs 25 cross-app grants, flat
BuiltinApps.cs 15 apps, and the projections over them
Ownership is a tree, so drives and circles nest under the app that owns them.
Grants are not: eleven of the eighteen supplied rows cross app boundaries --
Chat holds ReadWrite on ContactDrive, which Contacts owns -- so nesting them
would mean one app's node referencing another's, which is the static
initializer cycle that already bit us once. They sit in a flat sibling list
that references the drive constants directly.
Circle drive-grants are the opposite and do nest: every circle grants only
drives its own app owns, with no exceptions. The two system circles are the one
thing that does not fit -- owned by no app, granting across six drives -- so
they stay in SystemCircleConstants until they retire.
Nothing reads any of this yet. The values are copied, not moved, and
SystemDriveConstants, WellKnownAppCircles and BuiltInCircleConstants are still
what runs. Both copies were diffed field by field: drive settings match on
name, anonymous reads, owner-only, subscriptions, CDN, target drive and app id;
circles match on id, owning app, GrantOn and drive grants.
Slugs are stated here rather than derived. Each drive carries its own, so the
36 entries in DriveSlugGenerator's lookup tables stop being a second place for
them to live. Apps carry one too: only five were fixed before, so ten would
have been derived from their display name, and the one for the app formerly
called Owner still read "owner" after the rename.
Name, AppSlug and Permissions have no reader yet. They exist to build an
AppRegistrationRequest, which cannot be derived until AuthorizedCircles has
somewhere to point -- today Chat and Mail aim theirs at the system circles.
Verified against the four supplied mappings: 15 apps and their built-in flags,
18 drives with owning app and both slugs, 17 circles with owning app and
GrantOn, and 18 drive grants.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Provision an identity from the app tree
BuiltinProvisioner reads the tree and creates what an identity starts with, so
the set provisioned is a projection rather than a list restated in
TenantConfigService. EnsureInitialOwnerSetupAsync calls EnsureAllAsync; the
version ladder keeps calling EnsureDrivesAsync on its own, since
VersionUpgradeService does one up-front pass so migrations can assume every
drive exists. TenantConfigService keeps EnsureSystemDrivesExist and
EnsureBuiltInApps as forwarders so those callers are untouched.
Verified the same 14 drives, 9 apps and 13 circles as before -- diffed against
the previous commit, not assumed.
Provisioning order is drives, then circles, then apps, which is a change.
Circles used to come first, and that made a check misreport: the deposit-only
guard runs whether or not validation is skipped, and for an ambient circle with
a read grant it reads the drive to see if it allows anonymous reads. With no
drive there yet the lookup returned nothing and the error blamed the read grant
rather than the ordering. Nothing needed circles first -- HandleDriveAdded only
touches the two system circles, and those are created by the caller before the
provisioner runs.
Within drives, non-anonymous first. Creating an anonymous-read drive makes
HandleDriveAdded grant read on it to the system circles, so every drive those
circles already grant has to exist by then. All six are non-anonymous, so
ordering on that flag satisfies the constraint by construction. It used to be a
comment asking the next person to keep ListsDrive above the anonymous ones.
The tree now separates the two groups instead of flagging them: Builtin is the
nine an identity is configured with, Wellknown the six that arrive only when
the owner installs them. The BuiltIn property is gone -- list membership is the
fact, and holding both invites an app in one list claiming the other.
SystemAppConstants.BuiltInAppIds and IsBuiltInApp are deleted for the same
reason; their only caller moved into the provisioner.
Three of the six own a drive every identity already has, seeded long before
ownership existed: ListsDrive, MomentsDrive and WalletDrive. Those need
stamping by the conversion, which is still to write. The other three own
nothing that exists.
Also record why drive creation must not check that the owning app exists. It
does not today, but only by omission, and the dependency runs the other way: a
registration is granted drives, and a grant cannot be issued for a drive that
is absent. Validating the app at drive creation would make the two constraints
unsatisfiable for every built-in app.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Bring existing identities onto the app tree
The addressing columns landed several commits ago and nothing ever filled them,
so on a tenant that predates this every drive carries AppId, DriveSlug and
DriveTypeSlug as NULL, and the circles the owner console's setup wizard created
carry no AppId. v13 -> v14 fills them in from BuiltinApps.
Two halves, and they are different jobs. Stamping fills rows that already
exist. Provisioning then creates what is missing, and is the same
BuiltinProvisioner a new identity gets -- so an upgraded identity converges on
what a fresh one has, rather than the two paths drifting. Stamping runs first,
so provisioning sees a drive that is already owned rather than trying to create
one that is there.
It walks the whole tree, not just the built-in apps. ListsDrive, MomentsDrive
and WalletDrive belong to apps that are not built-in, yet sit on every identity
because they were seeded long before ownership existed. Those three are the
reason the Wellknown list is not simply inert.
Additive throughout: anything that already has an owner is skipped rather than
reassigned, so a partial run repeats safely and a value set by hand is never
overwritten. Nothing is deleted -- WalletDrive stops being seeded for new
identities, but the ones that have it keep it, stamped like the rest.
This needs two setters that deliberately did not exist. Ownership is not
reassignable through the normal write paths -- CircleDefinitionService.UpdateAsync
refuses to take AppId from a request precisely so nobody who can PUT a
definition can hand a circle to an app, and nothing updates a drive's slug
because it is a wire address other identities resolve against. Both new methods
are internal, refuse an item that already has an owner, and exist only to give
a row the values it would have been created with today. The drive one also
skips the system-drive guard every other setter has: all fourteen are system
drives, so guarding would make it useless for its one job.
Version.DataVersionNumber goes to 14. That constant is the gate --
RequiresUpgradeAsync compares against it -- so without the bump the rung would
exist and never run.
Supersedes #1691, now closed. That branch numbered a different v13 -> v14 which
gave the relationship circles to chat; the mapping puts them under Contacts, so
it was contradicted rather than merely renumbered. It was never deployed, so no
identity is recorded at a v14 that meant something else.
Untested. Nothing here has been exercised: the migration path is hosting
integration territory, and the checklist already notes that every migration
test starts from an empty database, which is the case that works.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Move the drive constants into the tree
The 15 CreateDriveRequest constants were duplicated by BuiltinDrives and read
by nothing but one test file, so they are deleted and AllowCdnTests points at
the tree instead. That last part matters more than the line count: those tests
pin CDN settings, and aimed at the copy the provisioner no longer uses they
would have kept passing while the real values drifted.
SystemDrives becomes BuiltinDrives.Protected. It was never a list of system
drives -- ListsDrive and MomentsDrive are in it and belong to apps that are not
even built-in, provisioned only because the system circles grant them and a
grant for an absent drive throws. What the list actually decides is whether the
owner may rename, re-mode or archive a drive, which is its only use: three
guards in DriveManager and the flag the owner console renders. It is named for
that now, and says out loud that protected means "we provisioned it" rather
than "it is systemic".
Verified the new list identical to the old, same entries in the same order,
before repointing any of the twelve usages.
SystemDriveConstants drops from 218 lines to 33: the transient drive's
identity, which still has around ninety references, and a forwarder for the
channel type. Moving those two would retire the file, but it is a couple of
hundred call sites and better done deliberately.
WellKnownAppCircles is gone, deleted separately; BuiltinCircles is now the only
declaration of the app-owned circles, and its comment no longer claims
otherwise.
Protected is still hand-listed rather than derived from what is actually
provisioned. The two drifted once already -- WalletDrive left the seeded set
and EmailAppDrive joined it, and neither was reflected -- so deriving it is the
real fix. It needs the provisioned set named first, which is currently computed
inline in EnsureDrivesAsync, and the static initializer cycle between these
types has bitten once, so that is worth doing on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Make the upgrade enforce the app tree, not just fill it in
The tree is the source of truth for the drives, circles and apps it declares,
so the upgrade now makes them match rather than filling in whatever is empty.
Each stamp used to skip anything that already had a value, which meant a row
written by an earlier build kept that value forever.
That is not hypothetical. An earlier branch briefly gave the relationship
circles to chat before the mapping put them under Contacts, and any identity
that ran it holds Chat-owned Friends, Family, Work and Acquaintances that a
fill-only stamp would never correct. Same shape for app slugs: registrations
built before the tree was authoritative derived the slug from the display name,
so "Homebase - Location" was registered as homebase-locat -- and a slug is
immutable through every normal path, since other identities resolve against it,
so nothing else would ever fix it.
Three methods, all internal and migration-only, all returning whether they
changed anything so the log records corrections rather than visits:
ApplyTreeAddressAsync drive AppId, DriveSlug, DriveTypeSlug
ApplyTreeDefinitionAsync circle AppId, GrantOn, Designation
ApplyTreeSlugAsync app AppSlug -- new; there was no way in at all
Each one is an exception to a rule that exists for a reason. UpdateAsync
refuses to take AppId from a request so that nobody who can PUT a definition
can hand a circle to an app; an update carries the stored app slug forward
because it is a wire address. The remarks on each say why the exception is
warranted, so the rule is not quietly weakened.
Validation is stronger to match: it used to check a drive or circle had an
owner, and now checks the value equals what the tree says. A correction that
silently fails now fails the upgrade instead of recording success.
The app slug path checks uniqueness before writing, since UNIQUE(identityId,
AppSlug) would otherwise surface as a constraint violation. It throws naming
the conflicting app. One limitation: two apps that need to swap slugs cannot,
because whichever is corrected second still finds the first holding its target.
No mapping we have does that, and failing loudly beats half-applying a rename.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give user-created channel drives an owner and an address
A user makes channel drives at will, so there are arbitrarily many and none has
a fixed alias -- they are the one kind of drive the tree cannot list, and the
reason DriveSlugGenerator keeps its derivation at all. The upgrade now hands
them to the feed app with a slug derived from the drive name and a type slug of
"channel".
Fills rather than corrects, unlike the drives the tree declares. The tree is
authoritative for what it declares and it does not declare these: their slug
comes from a name the owner chose and may since have changed, so re-deriving on
every upgrade would move an address other identities resolve against. A channel
drive that already has one is left alone.
Ordering is load-bearing. It runs after the tree drives are stamped, so
FeedDrive and PublicPostsChannelDrive already hold "feed" and "posts" in
storage and the taken set read back is complete -- otherwise a channel named
Posts could take "posts" first. Uniqueness is scoped to the feed app, matching
UNIQUE(identityId, AppId, DriveSlug), and the taken set accumulates inside the
loop so two channels named News become news and news-2.
The type slug is stated rather than looked up. These drives came from a query
on ChannelDriveType so the type is already known, and TypeSlugFor can return
null -- which AssertValidOrNull permits, so a miss would have been stored
silently. Validation now checks the type slug too; it previously asserted only
the owning app and slug, so a null would have survived the upgrade unnoticed.
That matters because DriveTypeSlug is what ?type=channel will filter on once
those routes exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let the tree name every app's slug, not a second list
KnownAppSlugs listed five apps by hand while the tree named far more, so any
app it missed had its slug coined from the display name instead: "Homebase -
Recovery" became homebase-recov where the tree said recovery. The v13->v14
stamp then had to correct an address other identities may already hold.
Reading BuiltinApps.All removes the second list rather than lengthening it.
The tree is validated as it is indexed -- a duplicate app id or a malformed
slug is a mistake worth naming before it reaches a registration.
This settles one disagreement in the tree's favour: the System app derives as
'system' now, not 'owner', which is what the v14 stamp was already forcing.
EveryAppInTheTreeGetsTheSlugTheTreeNames is the test that would have caught
this, and cannot drift as the tree grows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give Emergency Location Access one definition and one creator
The circle was declared twice. EnsureBuiltInCirclesExistAsync created it from
BuiltInCircleConstants' own copy, whose CreateCircleRequest left out AppId,
GrantOn and Designation -- and because that runs before provisioning in
EnsureInitialOwnerSetupAsync, EnsureCircleExistsAsync then found the circle
present and skipped it. Every identity ended up with the unowned row, new ones
included, so the owner console showed the circle as belonging to no app while
the tree named Location as its owner.
UpdateAsync will not set AppId by design, so the reconcile branch could not
repair it either: it rewrote everything except the field that was wrong.
The definition now lives only in the tree and the row is only ever written by
EnsureCircleExistsAsync, which carries all three fields. v9 is where this
circle arrives, so that migration asks for it by name instead of relying on a
side effect of reconciling the system circles.
Existing identities still need the v13->v14 stamp, which corrects rather than
fills and already covers this circle.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add the Webdrop app and give every remaining drive a slug
Webdrop joins the tree as a built-in: its app id, drive, drive slug, circle and
registration request, and provisioning registers it like the rest.
The v13->v14 upgrade also gains a final pass over drives nothing declares --
created through the owner console or the setup wizard's own request.Drives, so
neither the tree nor the channel query reaches them. They would otherwise come
out of the upgrade with no address. Slug only: the owning app stays as it was,
which means the unique index cannot police these rows, so the pass dedupes them
itself and validation fails the upgrade if any drive is left without a slug or
two unowned drives share one.
ApplyTreeAddressAsync becomes ApplyAddressAsync, with a nullable app id and type
slug, since it now serves drives that have neither.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Webdrop is known, not shipped with every identity
It was in the built-in list, so provisioning created its drive, its circle and
its registration on every identity that upgraded -- and the drive allows
anonymous reads, which makes HandleDriveAdded grant read on it to both system
circles. A new public drive on every identity is a product decision, not a
side effect of a tree entry.
Wellknown is the list for this, and Lists, Moments and Vault are the
precedent: nothing is created for anyone, while the v13->v14 stamp still gives
the drive an owner and an address on identities that already have one. The
drive is made by chat-kmp, so those identities exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Answer from the blob until an identity has moved to the tables
The services read the Circle and AppRegistrations tables and nothing else, but
the rows only arrive in v12 -> v13, and that upgrade runs when the owner logs
in. Between a deploy and that login an identity has no apps and no circles: app
tokens resolve to a null registration, and IsEnabledAsync reports every circle
disabled, because a null definition is not a disabled one. A dormant identity
could sit there indefinitely.
So the four read paths fall back to the blob. LegacyDefinitionStore holds the
legacy keys and the frozen registration shape, which the migration now shares
rather than keeping its own copy.
The gate is the tenant's data version, not an empty table. The move
deliberately leaves the blob rows in place, so after it a missing row means the
owner deleted something -- falling back on a miss would resurrect it. The list
reads are a union, table first, since anything written during the window is
already there and is the newer of the two.
AssignSlugAsync now reserves the legacy slugs too: a slug free in the table
could still be one the move is about to coin, and the move would then fail on
UNIQUE(identityId, AppSlug).
Delete all of it once every environment reports v13 or later.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Keep the tests in step with what the tree provisions
Twenty-three tests were failing on the branch, in three groups.
Eighteen were mail tests that could not get past setup. They name the Email
drive in their DriveSpec and the fixture created it outright, but Email joined
the tree, so the system gets there first and CreateDrive answers with "Drive by
alias and type already exists". Naming a drive in a spec states a precondition
rather than a claim to have made it, so the fixture now calls EnsureDrive, which
creates only what is missing. CreateDrive is unchanged and still means create,
so a test about drive creation still fails loudly on a duplicate.
Three counted circles and drives by hand. They count from BuiltinApps.SeededCircles
now, so adding an app's circle cannot break an unrelated assertion, and a fresh
identity is no longer expected to have a WalletDrive -- that stopped being seeded.
Two iterated BuiltinDrives.Protected, which still listed WebDropDrive after
Webdrop moved to Wellknown and stopped being provisioned. That list's own
comment asks for exactly this: protected is the same set as provisioned, and
they drifted once before.
Odin.Hosting.Tests.V2 495/495, Odin.Hosting.Tests 787/787.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let a client ask what the upgrade is doing while it runs
VersionUpgradeMiddleware refuses every /api call with 503 while an upgrade is
in flight, and data-version-info is under /api like everything else. So the one
question worth asking during an upgrade was the one question that could not be
asked -- the endpoint that reports the version, the failure info and whether an
upgrade is still required answered only once there was nothing to report.
It is exempt now. The UpgradeIsRunning header is still appended, so a client
gets the state and the fact that an upgrade is running in the same response.
Scoped to that one route, named as a constant the way InitialSetupEndpoint
already is: the rest of the controller mutates and stays behind the guard.
The guard only started firing when #1686 moved the flag to a tenant singleton
the middleware can actually see -- before that it was structurally always
false, which is why this never came up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give Community the app id its client actually registers with
The tree said 7802a474-5235-4581-af81-dd1a96d81edf. Nothing in odin-js has ever
used that guid; the community client registers with 77ed6136-6b33-4654-8088-3d89c91e6065
(common-app/src/constants.ts, COMMUNITY_APP_ID), and that one appears nowhere in
odin-core. Two ids invented independently for one app.
So the registered app matched no tree entry. KnownAppSlugs missed it and the
slug came from the display name -- "Homebase - Community" truncated to
homebase-commu, where the tree says community. The community drive and circle
were keyed to the constant too, which means they named an app that does not
exist on any identity.
One literal; the tree entry, drive and circle all read the constant.
Identities not yet converted now get it right. One already on v13 is corrected
by StampAppSlugsAsync when it reaches v14, since that looks up by app id. One
already on v14 has missed the correction and keeps homebase-commu.
Nine other tree ids have no odin-js client to check against -- their clients are
in chat-kmp, and the same comparison is owed there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Retire BuiltInCircleConstants
It described Emergency Location Access before the tree did, and since the tree
took over it has held only the circle's id -- already derived from
BuiltinCircles.EmergencyLocationAccessCircle.Id. A file whose whole content is
an alias for something else is a place for the two to drift.
Its last two callers, both in the v8 -> v9 validation, read the tree directly
now. AllBuiltInCircles and IsBuiltInCircle went with it; nothing called them
once the circle-count assertions started counting from BuiltinApps.SeededCircles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give the Vault drive the guids chat-kmp actually uses
odin-core named 1f513d67.../4a1414da... and chat-kmp names
f47ac10b-58cc-4372-a567-0e02b2c3d479 with the Contact drive's type. Neither
value appeared in the other repo, so the two named different drives: the
v13 -> v14 stamp would have looked for one no identity has, exactly the way the
Community app id missed.
chat-kmp's values win because they are the ones in the wild -- its client makes
the drive. Both are placeholders by its own comment, the alias being the RFC
4122 example uuid, so a doc comment says so here rather than leaving the next
reader to file it as a bug.
The shared type is the part to keep an eye on. Slugs are unaffected --
KnownDriveTypeSlugs is keyed by alias, which is why HomePageConfig, Wallet and
Profile already share one type between them -- but GetDrivesByType on the
contact type now also returns Vault, on the owner, app and peer routes. Only
where chat-kmp made the drive; Vault is not provisioned.
Replace both guids here and in chat-kmp together when the real ones land.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Let an app name its own slugs, and refuse a duplicate drive slug
CreateDriveAsync validated the format of a caller-supplied drive slug but
only ever checked the taken set when deriving one. A supplied duplicate
therefore reached the insert and surfaced as a raw UNIQUE(identityId,
AppId, DriveSlug) violation rather than a client error. Read the taken set
whenever the drive has an owning app and use it for both questions: what a
derived slug must avoid, and whether a supplied one is already claimed.
Refuse rather than suffix -- a supplied slug is an address, so handing back
"news-2" would look like success while giving the caller a different one.
YouAuthAppParameters is a closed allowlist: it deserializes
permission_request by JsonPropertyName and re-emits via ToQueryString, so
an app's "as" key was dropped between its YouAuth URL and the /owner/appreg
screen. Carry AppSlug through both directions. Not required in Validate --
omitting it still means "derive from the app name".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Address drives by slug, locally and over peer
The addressing columns landed a while back, but nothing resolved them: no
route named a slug and no lookup could turn one into a drive. This adds
both halves.
The lookups filter the cached all-drives list rather than querying by slug.
Drives number in the tens, the list is already loaded for every other read
on DriveManager, and going through it means slug lookups inherit the same
archived/owner-only/anonymous filtering as their siblings -- a slug must not
surface a drive the guid routes would hide. App slugs resolve through the
AppRegistrations row, with no pre-v13 blob fallback: slugs only exist as
columns, so an un-migrated identity has no slug-addressable apps, which is
the truth rather than a gap.
Locally, GET /api/v2/apps/{appSlug}/drives lists what an app owns and
?type= filters it -- the replacement for /drives/metadata/channel-drives,
which exists only because channel drives had no other way to be named as a
group.
Over peer is the case that matters: it is how a caller addresses another
identity's drive without both hosts sharing hardcoded guid constants. The
slug is resolved by the recipient, exactly as a drive id is -- only the host
holding the drive knows what "chat" means there. So the sender asks
(new perimeter endpoint, guarded like metadata/type and returning 404 for
both unknown and unreadable), caches the answer, and then runs the ordinary
peer operation. No existing peer DTO or perimeter handler changes.
Incidentally more precise than the guid routes: those synthesize
TargetDrive { Alias = driveId, Type = Empty } and let the remote match on
alias alone. Resolution returns the real pair.
Peer *writes* are not covered. V2DrivePeerWriteController takes the remote
drive from the multipart instructions body rather than the route, so
resolve-then-delegate does not reach it; that needs the transfer
instruction set to carry a slug address.
Built, not exercised -- no runtime verification of route registration, the
perimeter round-trip, or the 404 paths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Test the slug-addressed routes, and fix what that turned up
43 tests across the four route groups: the local /apps/{appSlug}/drives
routes, the peer reads, the peer temporal reads, and the peer writes. Every
peer read is compared against its guid twin on the same drive -- a slug form
that merely succeeds is not the bar, it has to name the same drive.
Three things the tests found, none of which compiling had caught.
The perimeter resolver required Read or Write on the drive, and a
ConditionalTemporalRead grant is neither. Every temporal slug route was
therefore unusable by exactly the caller it exists for, the same way the
deposit-only writer was before. Any grant on the drive now resolves: holding
one already proves the caller knows of the drive.
Mail's app left both BuiltinApps lists but CircleConstants still grants
MailDrive from the two system circles, so identity setup threw
invalidGrantNonExistingDrive before finishing. MailDrive joins Lists and
Moments in SystemCircleCarryOverDrives, which exists for this exact case.
Moments and Vault joined Builtin, and a built-in app is registered at setup,
which needs a registration request. Added both.
Test helpers gained what the routes need to be exercised: CreateDrive takes
appId/driveSlug/driveTypeSlug, RegisterApp takes appSlug.
Full V2 project: 539 passed, 0 failed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Mint a write-only keypair for every drive
The Drives.WriteOnlyKeyPair column has been in the schema since
TableDrivesMigrationV202608040942 and nothing has ever read or written it.
This fills it in: an ECC-384 keypair whose private half is escrowed under the
drive's own storage key, so anyone can seal a deposit to a drive but only a
holder of the storage key can unseal it -- deposit custody is exactly
existing read access, at no extra grant.
Minted at creation because that is the one moment the storage key is
guaranteed in hand; it is derived from the master key two lines earlier. No
later path can be relied on: the caller who most needs the public half is a
stranger fetching it over peer to deposit, and that request carries no grant
on the drive, so nothing in it can reach the storage key the private half is
escrowed under. The v14 -> v15 migration mints for drives that predate this,
the same shape as the v11 -> v12 backfill of the connection-level keypair. It
never replaces an existing keypair -- that would strand every deposit already
sealed to the old public half.
ToRecord had to be fixed first. It backs every drive upsert and omitted the
field, so any rename, re-mode or archive wrote NULL over the key. Harmless
while nothing minted one; silent and permanent key loss now that creation
does. Two of the new tests fail without that line.
Note this departs from docs/drive-addressing.md on one point, deliberately
and pending a decision: the doc has a drive opt in via an AllowDeposits flag
(:127) and reads NULL as "deposits not enabled" (:240). AllowDeposits does
not exist in code. Minting unconditionally means NULL will only ever mean
"created before v15", and the flag -- when it lands -- would gate serving the
key rather than having one.
12 tests: six on the escrow itself (opens with the storage key, not with
another; survives the column round-trip with the private half intact), six on
creation, distinctness, the seeded drives, and the ToRecord regression.
Full V2 project: 545 passed, 0 failed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Stop capturing the peer query service twice
CI builds with --warnaserror, where CS9107 is fatal: each slug-addressed peer
controller took PeerDriveQueryService as a primary-constructor parameter,
passed it to the base, and then also used it in its own body -- capturing it
in both. Locally this is only a warning, which is why it built here and broke
there.
The base now exposes it once as a protected property and the derived
controllers go through that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Put Mail back on the tree, and the three that displaced it back where they were
Rewriting both BuiltinApps lists by hand in "Test the slug-addressed routes"
changed which apps an identity is provisioned with, in ways that commit did not
intend and its V2-only test run could not see.
Mail was dropped from both lists outright. EnsureAppsAsync walks Builtin, so the
Mail app stopped being registered at setup -- "there should be 3 app grants;
mail, chat, and the app created in this test" is that, stated by the test. With
MailAppId off the tree the slug generator no longer recognised it either and
slugged it off its display name as homebase-mail, which is exactly the bug the
generator reads the tree to avoid.
Webdrop, Moments and Vault went the other way, from Wellknown into Builtin. That
seeds WebDropDrive, WalletDrive and VaultDrive for every identity, none of which
are in BuiltinDrives.Protected -- 17 drives provisioned against a Protected of
14. Protected's own remark says to keep the two in step, and the initialize test
says WalletDrive is deliberately absent because it is no longer seeded. Webdrop's
promotion also silently reversed "Webdrop is known, not shipped with every
identity".
Membership returns to what it was before that commit. Lists stays commented out
of Wellknown, which that commit did deliberately and explained.
MailDrive leaves SystemCircleCarryOverDrives with it. It only joined to stop
identity setup throwing invalidGrantNonExistingDrive once its app was gone; with
Mail owning it in Builtin again it is a seeded drive, and the carry-over list is
back to the two whose apps really are not built-in. Provisioned drives are again
the 14 Protected names.
The seven tests that failed on this branch since that commit now pass:
AppSlugGeneratorTests.KnownSystemAppsGetTheirName, the two
SystemInitializeConfigTests drive-count tests, and the five
CircleNetworkServiceAppTests app-grant tests. Odin.Services.Tests: 573 passed,
0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Revert "Put Mail back on the tree, and the three that displaced it back where they were"
This reverts commit 0c4815d6a3a4e42425a48dc21b2a1dfc525ad25b.
* Make the tests say what the tree now says: no Mail app, Moments and Vault built-in
The tree changed in "Test the slug-addressed routes" and the tests were never
brought with it, which is what has been red since. Mail's app is gone for good,
Moments and Vault ship with every identity; these tests still described the world
before that.
Mail is no longer a known system app, so the slug generator has nothing to say
about it -- KnownSystemAppsGetTheirName stops asserting a slug for an app the
tree does not name.
Only Chat and Mail authorised the system circles, so with Mail unregistered a
new connection carries one built-in app grant instead of two. The five
CircleNetworkServiceAppTests counts go 3 -> 2.
Protected regains the drives provisioning actually creates, WebDropDrive and
VaultDrive, and is again the same set: 16 either way. Its remark asks for the
two to be kept in step and they had drifted.
Two things this turned up that were not just test text.
VaultAppRegistrationRequest granted ReadWrite on WalletDrive. Vault is built-in,
so it is registered at setup, and a grant for a drive that does not exist throws
-- with WalletDrive no longer seeded, identity setup failed outright for every
new identity, not only in these tests. The grant goes. WalletDrive is now owned
by nothing, so an identity that already has one keeps it with a derived slug and
no owning app, reachable by guid rather than by an /apps/{appSlug}/drives path.
Both v13->v14 remarks that said otherwise are corrected.
WebDropDrive is anonymous-read and Webdrop is built-in, so HandleDriveAdded
grants the system circles read on it as it is created. The connected circle
holds 10 drive grants now, not 9, and the test says why.
Odin.Services.Tests: 567 passed, 0 failed. The nine tests in the two affected
Hosting.Tests classes all pass, including the two that only failed once
WalletDrive stopped being seeded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Serve a drive's write-only public key over peer, and show it in the console
The keypair has been minted and stored since "Mint a write-only keypair for every
drive", but nothing could read it: no endpoint served the public half and no API
returned it, so the thing that makes write-without-read possible was invisible.
GET /api/v2/peer/{odinId}/apps/{appSlug}/drives/{driveSlug}/public-key, in the
three parts the slug routes already use -- outgoing controller, refit client,
perimeter. The perimeter resolves the address itself rather than calling
ResolveDriveAddressAsync, which accepts any grant: naming a drive discloses
nothing, but handing out the means to write to it does.
Write on the drive is required, and only Write. A reader has no use for this key,
and a caller without Write gets a security exception rather than a 404 -- unlike
resolution, which conflates the two so nobody can enumerate drives by name. Here
the caller has already resolved the address, so the drive's existence is not a
secret from it, and a 404 would send it hunting for a drive that is there.
The public half is not cached on the outgoing side, though the address is. A slug
is immutable; a keypair is not. A cached public half would keep this identity
sealing to a key the remote had rotated away from -- deposits nobody can open,
with nothing anywhere to say so.
EnsureWriteOnlyKeyPairAsync now takes the storage key from the caller's grant
rather than by decrypting MasterKeyEncryptedStorageKey with the master key. Same
key either way, but the grant is what the escrow actually means: the private half
is sealed under the key that grants access to this drive.
OwnerClientDriveData carries the public half and its crc32 so the owner console
can show which key a drive holds. Public half only -- the private half stays
escrowed and never leaves the server, and the peer endpoint hands this same value
to any caller with write access.
Tests. PeerAppDrivePublicKeyTests: a writer gets the key, a reader is refused with
403, unknown app and drive slugs are not found. The one that matters seals to the
key the endpoint served and has the drive's owner open it -- "a key came back"
would pass against any key at all.
DriveWriteOnlyKeyPairTests gains the escrow assertions it was missing.
ThePrivateHalfIsEscrowedUnderThatDrivesStorageKey was named for a claim it never
checked: it compared two public halves and never opened anything. It now fetches
the drive's real storage key and opens the private half with it, alongside the
negatives (another drive's storage key, the master key) and a seal/unseal round
trip that survives every write path through ToRecord.
docs/drive-addressing.md: "lazily on first request" is struck through rather than
deleted, with why. Lazy minting cannot work -- minting needs the drive's storage
key and the caller who triggers the first reques…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The second half of the Cat 3 groundwork. Stacked on
circle-definitions-to-table(#1661) — which is itself stacked onreview-stamp-cat1(#1649). Stacked rather than parallel because both add a version migration and would otherwise collide onDataVersionNumber.Why
AppRegistrationsshipped with part 1's schema work and has no caller —AppRegistrationServicestill writes to the sharedKeyThreeValueblob. Part 1 is explicit that this one isn't optional:It also gives
Circle.AppIdandDrives.AppIda real target, and makes delete app ⇒ delete its drives expressible in SQL.The slug problem
AppSlugisNOT NULL+UNIQUE(identityId, AppSlug), 1–12 chars,^[a-z0-9]([a-z0-9-]*[a-z0-9])?$. No existing registration has one, so the migration has to coin them — the one place in the system that derives a slug rather than being handed one.chat,feed,mail,photo,owner— the addresses drive addressing assumes"Acme Receipts"→acme-receipt"!!! ???"→a1b2c3d4e5f6)Resolved and validated as a whole set before anything is written.
GenerateAllorders known apps first — so an app literally named "Chat" cannot take the chat app's address — disambiguates collisions with a numeric suffix, and throws rather than returning a duplicate. The migration re-checks distinctness on top. A half-migrated app table with a slug collision is much worse than a migration that refuses to start.What changed
Promote-don't-duplicate, same rule as
ReviewedAtand the circle columns:AppId,AppSlug,Name,CorsHostNameare columns and[JsonIgnore]d out ofgrantJson.AppKeyStore,AuthorizedCircles,CircleMemberPermissionGrantstill ride the JSON.Nine storage call sites in
AppRegistrationServicemove fromThreeKeyValueStoragetodb.AppRegistrations, behindSaveAsync/ToRecord/FromRecord.Registration derives a slug the same way, so registering an app and migrating one land on the same value.
AppRegistrationRequesthas no slug field yet — an app declaring its own address arrives with drive addressing.Updates carry the slug forward. It's immutable, and other identities may already hold it. Dedupe seeds from stored slugs rather than re-derived ones, so an app holding
acme-2still holds it whatever its name would slugify to today.v14 → v15moves the rows. Idempotent and additive: an app already present is skipped so its slug is never reassigned; blob rows are left in place as a fallback.Testing
18 unit tests on
AppSlugGenerator— system apps keep their names, collisions disambiguate, a system app never yields its slug to a derived one, unslugifiable and null names still produce something valid, length cap holds, duplicate app ids are rejected.Full solution suite green: 2,542 passed, 0 failed (68 skipped, all pre-existing
[Explicit]).Worth a reviewer's eye
Nameis now a column, so it no longer appears ingrantJson. Nothing reads registrations out of the blob directly, but that's the kind of thing worth a second opinion.🤖 Generated with Claude Code