MEFORORG · wshallwshall · Aug 27, 2026
@@ -12398,6 +12398,35 @@ BUILDS it.*
 **Provenance.** Diagnosed by the lane whose own commit tripped it: seven tests in one file failed in a full run and **passed in isolation, twice**. It reported the negative control alongside the fix -- restoring the bare import reproduced exactly those seven failures -- which is what makes the green meaningful. Recorded here rather than left in session mail because the collision outlives the commit that revealed it. It is the same shape as the rest of this cluster: **the name resolved to the neighbouring module, and nothing said so.**
 
 ## 1256. the federated binding guards account-continuity but never subject-exclusivity, so two accounts can bind one identity
+> **THE ATOMICITY HALF IS BUILT 2026-08-27; banner left open for the archive pass.** A partial/filtered
+> unique index `ux_users_federated_subject` on `(oidc_issuer, oidc_subject)` now exists on all three
+> backends, and the race loser is rendered as the SAME `federated_subject_already_bound` outcome the
+> sequential path returns rather than a 500.
+>
+> **THE ACCEPTANCE DEMONSTRATES THE RACE, as this row's correction demands.** Two concurrent first
+> logins for one subject, interleave FORCED by an `asyncio.Barrier` so both reads complete before
+> either write. Removing the index reds it: both bind. **A second test asserts both logins observed
+> NO holder** -- without that, a future refactor that quietly serialises them would leave the first
+> test green while the index was never consulted, which is this row's own failure mode one level up.
+>
+> **THREE CORRECTIONS MADE WHILE BUILDING, each found by asking what the codebase already does:**
+> 1. The SQLite index CANNOT live in `_SCHEMA`: it runs at `store.py:2087`, `_migrate` at `:2088`, so
+>    on a users table predating the federated columns it references a column that does not exist yet.
+>    It sits in `_migrate` beside `ix_queue_body_ref`, which records that same reasoning.
+> 2. SQL Server needed a RE-TYPE migration, not just a declaration change. The `ALTER TABLE ... ADD`
+>    guards are `COL_LENGTH(...) IS NULL` and fire only when a column is ABSENT, so an existing table
+>    keeps `NVARCHAR(MAX)` -- which cannot be an index key. `COL_LENGTH` returns **-1** for MAX, which
+>    is the discriminator. NVARCHAR(256), not 450: 2x256x2 = 1024 bytes, inside the 1700-byte limit.
+> 3. **A contract exception was added and REVERTED.** `FederatedSubjectConflict` in `store/base.py`
+>    existed so `auth/` would not import three drivers -- but `auth/service.py:2558` already solves
+>    that without one, joining the class MRO names and testing for `Integrity`/`UniqueViolation`
+>    (ADR 0068 4's duplicate-label race, the same check-then-act shape). Sound reasoning, wrong
+>    mechanism, because the constraint it designed around had already been dissolved. Do not re-add.
+>
+> **The `WHERE ... IS NOT NULL` filter is stylistic on SQLite and Postgres and REQUIRED on SQL Server**,
+> where NULLs compare EQUAL in a unique index -- unfiltered, it would admit exactly ONE unfederated
+> user in the table. A reader who learned the rule from the SQLite file would delete it as redundant.
+>
 
 > 🔢 **Re-scored 2026-08-20 -> P2.** Value **5/10** · Difficulty **4/10** · _fill-in_. The behavioural half is closed in shipped code -- auth/service.py:1219-1232 refuses a second account binding one (issuer, subject), with the lookup on all three backends and a sequential test at tests/test_auth_oidc_service.py:456 -- so the remainder is only the database constraint that makes the read-then-write at :1219/:1232 atomic, which is worth a 5 as auth hardening with the app guard covering every non-concurrent case. Difficulty drops to 4 because the amendment moved the design half (refuse rather than re-point) into shipped code and comment, leaving a well-precedented seam: all three backends already declare unique indexes (ux_webauthn_label, ux_search_presets_owner_name), so the novel cost is one first-of-kind SQL Server ALTER COLUMN off NVARCHAR(MAX) (store/sqlserver.py:1358; the file contains zero ALTER COLUMN today), a filtered index for SQL Server NULL semantics, and a CI-only concurrent-bind test on the two server backends. _(was 7/10 · 5/10.)_
 >

@@ -1269,9 +1269,34 @@ async def _complete_ad_login(
             # First federated login for this account (or an unbound AD account's first): record the
             # (issuer, sub) binding so a later reassigned-username login carrying a different subject is
             # refused by the guard above. A matching binding is left untouched (no updated_at churn).
-            await self._store.set_user_federated_subject(
-                user.id, federated_subject[0], federated_subject[1]
-            )
+            try:
+                await self._store.set_user_federated_subject(
+                    user.id, federated_subject[0], federated_subject[1]
+                )
+            except Exception as exc:
+                # BACKLOG #1256. THE GUARD ABOVE IS CHECK-THEN-ACT: its read and this write are
+                # separate awaits, so two concurrent FIRST logins for one subject can both see
+                # `holder is None` and both reach here. `ux_users_federated_subject` refuses the
+                # loser on all three backends, and this renders that refusal as the SAME outcome the
+                # sequential path returns -- otherwise the race loser gets a 500 for a condition the
+                # gate handles cleanly one microsecond earlier.
+                #
+                # MRO BY NAME, matching the duplicate-label race at `_enroll_webauthn` (ADR 0068 4):
+                # each backend raises its own integrity class -- sqlite3.IntegrityError, asyncpg's
+                # UniqueViolationError, pyodbc's IntegrityError -- and naming them here would make
+                # this module import-aware of every driver and silently stop covering a backend added
+                # later. Anything that is NOT an integrity violation re-raises untouched.
+                mro = "".join(t.__name__ for t in type(exc).__mro__)
+                if "Integrity" not in mro and "UniqueViolation" not in mro:
+                    raise
+                await self._directory_reject_audit(
+                    principal.username, "oidc", "federated_subject_already_bound"
+                )
+                return LoginOutcome(
+                    ok=False,
+                    error="federated sign-in failed",
+                    reason="federated_subject_already_bound",
+                )
             # BACKLOG #1248. Binding an external identity decides WHO MAY SIGN IN as this account
             # from now on, so it is a privilege change and gets the same two records the role
             # resync below emits: an audit row, and an out-of-band notice to the account holder

@@ -545,6 +545,14 @@ def __init__(self, method: str, outbox_ids: tuple[str, ...]) -> None:
         oidc_subject         TEXT,
         password_claimed_at  DOUBLE PRECISION
     )""",
+    # BACKLOG #1256: the atomicity the CHECK-THEN-ACT guard in auth/service.py cannot give itself --
+    # its read and its write are separate awaits, so two concurrent FIRST logins for one subject can
+    # both observe "no holder" and both bind. Same shape as ux_webauthn_label (ADR 0068 4).
+    # PARTIAL so unfederated rows coexist; Postgres treats NULLs as distinct, so the WHERE states
+    # intent here and is REQUIRED on SQL Server, where NULLs compare equal.
+    """CREATE UNIQUE INDEX IF NOT EXISTS ux_users_federated_subject
+        ON users(oidc_issuer, oidc_subject)
+        WHERE oidc_issuer IS NOT NULL AND oidc_subject IS NOT NULL""",
     """CREATE TABLE IF NOT EXISTS roles (
         id           TEXT PRIMARY KEY,
         display_name TEXT NOT NULL,

@@ -1376,7 +1376,12 @@ def __init__(self, conn: Any, cur: Any) -> None:
         channel_scope NVARCHAR(MAX) NULL, totp_secret NVARCHAR(MAX) NULL,
         totp_enabled BIT NOT NULL DEFAULT 0, totp_enrolled_at FLOAT NULL,
         totp_recovery_codes NVARCHAR(MAX) NULL, last_totp_step INT NULL,
-        oidc_issuer NVARCHAR(MAX) NULL, oidc_subject NVARCHAR(MAX) NULL,
+        -- BACKLOG #1256: SIZED, NOT MAX, BECAUSE A MAX COLUMN CANNOT BE AN INDEX KEY on SQL
+        -- Server, and ux_users_federated_subject below is what makes the application guard's
+        -- read-then-write atomic. 256 is this file's dominant width (55 uses); the composite
+        -- key is then 2 x 256 x 2 bytes = 1024, inside the 1700-byte nonclustered limit.
+        -- NVARCHAR(450), the other precedent here, would be 1800 across two columns and fail.
+        oidc_issuer NVARCHAR(256) NULL, oidc_subject NVARCHAR(256) NULL,
         password_claimed_at FLOAT NULL)""",
     """IF COL_LENGTH('users','channel_scope') IS NULL
         ALTER TABLE users ADD channel_scope NVARCHAR(MAX) NULL""",
@@ -1395,9 +1400,28 @@ def __init__(self, conn: Any, cur: Any) -> None:
     # Federated (issuer, sub) identity keying (BACKLOG #1015): COL_LENGTH-gated ADD on a pre-existing
     # users table. NULL on existing rows = "not yet federated" (username stays the sole key). Idempotent.
     """IF COL_LENGTH('users','oidc_issuer') IS NULL
-        ALTER TABLE users ADD oidc_issuer NVARCHAR(MAX) NULL""",
+        ALTER TABLE users ADD oidc_issuer NVARCHAR(256) NULL""",
     """IF COL_LENGTH('users','oidc_subject') IS NULL
-        ALTER TABLE users ADD oidc_subject NVARCHAR(MAX) NULL""",
+        ALTER TABLE users ADD oidc_subject NVARCHAR(256) NULL""",
+    # BACKLOG #1256: RE-TYPE A PRE-EXISTING MAX COLUMN, WHICH THE COL_LENGTH-GATED ADDs ABOVE CANNOT
+    # REACH. They fire only when the column is ABSENT, so a users table created before this change
+    # keeps NVARCHAR(MAX) -- and a MAX column CANNOT BE AN INDEX KEY, so the index below would fail
+    # against exactly the databases that already exist. COL_LENGTH returns -1 for a MAX column, which
+    # is how this tells "already sized" from "needs re-typing" without reading catalogue views.
+    #
+    # MUST RUN BEFORE the index. _SCHEMA is applied in order, so position here is load-bearing.
+    """IF COL_LENGTH('users','oidc_issuer') = -1
+        ALTER TABLE users ALTER COLUMN oidc_issuer NVARCHAR(256) NULL""",
+    """IF COL_LENGTH('users','oidc_subject') = -1
+        ALTER TABLE users ALTER COLUMN oidc_subject NVARCHAR(256) NULL""",
+    # The atomicity the application guard cannot provide: auth/service.py reads the current holder and
+    # writes the binding in two separate awaits, so two concurrent FIRST logins for one subject can
+    # both observe "no holder" and both bind. FILTERED so unfederated rows coexist -- and on SQL Server
+    # the filter is REQUIRED, not stylistic: unlike SQLite and Postgres it treats NULLs as EQUAL in a
+    # unique index, so an unfiltered index would permit exactly ONE unfederated user in the table.
+    """IF INDEXPROPERTY(OBJECT_ID('users'),'ux_users_federated_subject','IndexID') IS NULL
+        CREATE UNIQUE INDEX ux_users_federated_subject ON users(oidc_issuer, oidc_subject)
+        WHERE oidc_issuer IS NOT NULL AND oidc_subject IS NOT NULL""",
     # Claimed-ness of the bootstrap admin (BACKLOG #1245): NULL on an existing row would read as
     # "never claimed", which is what would retire an account whose holder claimed it long ago — this
     # defect, re-introduced by its own fix. So the ADD is paired with a one-time backfill: a local

@@ -3169,6 +3169,25 @@ async def _migrate(db: aiosqlite.Connection) -> None:
         # The body_ref deref index lives here (not _SCHEMA) so it is created only AFTER the column is
         # guaranteed present — on a Step-A queue it'd otherwise reference a not-yet-added column.
         await db.execute("CREATE INDEX IF NOT EXISTS ix_queue_body_ref ON queue(body_ref)")
+        # BACKLOG #1256: the federated guard in auth/service.py is CHECK-THEN-ACT -- it reads the
+        # current holder and writes the binding in two separate awaits, so two concurrent FIRST
+        # logins for one subject can both observe "no holder" and both bind. This index is the
+        # atomicity that guard cannot give itself. Same shape as ux_webauthn_label and ADR 0068 4's
+        # concurrent-enroll race: the loser's driver error is rendered by the caller as the same
+        # refusal its pre-check returns.
+        #
+        # HERE RATHER THAN _SCHEMA, for the reason ix_queue_body_ref states one line up: _SCHEMA runs
+        # BEFORE this migration, and on a users table predating the federated columns the index would
+        # reference a column that does not exist yet.
+        #
+        # PARTIAL so unfederated rows coexist. SQLite treats NULLs as distinct here so the WHERE is
+        # not strictly required -- it is written anyway because on SQL Server the same filter IS
+        # required (NULLs compare EQUAL there, so an unfiltered index admits ONE unfederated user).
+        await db.execute(
+            "CREATE UNIQUE INDEX IF NOT EXISTS ux_users_federated_subject"
+            " ON users(oidc_issuer, oidc_subject)"
+            " WHERE oidc_issuer IS NOT NULL AND oidc_subject IS NOT NULL"
+        )
         # BACKLOG #154: a DB whose `response` table predates resp_headers gains it here (NULL on existing
         # rows = "no captured headers", byte-identical). The table itself is created by _SCHEMA.
         cur = await db.execute("PRAGMA table_info(response)")