Pre-push review (round 4): T-2291/bugfix-remove-site-membership-quarantine-refresh

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+Sites.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+Sites.swiftindex eef8ff2..bcadbe2 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+Sites.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+Sites.swift@@ -185,6 +185,18 @@ extension LibraryRepository {                     operation: "removing a site membership",                     reason: String(describing: error))             }+            // T-2291: publish what this commit's own validation left behind, for+            // every hostname it wrote to — the step the deletion commit and the+            // teaching commits take, and the one this path was missing.+            //+            // A removal is the repair for exactly the diagnosis the removed row+            // caused, so without this the hostname stays quarantined until the+            // next launch: the foreground refresh carries the tuple set forward+            // (`LibraryDiagnostics.union`) rather than re-deriving it, so a stale+            // entry never falls out on its own. A diagnosis the removal left+            // standing is re-recorded unchanged, so nothing it did not repair is+            // cleared.+            self.publishPostCommitDiagnoses(across: hostnames, in: diagnoses)         }     } 

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swiftindex ad0b45f..05f0653 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift@@ -115,11 +115,21 @@ extension LibraryRepository {                 row.modifiedAt = urlTimestamp             } -            // Validate the complete prospective graph (V4), then save once. An-            // invalid result on the affected Site rolls back and reports.+            // Validate the prospective graph, then save once. An invalid result+            // on the affected Site rolls back and reports.+            //+            // T-2291: **scoped to the hostname this commit wrote to.** A Work URL+            // lands on one membership, and the gate below and the publication+            // after the save both read that hostname alone — so validating the+            // whole library replayed every rule against every Entry to answer a+            // question about one site. The scoped overload gives a hostname+            // holding a Site row the answer a whole-graph validation would give+            // it, and reports the tuple class alone, as `quarantineMap()` does —+            // read its own doc for the arms where the two derivations disagree.             let diagnoses: [String: LibraryValidationError]             do {-                diagnoses = try LibraryValidator.validate(context: context).quarantineMap()+                diagnoses = try LibraryValidator.validate(+                    hostnames: [hostname], context: context)             } catch {                 context.rollback()                 workMergeLogger.error(@@ -457,12 +467,18 @@ extension LibraryRepository {             // partial merge the old refusal was avoiding.             for row in sourceGroup.rows { context.delete(row) } -            // Validate the complete prospective graph, then save once. Merge is-            // the highest-risk mutation (Work deletion plus bulk reassignment);-            // an invalid result rolls back and reports instead of persisting damage.+            // Validate the prospective graph, then save once. Merge is the+            // highest-risk mutation (Work deletion plus bulk reassignment); an+            // invalid result rolls back and reports instead of persisting damage.+            //+            // T-2291: scoped to `mergeHostnames` — every site of both Works, and+            // therefore every site this merge wrote to. That is already the set+            // the gate and the publication read, so the whole-library pass was+            // deriving diagnoses for hostnames neither of them would consult.             let diagnoses: [String: LibraryValidationError]             do {-                diagnoses = try LibraryValidator.validate(context: context).quarantineMap()+                diagnoses = try LibraryValidator.validate(+                    hostnames: Array(mergeHostnames), context: context)             } catch {                 context.rollback()                 workMergeLogger.error(

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+DuplicateResolution.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+DuplicateResolution.swiftindex ad143b6..661107a 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+DuplicateResolution.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+DuplicateResolution.swift@@ -798,9 +798,12 @@ extension LibraryRepository {     private func commitResolution(         context: ModelContext, hostnames: [String], operation: String     ) throws -> DuplicateResolutionOutcome? {+        // T-2291: scoped to `hostnames` — every site the resolution wrote to,+        // which is already the set the gate and the publication below read. The+        // whole-library pass was deriving diagnoses neither of them consults.         let diagnoses: [String: LibraryValidationError]         do {-            diagnoses = try LibraryValidator.validate(context: context).quarantineMap()+            diagnoses = try LibraryValidator.validate(hostnames: hostnames, context: context)         } catch {             context.rollback()             _ = try? context.fetch(FetchDescriptor<Entry>())

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+ComposedTeaching.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+ComposedTeaching.swiftindex 4941a08..1bc142e 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+ComposedTeaching.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+ComposedTeaching.swift@@ -82,10 +82,6 @@ extension LibraryRepository {         return try await withLockedContext(mode: .exclusive, operation: "committing composed teaching") { context in             let hostname = contract.basis.hostname -            // What this hostname was already diagnosed with, read before any-            // mutation. Step 7 rolls back only when the commit *changes* it.-            let priorDiagnosis = self.quarantineReason(hostname: hostname)-             // 1. Refetch basis and re-project.             let currentBasis: ComposedTeachingBasis             do { currentBasis = try self.buildComposedTeachingBasis(hostname: hostname, context: context) }@@ -226,16 +222,24 @@ extension LibraryRepository {             // `quarantineMap` is the projection this guard has always compared             // against — an illegal tuple, or now a second Site row.             //-            // **The comparison is against `priorDiagnosis`, not against nil**-            // (Req 3.2, 3.3, Decision 8). This guard used to roll back whenever-            // the hostname carried any diagnosis afterwards, which made a-            // diagnosed hostname impossible to re-teach: the app reported+            // **The comparison is against the hostname's prior diagnosis, not+            // against nil** (Req 3.2, 3.3, Decision 8). This guard used to roll+            // back whenever the hostname carried any diagnosis afterwards, which+            // made a diagnosed hostname impossible to re-teach: the app reported             // something wrong and then refused the only action that would fix             // it. Equality is the comparison, not a severity order —             // `LibraryValidationError` has no ordering that would not be invented —             // so an unchanged diagnosis commits and a changed one rolls back.             // Clearing is `diagnoses[hostname] == nil`, which is never a             // difference worth rolling back for.+            //+            // T-2291: through `introducedDiagnosis(across:in:)`, like every other+            // commit gate. The rule was spelled inline here and in+            // `commitRecalculation` below, which is a second place it can drift+            // from the shared helper. The helper reads the quarantine latch+            // itself — untouched until this commit publishes — so reading it+            // here, after the mutation and before the save, is still reading the+            // pre-write answer.             let diagnoses: [String: LibraryValidationError]             do {                 diagnoses = try LibraryValidator.validate(context: context).quarantineMap()@@ -243,11 +247,11 @@ extension LibraryRepository {                 context.rollback()                 return .invalidated(reason: "composed teaching produced an invalid library: \(error)")             }-            if let diagnosis = diagnoses[hostname], diagnosis != priorDiagnosis {+            if let introduced = self.introducedDiagnosis(across: [hostname], in: diagnoses) {                 context.rollback()                 return .invalidated(                     reason: "composed teaching would introduce a new diagnosis on Site "-                        + "'\(hostname)': \(diagnosis)")+                        + "'\(hostname)': \(introduced.diagnosis)")             }              do { try self.saveStrategy.save(context) }@@ -298,7 +302,6 @@ extension LibraryRepository {     ) async throws -> ComposedRecalculationOutcome {         try await withLockedContext(mode: .exclusive, operation: "committing recalculation") { context in             let hostname = contract.basis.hostname-            let priorDiagnosis = self.quarantineReason(hostname: hostname)              let currentBasis: ComposedTeachingBasis             do { currentBasis = try self.buildComposedTeachingBasis(hostname: hostname, context: context) }@@ -359,14 +362,14 @@ extension LibraryRepository {                 context.rollback()                 return .invalidated(reason: "recalculation produced an invalid library: \(error)")             }-            // The same comparison as the composed commit, for the same reason:-            // roll back only when the recalculation changed the hostname's-            // diagnosis (Req 3.2, 3.3, Decision 8).-            if let diagnosis = diagnoses[hostname], diagnosis != priorDiagnosis {+            // The same comparison as the composed commit, through the same+            // helper, for the same reason: roll back only for a diagnosis the+            // recalculation itself introduced (Req 3.2, 3.3, Decision 8).+            if let introduced = self.introducedDiagnosis(across: [hostname], in: diagnoses) {                 context.rollback()                 return .invalidated(                     reason: "recalculation would introduce a new diagnosis on Site "-                        + "'\(hostname)': \(diagnosis)")+                        + "'\(hostname)': \(introduced.diagnosis)")             }              do { try self.saveStrategy.save(context) }

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkDeletion.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkDeletion.swiftindex dbd2a46..8e6885a 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkDeletion.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkDeletion.swift@@ -213,8 +213,11 @@ extension LibraryRepository {             // relationship graph the way Merge does, so it takes Merge's             // validator discipline (Q21) — but compared against the *prior*             // diagnosis rather than against nil, or a hostname that was already-            // diagnosed would have undeletable works (Decision 8 of-            // `specs/title-teaching-retroactive-parsing`).+            // diagnosed would have undeletable works (Q67 of+            // `specs/multi-site-works`, which extends Decision 8 of+            // `specs/library-integrity-tolerance` to this gate; the citation+            // here used to name `specs/title-teaching-retroactive-parsing`,+            // whose Decision 8 is about backup V2 support).             let diagnoses: [String: LibraryValidationError]             do {                 diagnoses = try LibraryValidator.validate(

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository.swiftindex 1be2d31..30f12ba 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository.swift@@ -146,12 +146,12 @@ public actor LibraryRepository {     /// Decision 7 predicts exactly this failure; `RefreshUnionInvariantTests` is     /// the regression.     ///-    /// The carry-forward is a *cache* of the last full validation, so the two-    /// passes that can repair a hostname without one invalidate it themselves:-    /// a teaching commit through `recordPostCommitDiagnosis`, and a reconcile-    /// pass through the re-validation in `reconcileAfterSync`. Both re-validate-    /// before they clear; neither drops an entry for having written to the-    /// hostname.+    /// The carry-forward is a *cache* of the last full validation, so every pass+    /// that can repair a hostname without one invalidates it itself: each commit+    /// gate through `publishPostCommitDiagnoses` or `recordPostCommitDiagnosis`,+    /// and a reconcile pass through the re-validation in `reconcileAfterSync`.+    /// All of them re-validate before they clear; none drops an entry merely for+    /// having written to the hostname.     ///     /// App only. The extension runs the full validator once at open and nothing     /// after it: it has no foreground and no surface that reports diagnoses@@ -799,9 +799,15 @@ public actor LibraryRepository {     /// hostname it wrote to — the diagnosis it left behind, or nil, which     /// clears the quarantine. A commit that skips this leaves a hostname it     /// repaired quarantined until the next launch, because the foreground-    /// refresh cannot re-derive the tuple set. The teaching commit always did-    /// this through `recordPostCommitDiagnosis`; the Work URL, Merge and-    /// duplicate-resolution commits now do too.+    /// refresh cannot re-derive the tuple set.+    ///+    /// Every commit gate that validates and saves owes this step, over the same+    /// hostnames it gated on. Some reach it through `recordPostCommitDiagnosis`+    /// directly, because they write one hostname (the teaching commits) or+    /// already hold the diagnoses in a loop (the deletion commit). No caller+    /// list is kept here: both omissions this helper was written for (T-2289,+    /// T-2291) were found by reading a gate, not by reading a list — and a list+    /// is one more thing that goes stale when a gate is added.     func publishPostCommitDiagnoses(         across hostnames: some Sequence<String>, in diagnoses: [String: LibraryValidationError]     ) {

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryValidator.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryValidator.swiftindex 8752e71..0cbc80d 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryValidator.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryValidator.swift@@ -163,14 +163,22 @@ public enum LibraryValidator {     ///     /// The same per-row arm `validate(graph:)` runs — `validate(site:)`,     /// `validate(work:)`, `validate(entry:)`, in that order, first failure per-    /// hostname wins — over a graph narrowed to the hostnames asked about. The-    /// answer for a hostname is therefore the answer a whole-graph validation-    /// would give it, at the cost of that hostname rather than of the library.+    /// hostname wins — over a graph narrowed to the hostnames asked about. For a+    /// hostname that **has a Site row** and holds no split Work group the answer+    /// is the answer a whole-graph validation would give it, at the cost of that+    /// hostname rather than of the library. The two divergences are spelled out+    /// below.     ///-    /// It exists for `reconcileAfterSync`, whose question after a repair is about-    /// the one or two hostnames the pass consolidated (Req 2.2). Running the full-    /// validator to answer it replayed every rule against every Entry in the-    /// library on every arrival that repaired anything.+    /// It was written for `reconcileAfterSync`, whose question after a repair is+    /// about the one or two hostnames the pass consolidated (Req 2.2); it is now+    /// the validator of every commit gate **but the two teaching commits** —+    /// `commitComposedTeaching` and `commitRecalculation` still run+    /// `validate(context:)` over the whole library (T-2291 moved the other gates+    /// onto this overload, taking up T-1956 follow-up (c), which named+    /// `commitWorkURL`; the teaching pair was left alone). Every gate on this+    /// overload likewise gates and publishes over the hostnames it wrote to and+    /// reads nothing else. Running the full validator to answer that replayed+    /// every rule against every Entry in the library on every such write.     ///     /// Three things the narrowing deliberately preserves:     ///@@ -182,9 +190,33 @@ public enum LibraryValidator {     ///   holding an Entry of another hostname resolves through the index here as     ///   it does in the full pass, so the cross-Site failure is still reported     ///   against the Entry's hostname and not manufactured against the Work's.-    /// - **A hostname with no Site row yields no diagnosis**, exactly as the full-    ///   pass does: that is `.siteMissing`, a tolerated state rather than a tuple-    ///   failure, and there is no row for a tuple to be illegal on.+    /// - **A hostname with no Site row yields no *tuple* diagnosis.** That is+    ///   `.siteMissing`, a tolerated state rather than a tuple failure, and there+    ///   is no row for a tuple to be illegal on.+    ///+    /// **Where it does not agree with the full pass (T-2291).** Two arms, in+    /// opposite directions.+    ///+    /// - *This pass reports less.* The whole-graph membership arm records under+    ///   `membership.hostname` with no Site check, so a *malformed membership row+    ///   on a hostname that has no Site row* is a diagnosis there and is not one+    ///   here: this pass `continue`s at the Site-winner guard below before it+    ///   reaches its membership loop. A library in that state is quarantined for+    ///   it at bootstrap and un-quarantined by any commit gate that publishes+    ///   over the hostname, while the damage stands.+    /// - *This pass reports more.* For a duplicated-UUID Work group the two+    ///   passes rank different candidate sets: this one picks the winner among+    ///   the rows reachable from the hostname, the full pass among every row in+    ///   the library, then reads the hostnames off that winner's memberships. A+    ///   group whose `GroupOrdering` winner carries no membership on this+    ///   hostname — the Req 8.1 lagging-writer state — is diagnosed here and not+    ///   there. Reporting more can only quarantine; it cannot clear anything the+    ///   full pass would hold.+    ///+    /// Neither pass repairs a row; the derivations only disagree about who is+    /// told. Read the paragraph above as a per-hostname equality for a hostname+    /// that holds a Site row and no split Work group, not as one across all+    /// inputs.     public static func validate(         hostnames: [String], context: ModelContext     ) throws -> [String: LibraryValidationError] {

diff --git a/Packages/AsterismCore/Tests/AsterismCoreTests/MembershipRemovalTests.swift b/Packages/AsterismCore/Tests/AsterismCoreTests/MembershipRemovalTests.swiftindex 88ce3c6..be0ddf6 100644--- a/Packages/AsterismCore/Tests/AsterismCoreTests/MembershipRemovalTests.swift+++ b/Packages/AsterismCore/Tests/AsterismCoreTests/MembershipRemovalTests.swift@@ -126,7 +126,14 @@ struct MembershipRemovalTests {     /// the diagnosis as "unknown", because there was none left to name. The     /// state removal exists to let the reader out of was the state it would not     /// let them out of.-    @Test("A removal that repairs the removed site's diagnosis commits")+    ///+    /// T-2291: and the repair has to be *published*, not just permitted. This+    /// path validated its own hostnames, gated on them and saved without ever+    /// calling `publishPostCommitDiagnoses`, so the site the removal had just+    /// repaired stayed quarantined for the rest of the session — the foreground+    /// refresh carries the tuple set forward (`LibraryDiagnostics.union`) rather+    /// than re-deriving it, so a stale entry never falls out on its own.+    @Test("A removal that repairs the removed site's diagnosis commits and clears its quarantine")     func removalThatClearsADiagnosisCommits() async throws {         let fixture = try await twoSiteFixture()         try await fixture.repository.setMembershipIdentity(@@ -141,11 +148,16 @@ struct MembershipRemovalTests {         #expect(             try await fixture.repository.membershipRows(of: Self.workID).map(\.hostname)                 == ["first.example"])-        // The cached quarantine is not asserted: unlike Work deletion, this path-        // does not republish the map after its save, so the removed site keeps a-        // stale entry until the next full validation. That is a separate-        // question from whether the removal was allowed, which is what this-        // test is about.+        // The quarantine the removed row caused went with it, without waiting+        // for the next launch (T-2291).+        #expect(await fixture.repository.quarantineReason(hostname: "second.example") == nil)+        // The latch alone is not the assertion: the tuple set the next+        // foreground refresh unions against has to have moved too, or the+        // refresh puts the hostname this removal repaired straight back into+        // quarantine.+        #expect(await fixture.repository.diagnostics.tupleDiagnoses["second.example"] == nil)+        try await fixture.repository.refreshDiagnostics()+        #expect(await fixture.repository.quarantineReason(hostname: "second.example") == nil)          // Q59's standing guard: the superseded columns still mirror the primary         // membership after every write this test made.@@ -155,6 +167,10 @@ struct MembershipRemovalTests {     /// found it is not one it introduced either, so the surviving site's     /// standing quarantine must not make the Work's other memberships     /// unremovable.+    ///+    /// T-2291's other arm too: the publication after the save re-records that+    /// standing diagnosis unchanged rather than clearing it, so a removal that+    /// repaired nothing on this hostname does not un-quarantine it.     @Test("A standing diagnosis on the surviving site does not refuse the removal")     func removalCommitsUnderAnUnchangedDiagnosis() async throws {         let fixture = try await twoSiteFixture()

diff --git a/specs/bugfixes/membership-removal-never-refreshes-quarantine/report.md b/specs/bugfixes/membership-removal-never-refreshes-quarantine/report.mdnew file mode 100644index 0000000..6195221--- /dev/null+++ b/specs/bugfixes/membership-removal-never-refreshes-quarantine/report.md@@ -0,0 +1,325 @@+# Bugfix Report: removeSiteMembership never refreshes the cached quarantine++**Date:** 2026-09-05+**Status:** Fixed+**Ticket:** T-2291++## Description of the Issue++`LibraryRepository.removeSiteMembership(workID:hostname:)` validates the+prospective graph, gates on `introducedDiagnosis(across:in:)`, saves — and+returns. It never calls `recordPostCommitDiagnosis` /+`publishPostCommitDiagnoses`.++A removal that *repairs* a hostname — dropping the very membership row whose+malformed tuple diagnosed it — therefore leaves the hostname quarantined in the+repository's in-memory map until the next launch. The foreground refresh cannot+recover: `LibraryDiagnostics.union` carries the tuple set forward from the last+full derivation rather than re-deriving it, so a stale entry survives every+refresh.++This is exactly the T-2289 defect (`0c4f0e8`, PR #40) on the one write path+T-2289 missed. T-2289 added `publishPostCommitDiagnoses` and wired it into+`commitWorkURL`, `commitMerge` and `commitResolution`; the sibling+`LibraryRepository+WorkDeletion.swift` has gated *and* published since it was+written.++**Reproduction steps:**+1. Have a two-site Work whose membership on `b.example` carries a malformed+   tuple (for instance an unrecognised `urlIdentityStateRaw`), so `b.example`+   is quarantined at launch.+2. Remove that site from the Work (Work detail -> remove site).+3. The removal succeeds and the offending row is gone, but+   `quarantineReason(hostname: "b.example")` still reports the diagnosis, and+   every path gated on the quarantine stays disabled until the app is+   relaunched.++**Impact:** Medium. No data loss and no incorrect write — the library on disk is+correct. What is wrong is the cached diagnosis: rule application on capture+(`LibraryRepository+ReparseCapture.swift`) stays disabled for the repaired+hostname and Check Library / Settings keep reporting it, for the rest of the app+session. Backup export is *not* affected — it used to be the second consumer of+the latch, but its quarantine gate was retired by Req 3.1 of+`specs/cloudkit-mirroring` (`BackupV9Exporter.swift`, "the quarantine and+unresolved gates are gone"; `LibraryRepository.swift`, the `refreshDiagnostics`+doc), and a quarantined library exports.++## Investigation Summary++- **Symptoms examined:** the quarantine latch surviving a repairing write, and+  the same three commits validating more of the graph than they write to.+- **Code inspected:**+  - `LibraryRepository+Sites.swift` — `removeSiteMembership`+  - `LibraryRepository.swift` — `introducedDiagnosis`,+    `publishPostCommitDiagnoses`, `recordPostCommitDiagnosis`+  - `LibraryRepository+WorkDeletion.swift` — the correct sibling+  - `LibraryRepository+WorkMerge.swift`, `+DuplicateResolution.swift` — the+    three commits still validating the whole graph+  - `LibraryValidator.swift` — `validate(context:)` vs+    `validate(hostnames:context:)`, and `LibraryDiagnostics.quarantineMap()`+- **Hypotheses tested:** whether the foreground refresh could recover the stale+  entry on its own — it cannot; `LibraryDiagnostics.union` documents exactly+  why (the scan produces no `.siteTuple`, so the tuple set has to be carried+  forward, and a stale carried-forward entry is never re-derived).++## Discovered Root Cause++**Defect type:** Missing state publication after a successful commit.++`removeSiteMembership` is a full commit gate — it validates, refuses an+introduced diagnosis, and saves — but it is the only such gate that does not+publish its own validation result. The in-memory quarantine map and the+`diagnostics` tuple set are therefore left holding the *pre-write* answer for a+hostname the write just changed.++**Why it occurred:** the publication step was added to the teaching commits+first and to the deletion commit when that was written; T-2289 (`0c4f0e8`,+PR #40) swept the Work URL, Merge and duplicate-resolution commits into line.+`removeSiteMembership` was not simply overlooked — **T-1956**+(`specs/bugfixes/work-url-and-merge-commits-roll-back-on-any-diagnosis/report.md`,+`7006b91`, PR #43) had already named it in its Follow-up (a), alongside those+same three gates, as never calling `recordPostCommitDiagnosis`. The two+branches were open at once — PR #40 was raised first and merged twenty minutes+after PR #43 — so T-2289's sweep and T-1956's note never met: T-2289 fixed the+three gates that lived in the files it was already editing, and the fourth, in+a different file, stayed a recorded follow-up. This ticket is that follow-up.++**Contributing factors:** three of the four gates in that family still called+the full-library `LibraryValidator.validate(context:)`, which makes the shape of+the "validate, gate, publish" pattern harder to see as one thing across the+files that implement it.++## Resolution for the Issue++**Changes made:**++1. `LibraryRepository+Sites.swift` — `removeSiteMembership` now calls+   `publishPostCommitDiagnoses(across: hostnames, in: diagnoses)` after the+   save, over the same hostname set it validated and gated on (every hostname+   of the Work, not only the one being dropped). A hostname the removal+   repaired is un-quarantined immediately; a diagnosis it left standing is+   re-recorded unchanged, so nothing it did not repair is cleared.++2. `LibraryRepository+WorkMerge.swift`, `LibraryRepository+DuplicateResolution.swift`+   — `commitWorkURL`, `commitMerge` and `commitResolution` swapped+   `LibraryValidator.validate(context:).quarantineMap()` for the hostname-scoped+   `LibraryValidator.validate(hostnames:context:)`, over the hostname set each+   already gates and publishes on (`[hostname]`, `mergeHostnames`, `hostnames`).+   Behaviour is unchanged for the hostnames each gate reads — see the caveat+   below for the two states where the derivations diverge — and the cost is not:+   the whole-library pass replayed every rule against every Entry in the library+   to answer a question about one or two sites, and every diagnosis it derived+   outside that set was discarded unread. What it also did, per **Q110 of+   `specs/duplicate-reconciliation`**, was act as a throw-on-corruption net for+   `commitResolution`: "the validation still *runs* over the whole graph, so a+   resolution that corrupted the store would throw rather than pass". That+   property is deliberately given up here — a gate's refusal is about the graph+   it wrote to, and the whole-graph re-derivation on every resolution is the cost+   Q110 accepted before the scoped overload existed. Q122 of+   `specs/multi-site-works` supersedes Q110 on that point.++3. `LibraryRepository+ComposedTeaching.swift` — the two hand-rolled+   `diagnoses[hostname] != priorDiagnosis` comparisons (`commitComposedTeaching`,+   `commitRecalculation`) now go through `introducedDiagnosis(across:in:)` like+   every other commit gate; the now-unused `priorDiagnosis` locals are gone.+   Behaviourally identical for one hostname — the helper reads the same+   quarantine latch, which nothing touches between the capture point and the+   gate — but it is one rule in one place rather than three spellings of it.++4. `LibraryRepository+WorkDeletion.swift` — corrected a comment citation: the+   introduced-diagnosis rule is Q67 of `specs/multi-site-works` (extending+   Decision 8 of `specs/library-integrity-tolerance`), not Decision 8 of+   `specs/title-teaching-retroactive-parsing`, which is about backup V2 support.++**Approach rationale:** the publication step is what every other commit gate+already does; adding the missing call is the whole fix, and using the existing+`publishPostCommitDiagnoses` helper puts `removeSiteMembership` on the same+four-step shape as `commitWorkURL`, `commitMerge` and `commitResolution` —+validate scoped to the hostnames written, gate through+`introducedDiagnosis(across:in:)`, save, publish through+`publishPostCommitDiagnoses(across:in:)`. Those four are now spelled the same+way; the family as a whole is not, and this change does not make it so:+`commitWorkDeletion` publishes through its own `recordPostCommitDiagnosis` loop,+the two+teaching commits call `recordPostCommitDiagnosis` directly for their one+hostname and still validate the whole library, `commitResolution` alone refetches+Entry and Work after a rollback, and only some gates log.++The validator narrowing is safe because each gate only ever consulted its own+hostnames, and both passes divert the tolerated wrong-host Work URL finding out+of the tuple map. **Two caveats, found in review:** the two derivations do *not*+agree per hostname in every state.++*First,* the whole-graph membership arm records under+`membership.hostname` with no Site check+(`LibraryValidator.swift`, `validate(graph:)`), while the scoped pass `continue`s+at its Site-winner guard before reaching its membership loop — so a malformed+membership row on a hostname that holds **no Site row** is a diagnosis in the+full pass and none in the scoped one. Such a hostname is therefore quarantined at+bootstrap and un-quarantined by any scoped gate that publishes over it, while the+damage stands. This is pre-existing for `commitWorkDeletion`,+`removeSiteMembership` and+`reconcileAfterSync`; this change extends it to `commitWorkURL`, `commitMerge`+and `commitResolution`. The scoped overload's own doc now states it (it+previously claimed the opposite for that arm). Moving the membership arm above+the Site-winner guard would close it and was left out of scope here: it changes+what the scoped pass reports rather than where it is called from.++*Second,* the two passes pick a duplicated-UUID Work group's winner from+different candidate sets: the scoped pass among the rows reachable from the+hostname, the full pass among every row in the library, reading the hostnames off+that winner's memberships. For a group whose `GroupOrdering` winner does not+carry a membership on this hostname — the Req 8.1 lagging-writer state — the+scoped pass reports the Work and the full pass does not. This one runs the other+way: the scoped pass reports *more*, so it can only quarantine where the full+pass would not, never un-quarantine.++**Alternatives considered:**+- Re-derive the quarantine on the next foreground refresh instead of publishing+  at the commit — rejected; `LibraryDiagnostics.union` documents why that cannot+  work (the scan produces no `.siteTuple`, so the tuple set is carried forward,+  never re-derived).+- Leave the three commits on the full validator — rejected; the ticket names it,+  and the narrowing is exactly the change `removeSiteMembership` and+  `commitWorkDeletion` already made.++## Regression Test++**Test file:**+`Packages/AsterismCore/Tests/AsterismCoreTests/MembershipRemovalTests.swift`+**Test names:** `removalThatClearsADiagnosisCommits`,+`removalCommitsUnderAnUnchangedDiagnosis`++Both scenarios were already seeded by this suite for Q67 — a removal that repairs+the removed site's diagnosis, and one that leaves the surviving site's standing —+so the coverage went into them rather than into a second fixture of the same+shape. The first of them carried a comment stating that this path "does not+republish the map after its save", which was the bug recorded as intended+behaviour; it is now the assertion instead.++**What it verifies:**+- `removalThatClearsADiagnosisCommits`: a hostname quarantined by the malformed+  membership tuple on the site being dropped has its quarantine — *and* its entry+  in the carried-forward tuple set — cleared by the removal, and a subsequent+  `refreshDiagnostics()` does not put it back.+- `removalCommitsUnderAnUnchangedDiagnosis`: a diagnosis on the hostname the Work+  stays on is re-recorded unchanged by the publication, so the removal does not+  over-clear.++**Run command:** `make test-core CORE_TEST='MembershipRemovalTests'`++## Affected Files++| File | Change |+|------|--------|+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+Sites.swift` | `removeSiteMembership` publishes its post-commit diagnoses |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift` | `commitWorkURL` and `commitMerge` validate hostname-scoped |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+DuplicateResolution.swift` | `commitResolution` validates hostname-scoped |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+ComposedTeaching.swift` | both teaching gates go through `introducedDiagnosis(across:in:)` |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkDeletion.swift` | comment citation corrected |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryValidator.swift` | scoped overload's doc: which gates use it (and which two do not), and the two arms where it does not match the full pass |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository.swift` | `publishPostCommitDiagnoses` doc: stale caller list dropped; `refreshDiagnostics` doc: the cache-invalidating passes are no longer just two |+| `Packages/AsterismCore/Tests/AsterismCoreTests/MembershipRemovalTests.swift` | regression assertions, replacing the comment that documented the bug |+| `docs/agent-notes/testing.md` | `StoreMetadataTests.storeBornAtV5ReadsAtOrAboveV5` added to the store-digest flake family |++## Verification++**Automated:**+- [x] The regression assertions fail before the fix and pass after it (verified+      by disabling the new `publishPostCommitDiagnoses` call and re-running).+      Only the clearing arm is red without it: the new+      `quarantineReason == nil` / `tupleDiagnoses[...] == nil` assertions in+      `removalThatClearsADiagnosisCommits` are what the fix turns green.+      `removalCommitsUnderAnUnchangedDiagnosis` passes before and after — it+      guards against the publication over-clearing, which is a property of the+      fix rather than a symptom of the bug.+- [x] `make test-core` — clean full runs, exit 0, 2,316 tests (2026-09-05; once+      before the pre-push review and once after its fixes)+- [x] No new compiler warnings (no code-style linter is configured in this repo)++**Known flake seen during verification:** across the branch's two verification+rounds, six further full runs each failed on a *different* single cell of the+store-digest family — `BootstrapActionTests.aFailedOpenChangesNothing` (twice),+`BootstrapClassifierTests`' "classifying the state changed it" cell (twice), and+`StoreMetadataTests.storeBornAtV5ReadsAtOrAboveV5`, which is the same+WAL-checkpoint nondeterminism read from the other end (`.indeterminate` for a+store that was just written at V5). Every one of them passed on an isolated+re-run (`make test-core CORE_TEST=…`, exit 0), and a full run on the unmodified+tree reproduced the family too. This is what+`docs/agent-notes/testing.md` documents as "Known flaky family: the store-digest+comparisons", whose note says one green full run on the branch is what separates+the flake from a real failure — that green run is recorded above.++**Manual verification:**+- Not performed on device. To verify: on a site quarantined by a malformed+  membership row, remove that site from the Work; the site should stop being+  reported as quarantined without relaunching the app.++## Prevention++**Recommendations to avoid similar bugs:**+- Four of the gates — `commitWorkURL`, `commitMerge`, `commitResolution`,+  `removeSiteMembership` — are now spelled identically: validate scoped to the+  hostnames written, gate through `introducedDiagnosis(across:in:)`, save,+  publish through `publishPostCommitDiagnoses(across:in:)`. A new gate that+  omits a step is visibly different from those four rather than merely shorter.+  The rest of the family still varies (see the approach rationale above), so the+  four are a template to copy, not a shape the compiler enforces.+- A commit gate that validates is also a fresher answer than the quarantine+  latch. Any future gate that computes diagnoses must publish them; computing+  them and discarding them is the defect both T-2289 and T-2291 were.++## Deliberately not done++- **Q100 in `specs/multi-site-works/decision_log.md` was left as written.** Its+  wording ("the merge and duplicate-resolution gates change behaviour to match")+  omits `commitWorkURL`, which T-2289 later swept in, and now also the two+  teaching gates. It is a dated historical record of what was decided on+  2026-08-26; amending it to describe work done months later would make the log+  less accurate, not more. **Q122 records the current state of the rule+  instead**, which is the convention the sibling bugfixes follow (Q121 for+  T-2295; Q65, Q66 in `specs/library-integrity-tolerance`).++- **The scoped/full membership-arm divergence was not closed.** Moving the+  membership loop above the Site-winner guard in `validate(hostnames:context:)`+  would make the two derivations agree for a Site-less hostname, but it changes+  what the scoped pass reports rather than where it is called from — a validator+  change, not a call-site one, and out of this ticket's scope. It is documented+  at the overload and in the approach rationale above. The same goes for the+  split-Work-group divergence: it reports more, not less, and closing it means+  changing which rows the scoped pass ranks.++- **Q110 of `specs/duplicate-reconciliation` was left as written.** Like Q100 it+  is a dated record — of 2026-08-02, before the hostname-scoped overload had any+  gate callers — and its throw-on-corruption-anywhere clause was true of the code+  it described. Q122 of `specs/multi-site-works` supersedes it on that point and+  cites it; the approach rationale above says what the branch gives up.++## Follow-ups (out of scope)++Noticed in review, not part of this fix:++- (a) **No test reaches `commitMerge`'s post-write introduced-diagnosis refusal+  or its publication.** `CrossSiteMergeTests` (~`:195-205`) covers the *pre*-write+  Req 4.5 refusal, which is a different branch. The post-write arm is exercised+  only indirectly.+- (b) **No test reaches `commitResolution`'s post-write refusal or its+  publication** either; the `DuplicateResolutionTests` refusal cases are the+  gone/split-reason arms, not the introduced-diagnosis one.+- (c) **Nothing pins the documented scoped/full divergences.**+  `MembershipValidationTests.nilMembershipSiteIsTolerated` covers a well-formed+  row only; neither the Site-less-hostname arm nor the split-Work-group arm has a+  test that would notice if a later change closed or widened it.++## Related++- T-2291 (this fix); T-2289 (`0c4f0e8`, PR #40) — added+  `publishPostCommitDiagnoses` and wired the three sibling gates+- `specs/bugfixes/work-url-and-merge-commits-roll-back-on-any-diagnosis/report.md`+  — T-1956 (`7006b91`, PR #43), which recorded Follow-ups (a), (b), (c) and (d);+  this branch takes up all four+- `specs/multi-site-works/decision_log.md` Q67, Q100, Q122+- `specs/duplicate-reconciliation/decision_log.md` Q110 — superseded by Q122 on+  the whole-graph pass in `commitResolution`+- `specs/cloudkit-mirroring` Req 3.1 — why backup export is no longer a consumer+  of the quarantine latch

diff --git a/specs/multi-site-works/decision_log.md b/specs/multi-site-works/decision_log.mdindex 15159c3..b277041 100644--- a/specs/multi-site-works/decision_log.md+++ b/specs/multi-site-works/decision_log.md@@ -125,6 +125,7 @@ parallel and were numbered from Q90 and Q110 respectively. | Q119 | 2026-08-26 | Req 8.6's "recorded bands" was implemented as the *budgets* in `make test-performance-m4`, and Q89's not-met set is amended to include `diagnosis-refresh-*` | A "band" is the spread of a measurement and a "budget" is what the suite asserts; the requirement's SHALL reads as the first and the work applied the second, which is the checkable one. The three `diagnosis-refresh` arms went 0.300–0.302 s → 0.360–0.367 s against a 250 ms budget that was **already** a known issue before this branch, so the branch deepened a pre-existing breach rather than opening a new one — still inside the 400 ms ceiling, and belonging in Q89's list rather than unmentioned | | Q120 | 2026-08-26 | `MembershipReconciler.reattach` returns the Work ids it could not attach, and `dedupeMemberships` skips their rows — the orphan exclusion is by that set, never by `resolvedWorkID` being nil | The guard was `resolvedWorkID`, which an orphan always has: its `workID` column is how phase 0 finds it (Q37), so the "an orphan is keyless here" exclusion was dead and two orphan rows for one unarrived Work were deduped, deleting the loser against Req 8.3 and Q22. Phase 0 already fetches the orphans by predicate and every Work by id, so it can name the unarrived Works for free — asking `membership.work == nil` per row in phase 2 instead would fault one to-one relationship for every membership in the library, which is the cost Q84 keeps this pass clear of | | Q121 | 2026-08-29 | `.duplicateSiteRows` is diagnosed only while a row other than the winner still owns a title pattern or URL rule (`LibraryDiagnosis.duplicateRowsAwaitReconciliation`, shared by the validator and the tolerance scan); a duplicate whose losers own nothing is a settled state and produces no diagnosis (T-2295) | Decision 6 never deletes a stripped loser, so a diagnosis keyed on row count could never clear: Recent kept its "records could not be resolved" banner and attention edge up for good while Library Check said there was nothing to do — the shape Q57 already retired for duplicate application UUIDs. "No loser owns a rule" is exactly what one completed reconciler pass leaves, and what two untaught twins were from the start. Deleting the converged loser (the ticket's option 2) stays out: the cascade race Decision 6 guards against is real, and the symptom is fully closed without it. See `specs/bugfixes/reconciled-duplicate-banner/report.md` |+| Q122 | 2026-09-05 | A commit gate that validates **publishes what it validated**, over the hostnames it gated on, through `publishPostCommitDiagnoses(across:in:)`; and — with one exception — it validates through the hostname-scoped `LibraryValidator.validate(hostnames:context:)` rather than the whole-library pass (T-2291). The exception is the two teaching commits, `commitComposedTeaching` and `commitRecalculation`, which still call `LibraryValidator.validate(context:)`; they gate and publish over one hostname like the rest | Q100 made every gate refuse only a diagnosis it introduced but said nothing about the step after the save, and `removeSiteMembership` was computing its diagnoses and discarding them: a removal that dropped the very membership row whose malformed tuple diagnosed a site left that site quarantined for the rest of the session, because `LibraryDiagnostics.union` carries the tuple set forward rather than re-deriving it, so no foreground refresh can clear a stale entry. The same omission was T-2289 on three other gates. The scoping is the second half because a gate reads `diagnoses[h]` only for its own hostnames — the whole-library pass replayed every rule against every Entry and threw the rest away. **This supersedes Q110 of `specs/duplicate-reconciliation`** on its last clause: Q110 recorded `commitResolution`'s whole-graph pass as a deliberate throw-on-corruption net ("a resolution that corrupted the store would throw rather than pass"), which the scoping gives up — a gate's refusal is about the graph it wrote to, and Q110 was written before the scoped overload had any gate callers. Caveats, recorded at the overload: the scoped pass skips a Site-less hostname before its membership loop, so a malformed membership row there is a bootstrap diagnosis that a scoped gate clears while the damage stands; and it ranks a split Work group's rows within the hostname, so it reports a group the full pass does not when the group's winner holds no membership here — that one only ever quarantines. See `specs/bugfixes/membership-removal-never-refreshes-quarantine/report.md` |  ## Decision 1: Drop the superseded Work columns in V8, no dual-write window 

diff --git a/docs/agent-notes/testing.md b/docs/agent-notes/testing.mdindex be5cd09..34ca3e5 100644--- a/docs/agent-notes/testing.md+++ b/docs/agent-notes/testing.md@@ -90,6 +90,9 @@ before and after: - `BootstrapClassifierTests`' "classifying the state changed it" cell   (`BootstrapClassifierTests.swift:76`, `store=fullFamily`,   `seededVersion=atOrAboveV5`)+- `StoreMetadataTests.storeBornAtV5ReadsAtOrAboveV5` — the same nondeterminism+  read from the other end: the version reads back `.indeterminate` for a store+  that was just written at V5 (seen during T-2291, 2026-09-05)  **How it presents**: a *different* one of them fails per run, only in a full `make test-core`, and it passes both in isolation (`CORE_TEST=…`) and on a clean