Pre-push review: T-1956 Work URL commit gate

diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swiftindex 0d0c211..875a5b8 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift@@ -132,10 +132,20 @@ extension LibraryRepository {             // The quarantine key is the membership's hostname, not the Work's             // primary one: the write happened on that site, and that is the site             // whose diagnoses can refuse it (Req 8.5).-            if let reason = diagnoses[hostname] {+            //+            // T-1956: this used to roll back on **any** post-commit diagnosis+            // for the hostname, with no comparison against what it already+            // carried. Decision 8 (`specs/library-integrity-tolerance`) applies+            // here exactly as it does to the teaching commits: refuse only for+            // a diagnosis this write **introduced**, not one the hostname+            // already had — an unrelated pre-existing diagnosis must not roll+            // back a legal Work URL confirmation. Q67 and Q100+            // (`specs/multi-site-works`) moved every other gate onto+            // `introducedDiagnosis` and missed this one.+            if let introduced = self.introducedDiagnosis(across: [hostname], in: diagnoses) {                 context.rollback()                 return .invalidated(-                    reason: "Work URL change produced an invalid library state: \(reason)"+                    reason: "Work URL change produced an invalid library state: \(introduced.diagnosis)"                 )             }             do {

diff --git a/Packages/AsterismCore/Tests/AsterismCoreTests/WorkURLDiagnosisComparisonTests.swift b/Packages/AsterismCore/Tests/AsterismCoreTests/WorkURLDiagnosisComparisonTests.swiftnew file mode 100644index 0000000..a29d1ce--- /dev/null+++ b/Packages/AsterismCore/Tests/AsterismCoreTests/WorkURLDiagnosisComparisonTests.swift@@ -0,0 +1,162 @@+import Foundation+import SwiftData+import Testing++@testable import AsterismCore++/// T-1956: `commitWorkURL` rolled back on **any** post-commit diagnosis for the+/// Work's hostname, with no comparison against what the hostname already+/// carried before the commit. That made confirming a Work URL fail with a+/// generic reason even on a hostname whose pre-existing diagnosis the commit+/// left completely untouched — the same defect Decision 8+/// (`specs/library-integrity-tolerance/decision_log.md`) identified and fixed+/// for `commitComposedTeaching` and `commitRecalculation`.+///+/// Mirrors `ReteachDiagnosisComparisonTests`: the comparison is equality+/// against the prior diagnosis, not "any diagnosis at all", and never against+/// severity — an unchanged diagnosis commits, and only a diagnosis the write+/// itself introduces rolls back.+@Suite("Work URL commit diagnosis comparison", .serialized)+struct WorkURLDiagnosisComparisonTests {+    private let host = "workurl-reteach.example"++    @Test("Work URL confirmation commits when the hostname's diagnosis is unchanged")+    func commitsWhenDiagnosisUnchanged() async throws {+        let fixture = try WorkURLDiagnosisFixture()+        let workID = UUID()+        // An illegal taught tuple — two active title patterns — which a Work+        // URL commit neither causes nor can repair. Before the fix, this alone+        // rolled back every Work URL confirmation on the hostname.+        try fixture.seed { context in+            let site = Site(hostname: self.host)+            site.mode = .taught+            context.insert(site)+            context.insert(try TitlePattern(+                version: 1, isActive: true, createdAt: Date(timeIntervalSince1970: 1),+                definition: .wholeTitle, site: site))+            context.insert(try TitlePattern(+                version: 2, isActive: true, createdAt: Date(timeIntervalSince1970: 2),+                definition: .wholeTitle, site: site))++            let work = Work.create(+                in: context, id: workID, title: "A Work", hostname: self.host,+                timestamp: Date(timeIntervalSince1970: 3))+            work.lastParsedTitle = "A Work"+            work.titleProvenanceRaw = TitleProvenance.parsed.rawValue+        }+        let repository = try fixture.diagnosedRepository()+        let before = try #require(await repository.quarantineReason(hostname: host))++        let contract = try await repository.projectWorkURL(+            workID: workID, hostname: host,+            request: .replaceManual("https://\(host)/confirmed"))+        let outcome = try await repository.commitWorkURL(contract)+        guard case .committed = outcome else {+            Issue.record("expected committed, got \(outcome)"); return+        }++        // The write landed…+        let context = fixture.freshContext()+        let work = try #require(try context.fetch(FetchDescriptor<Work>()).first { $0.id == workID })+        #expect(work.membershipValues.first?.workURLString == "https://\(host)/confirmed")+        // …and the diagnosis it did not touch is still recorded, not cleared by+        // a commit that repaired nothing.+        #expect(await repository.quarantineReason(hostname: host) == before)+    }++    @Test("Work URL confirmation rolls back when it would introduce a new diagnosis")+    func rollsBackNewDiagnosis() async throws {+        let fixture = try WorkURLDiagnosisFixture()+        let workAID = UUID()+        let workBID = UUID()+        try fixture.seed { context in+            let site = Site(hostname: self.host)+            site.mode = .taught+            context.insert(site)+            context.insert(try TitlePattern(+                version: 1, isActive: true, createdAt: Date(timeIntervalSince1970: 1),+                definition: .wholeTitle, site: site))++            let workA = Work.create(+                in: context, id: workAID, title: "Work A", hostname: self.host,+                timestamp: Date(timeIntervalSince1970: 2))+            workA.lastParsedTitle = "Work A"+            workA.titleProvenanceRaw = TitleProvenance.parsed.rawValue++            let workB = Work.create(+                in: context, id: workBID, title: "Work B", hostname: self.host,+                timestamp: Date(timeIntervalSince1970: 3))+            workB.lastParsedTitle = "Work B"+            workB.titleProvenanceRaw = TitleProvenance.parsed.rawValue+        }+        let repository = try fixture.diagnosedRepository()+        #expect(await repository.quarantineReason(hostname: host) == nil)++        let contract = try await repository.projectWorkURL(+            workID: workAID, hostname: host,+            request: .replaceManual("https://\(host)/a"))++        // A concurrent write between project and commit corrupts Work B's own+        // confirmed URL — a field Work A's basis does not observe, so the+        // contract does not go stale (mirrors Q46 of+        // `specs/library-integrity-tolerance` for `commitRecalculation`).+        // This is what "introduced" means: a diagnosis the hostname did not+        // carry before this commit.+        try fixture.seed { context in+            let workB = try context.fetch(FetchDescriptor<Work>()).first { $0.id == workBID }+            workB?.membershipValues.first?.workURLString = "not a url"+        }++        let outcome = try await repository.commitWorkURL(contract)+        guard case .invalidated(let reason) = outcome else {+            Issue.record("expected invalidated, got \(outcome)"); return+        }+        #expect(reason.contains("absolute HTTP"), "reason did not name the diagnosis: \(reason)")++        // Rolled back: Work A never received the confirmed URL.+        let context = fixture.freshContext()+        let workA = try #require(try context.fetch(FetchDescriptor<Work>()).first { $0.id == workAID })+        #expect(workA.membershipValues.first?.workURLString == nil)+    }+}++// MARK: - Fixture++/// A minimal analogue of `ReteachFixture` (`ReteachDiagnosisComparisonTests.swift`)+/// scoped to Work URL commits: raw SwiftData seeding, then a repository whose+/// quarantine map comes from one full validation — the same shape the+/// bootstrap uses.+private struct WorkURLDiagnosisFixture {+    let directory: URL+    let configuration: LibraryConfiguration+    let container: ModelContainer+    private let clock: FixedRepositoryClock++    init() throws {+        directory = FileManager.default.temporaryDirectory+            .appending(path: "AsterismWorkURLDiagnosisTests-\(UUID())", directoryHint: .isDirectory)+        try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)+        configuration = LibraryConfiguration(rootDirectory: directory)+        try FileManager.default.createDirectory(+            at: configuration.storeURL.deletingLastPathComponent(), withIntermediateDirectories: true)+        container = try LibraryRepository.openContainer(at: configuration.storeURL)+        clock = FixedRepositoryClock(Date(timeIntervalSince1970: 1_800_000_000))+    }++    func freshContext() -> ModelContext { ModelContext(container) }++    func seed(_ mutate: (ModelContext) throws -> Void) throws {+        let context = ModelContext(container)+        try mutate(context)+        try context.save()+    }++    /// Mirrors the bootstrap: one validation of the store as it stands feeds+    /// both the diagnoses and the quarantine projection.+    func diagnosedRepository() throws -> LibraryRepository {+        let diagnostics = try LibraryValidator.validate(context: freshContext())+        return LibraryRepository.makeRepository(+            configuration, container, .m4, clock, ModelContextSaveStrategy(),+            quarantined: diagnostics.quarantineMap(), diagnostics: diagnostics)+    }+}

diff --git a/specs/bugfixes/work-url-and-merge-commits-roll-back-on-any-diagnosis/report.md b/specs/bugfixes/work-url-and-merge-commits-roll-back-on-any-diagnosis/report.mdnew file mode 100644index 0000000..2b5b0c7--- /dev/null+++ b/specs/bugfixes/work-url-and-merge-commits-roll-back-on-any-diagnosis/report.md@@ -0,0 +1,169 @@+# Bugfix Report: Work URL confirmation rolls back on any diagnosis, not just an introduced one++**Date:** 2026-08-29+**Status:** Fixed+**Ticket:** T-1956++## Description of the Issue++Confirming a Work URL on a hostname that already carried an unrelated,+pre-existing diagnosis failed with a generic "invalid library state" reason —+even though the confirmation itself was legal and touched nothing the+diagnosis was about.++**Reproduction steps:**+1. Teach a hostname such that it carries a diagnosis unrelated to Work URLs+   (e.g. two active title patterns on one Site).+2. On that same hostname, confirm a legal Work URL for a Work that has nothing+   to do with the diagnosis.+3. The commit rolls back and reports "Work URL change produced an invalid+   library state", quoting the pre-existing diagnosis, even though the commit+   introduced nothing new.++**Impact:** Any hostname already under quarantine for an unrelated reason+became unable to accept Work URL confirmations at all, with no way to tell+from the reason given that the diagnosis predated the write.++## Investigation Summary++- **Symptoms examined:** a Work URL commit refused with a diagnosis that+  named a defect the commit did not cause and could not repair.+- **Code inspected:** `LibraryRepository+WorkMerge.swift` (`commitWorkURL`,+  `commitMerge`), `LibraryRepository.swift` (`introducedDiagnosis(across:in:)`,+  `recordPostCommitDiagnosis`), `LibraryRepository+DuplicateResolution.swift`+  (`commitResolution`), `LibraryRepository+Sites.swift`+  (`removeSiteMembership`), `LibraryRepository+ComposedTeaching.swift`.+- **Hypotheses tested:**+  - Validation itself mis-diagnosing the write — ruled out; the diagnosis was+    correctly attributed to the pre-existing state, just not compared against+    it.+  - `commitMerge` suffering the same defect — ruled out; it was already gated+    on `introducedDiagnosis`.+  - `commitWorkURL` comparing against the prior diagnosis before refusing —+    confirmed absent; it gated on the raw post-commit `diagnoses[hostname]`+    lookup with no prior-state comparison.++## Discovered Root Cause++`commitWorkURL` gated its rollback on `diagnoses[hostname]` directly — any+diagnosis present for the hostname after the write rolled the commit back,+regardless of whether that diagnosis existed before the write. Q100 of+`specs/multi-site-works` converted the other commit gates+(`commitMerge`, `commitResolution`'s resolution/deletion/membership-removal+gates) onto the shared `introducedDiagnosis(across:in:)` helper, which refuses+only a diagnosis the write itself introduced — comparing against what the+hostname already carried. `commitWorkURL` was missed in that pass and kept the+older, over-broad comparison.++**Defect type:** Logic error (missing prior-state comparison; a converted+sibling was left out of the conversion).++**Why it occurred:** Q100 named "the merge and duplicate-resolution gates" as+the ones changing to match Q67's rule, having already covered deletion and+membership-removal in Q67 itself. `commitWorkURL` was not named in either pass+and was overlooked.++## Resolution for the Issue++**Changes made:**+- `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift`+  — `commitWorkURL`'s rollback gate now calls+  `self.introducedDiagnosis(across: [hostname], in: diagnoses)` instead of+  reading `diagnoses[hostname]` directly, matching `commitMerge` and the other+  converted gates. The surrounding comment cites Q100 of+  `specs/multi-site-works` as the pass that converted every other gate and+  missed this one.++**Approach rationale:** a one-line switch to the helper every other commit+gate already uses — no new comparison logic, no behavioural change beyond+what Q67/Q100 already established as correct.++**Alternatives considered:**+- Duplicate the prior/post comparison inline in `commitWorkURL` — rejected;+  the whole point of Q100 was one helper so the rule cannot be half-applied+  again, and inlining here would reintroduce that risk.++## Regression Test++**Test file:** `Packages/AsterismCore/Tests/AsterismCoreTests/WorkURLDiagnosisComparisonTests.swift`+**Test names:** `commitsWhenDiagnosisUnchanged`, `rollsBackNewDiagnosis`++**What it verifies:**+- `commitsWhenDiagnosisUnchanged`: a hostname carrying an unrelated+  pre-existing diagnosis (two active title patterns on one Site) still+  commits a legal Work URL confirmation, and the pre-existing diagnosis is+  still reported afterward — the commit neither caused it nor cleared it.+- `rollsBackNewDiagnosis`: a diagnosis genuinely introduced by the write (a+  concurrent mutation that corrupts another Work's confirmed URL on the same+  hostname between project and commit) still rolls back the commit and+  reports the introduced diagnosis by name.++**Run command:** `make test-core CORE_TEST='WorkURLDiagnosisComparisonTests'`++## Affected Files++| File | Change |+|------|--------|+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+WorkMerge.swift` | `commitWorkURL` gate switched to `introducedDiagnosis(across:in:)`; comment cites Q100 |+| `Packages/AsterismCore/Tests/AsterismCoreTests/WorkURLDiagnosisComparisonTests.swift` | new regression tests |++## Verification++**Automated:**+- [x] Focused regression test passes (`make test-core CORE_TEST='WorkURLDiagnosisComparisonTests'`, 2026-08-29)+- [x] Full test suite passes (`make test-core`, 2026-08-29)+- [x] No compiler warnings in the changed files (no code-style linter is configured in this repo)++**Manual verification:**+- Not performed on device in this session. To verify: confirm a Work URL on a+  hostname already under quarantine for an unrelated reason; the confirmation+  should commit, and the quarantine reason should be unchanged afterward.++## Prevention++**Recommendations to avoid similar bugs:**+- When a decision log entry converts a set of call sites onto a shared+  helper, grep for every other call site sharing the old, unconverted pattern+  before closing the decision out — a Q entry naming specific gates is easy+  to under-scope if a sibling gate uses the same defect in different words.+- The five commit-and-validate gates that write to a hostname's graph+  (`commitWorkURL`, `commitMerge`, `commitResolution`, `removeSiteMembership`,+  `commitWorkDeletion`) are a natural family; a future audit of one is a+  reasonable trigger to re-check all five.++## Follow-ups (out of scope)++These were noticed while investigating this bug but are not part of this fix:++- (a) `commitWorkURL`, `commitMerge`, `commitResolution`, and+  `removeSiteMembership` never call `recordPostCommitDiagnosis` after a+  successful save. A commit that happens to *repair* a diagnosis (clears what+  was previously quarantined) leaves the stale quarantine latched in the+  repository's in-memory map until the next bootstrap — only the teaching+  commits (`LibraryRepository+ComposedTeaching.swift`) and+  `LibraryRepository+WorkDeletion.swift` call it today.+- (b) `LibraryRepository+ComposedTeaching.swift` still spells the+  introduced-diagnosis comparison inline at two sites (around lines 246 and+  366: `if let diagnosis = diagnoses[hostname], diagnosis != priorDiagnosis`)+  rather than calling `introducedDiagnosis(across:in:)`. Functionally+  equivalent for a single hostname, but it is a second place the rule can+  drift from the shared helper.+- (c) `commitWorkURL` validates the whole graph+  (`LibraryValidator.validate(context:)`) where the hostname-scoped overload+  (`LibraryValidator.validate(hostnames:context:)`, used by+  `removeSiteMembership`) would do — a Work URL confirmation only ever+  touches one hostname's membership.+- (d) `WorkDeletion.swift:216-217` cites "Decision 8 of+  `specs/title-teaching-retroactive-parsing`" for the introduced-diagnosis+  rule. That spec's Decision 8 is "Support Current Backup V2 Only" — unrelated+  content. The rule actually originates at Q67 of `specs/multi-site-works`+  (extended to the merge and duplicate-resolution gates at Q100 of the same+  spec's decision log); the citation should point there instead.++## Related++- T-1956 (this fix)+- `specs/multi-site-works/decision_log.md` Q67, Q100+- `specs/bugfixes/title-gate-blocks-identity-attachment/report.md` — another+  bugfix that traced a commit-gate defect to an over-broad condition applied+  where a narrower one belonged