rickub
nandi/gleanpublic⑂ Fork 0
⑂ 7f52a94
Branches
main View all branches → View all tags →
Commits
⬇ Clone ▾
git clone https://git.rickub.com/nandi/glean.git
git clone ssh://git@rickub.com/nandi/glean.git

Host key fingerprint (ed25519): SHA256:iycHnxEyq0Q7uyVpB7JlznP0G7JrTPXLYRcAU5CSLhc — verify it before your first connect.

fix(annotations): prevent duplicate annotations and mirror cleanupUnverified

Julien Robert committed 2026-06-24T14:46:29+02:00 Browse files
7f52a94 parent: d64ded7
modified internal/atproto/sync.go +49 -0
@@ -164,6 +164,7 @@ func (s *Sync) syncAnnotations(ctx context.Context, userDID string) error {
164164
165165 var annotations []*db.Annotation
166166 activeURIs := make(map[string]bool)
167+ seenContent := make(map[string]bool)
167168
168169 for _, r := range annRecs {
169170 var rec AnnotationRecord
@@ -174,6 +175,7 @@ func (s *Sync) syncAnnotations(ctx context.Context, userDID string) error {
174175 continue
175176 }
176177 activeURIs[r.URI] = true
178+ seenContent[annotationContentKey(userDID, rec.ArticleURL, rec.Quote, rec.Note)] = true
177179 t := parseRFC3339(rec.CreatedAt)
178180 a := &db.Annotation{
179181 URI: r.URI,
@@ -201,6 +203,14 @@ func (s *Sync) syncAnnotations(ctx context.Context, userDID string) error {
201203 if articleURL == "" {
202204 continue
203205 }
206+
207+ // A margin note that mirrors an existing glean annotation is skipped so it
208+ // does not show up as a duplicate. Its URI is intentionally not tracked as
209+ // active, so any stale duplicate row gets removed by orphan cleanup.
210+ if seenContent[annotationContentKey(userDID, articleURL, quote, note)] {
211+ continue
212+ }
213+
204214 activeURIs[r.URI] = true
205215
206216 feedURL := ""
@@ -266,6 +276,45 @@ func (s *Sync) syncFollows(ctx context.Context, userDID string) error {
266276 return s.users.SyncFollows(ctx, userDID, activeFollows)
267277 }
268278
279+// annotationContentKey is the identity used to detect that a margin note mirrors
280+// a glean annotation. It mirrors ArticleStore.AnnotationExistsByContent.
281+func annotationContentKey(authorDID, articleURL, quote, note string) string {
282+ return authorDID + "\x1f" + articleURL + "\x1f" + quote + "\x1f" + note
283+}
284+
285+// DeleteMirroredMarginNotes removes the user's at.margin.note records that mirror
286+// the given glean annotation (same article URL, quote and note). Glean mirrors
287+// every annotation it creates, so deleting an annotation must also delete its
288+// mirror or the next sync would resurrect it as a margin-note annotation.
289+func DeleteMirroredMarginNotes(ctx context.Context, client *Client, did, articleURL, quote, note string) error {
290+ records, err := listAllRecords(ctx, client, did, CollectionMarginNote)
291+ if err != nil {
292+ return err
293+ }
294+ target := annotationContentKey(did, articleURL, quote, note)
295+ for _, r := range records {
296+ var rec MarginNoteRecord
297+ if err := json.Unmarshal(r.Value, &rec); err != nil {
298+ continue
299+ }
300+ rArticleURL, rQuote, rNote, _ := rec.ToAnnotation()
301+ if rArticleURL == "" {
302+ continue
303+ }
304+ if annotationContentKey(did, rArticleURL, rQuote, rNote) != target {
305+ continue
306+ }
307+ parsed, ok := ParseRecordURI(r.URI)
308+ if !ok {
309+ continue
310+ }
311+ if err := client.DeleteRecord(ctx, did, CollectionMarginNote, parsed.RKey); err != nil {
312+ return err
313+ }
314+ }
315+ return nil
316+}
317+
269318 func (s *Sync) backfillMissingPDSRecords(ctx context.Context, userDID string) error {
270319 subs, err := s.articles.ListSubscriptionsWithoutURI(ctx, userDID)
271320 if err != nil {
@@ -164,6 +164,7 @@ func (s *Sync) syncAnnotations(ctx context.Context, userDID string) error {
164 164
165 var annotations []*db.Annotation165 var annotations []*db.Annotation
166 activeURIs := make(map[string]bool)166 activeURIs := make(map[string]bool)
167+ seenContent := make(map[string]bool)
167 168
168 for _, r := range annRecs {169 for _, r := range annRecs {
169 var rec AnnotationRecord170 var rec AnnotationRecord
@@ -174,6 +175,7 @@ func (s *Sync) syncAnnotations(ctx context.Context, userDID string) error {
174 continue175 continue
175 }176 }
176 activeURIs[r.URI] = true177 activeURIs[r.URI] = true
178+ seenContent[annotationContentKey(userDID, rec.ArticleURL, rec.Quote, rec.Note)] = true
177 t := parseRFC3339(rec.CreatedAt)179 t := parseRFC3339(rec.CreatedAt)
178 a := &db.Annotation{180 a := &db.Annotation{
179 URI: r.URI,181 URI: r.URI,
@@ -201,6 +203,14 @@ func (s *Sync) syncAnnotations(ctx context.Context, userDID string) error {
201 if articleURL == "" {203 if articleURL == "" {
202 continue204 continue
203 }205 }
206+
207+ // A margin note that mirrors an existing glean annotation is skipped so it
208+ // does not show up as a duplicate. Its URI is intentionally not tracked as
209+ // active, so any stale duplicate row gets removed by orphan cleanup.
210+ if seenContent[annotationContentKey(userDID, articleURL, quote, note)] {
211+ continue
212+ }
213+
204 activeURIs[r.URI] = true214 activeURIs[r.URI] = true
205 215
206 feedURL := ""216 feedURL := ""
@@ -266,6 +276,45 @@ func (s *Sync) syncFollows(ctx context.Context, userDID string) error {
266 return s.users.SyncFollows(ctx, userDID, activeFollows)276 return s.users.SyncFollows(ctx, userDID, activeFollows)
267 }277 }
268 278
279+// annotationContentKey is the identity used to detect that a margin note mirrors
280+// a glean annotation. It mirrors ArticleStore.AnnotationExistsByContent.
281+func annotationContentKey(authorDID, articleURL, quote, note string) string {
282+ return authorDID + "\x1f" + articleURL + "\x1f" + quote + "\x1f" + note
283+}
284+
285+// DeleteMirroredMarginNotes removes the user's at.margin.note records that mirror
286+// the given glean annotation (same article URL, quote and note). Glean mirrors
287+// every annotation it creates, so deleting an annotation must also delete its
288+// mirror or the next sync would resurrect it as a margin-note annotation.
289+func DeleteMirroredMarginNotes(ctx context.Context, client *Client, did, articleURL, quote, note string) error {
290+ records, err := listAllRecords(ctx, client, did, CollectionMarginNote)
291+ if err != nil {
292+ return err
293+ }
294+ target := annotationContentKey(did, articleURL, quote, note)
295+ for _, r := range records {
296+ var rec MarginNoteRecord
297+ if err := json.Unmarshal(r.Value, &rec); err != nil {
298+ continue
299+ }
300+ rArticleURL, rQuote, rNote, _ := rec.ToAnnotation()
301+ if rArticleURL == "" {
302+ continue
303+ }
304+ if annotationContentKey(did, rArticleURL, rQuote, rNote) != target {
305+ continue
306+ }
307+ parsed, ok := ParseRecordURI(r.URI)
308+ if !ok {
309+ continue
310+ }
311+ if err := client.DeleteRecord(ctx, did, CollectionMarginNote, parsed.RKey); err != nil {
312+ return err
313+ }
314+ }
315+ return nil
316+}
317+
269 func (s *Sync) backfillMissingPDSRecords(ctx context.Context, userDID string) error {318 func (s *Sync) backfillMissingPDSRecords(ctx context.Context, userDID string) error {
270 subs, err := s.articles.ListSubscriptionsWithoutURI(ctx, userDID)319 subs, err := s.articles.ListSubscriptionsWithoutURI(ctx, userDID)
271 if err != nil {320 if err != nil {
added internal/atproto/sync_test.go +222 -0
new file mode 100644
@@ -0,0 +1,222 @@
1+package atproto
2+
3+import (
4+ "context"
5+ "encoding/json"
6+ "log/slog"
7+ "net/http"
8+ "net/http/httptest"
9+ "os"
10+ "testing"
11+ "time"
12+
13+ "gotest.tools/v3/assert"
14+
15+ "pkg.rbrt.fr/glean/internal/db"
16+)
17+
18+func setupSyncTestDB(t *testing.T) *db.Store {
19+ t.Helper()
20+ f, err := os.CreateTemp("", "glean-sync-test-*.db")
21+ assert.NilError(t, err)
22+ assert.NilError(t, f.Close())
23+ path := f.Name()
24+ t.Cleanup(func() {
25+ for _, suffix := range []string{"", "_users", "_users-shm", "_users-wal", "_articles", "_articles-shm", "_articles-wal", "_recs", "_recs-shm", "_recs-wal"} {
26+ _ = os.Remove(path + suffix)
27+ }
28+ })
29+ dbs, err := db.Open(path)
30+ assert.NilError(t, err)
31+ t.Cleanup(func() { _ = dbs.Close() })
32+ return dbs
33+}
34+
35+type rawRecord struct {
36+ URI string `json:"uri"`
37+ CID string `json:"cid"`
38+ Value json.RawMessage `json:"value"`
39+}
40+
41+// newSyncTestServer returns an httptest PDS that replies to listRecords for the
42+// given collection -> records map.
43+func newSyncTestServer(t *testing.T, byCollection map[string][]Record) *httptest.Server {
44+ t.Helper()
45+ mux := http.NewServeMux()
46+ mux.HandleFunc("/xrpc/", func(w http.ResponseWriter, r *http.Request) {
47+ w.Header().Set("Content-Type", "application/json")
48+ var raw []rawRecord
49+ for _, rec := range byCollection[r.URL.Query().Get("collection")] {
50+ raw = append(raw, rawRecord{URI: rec.URI, CID: rec.CID, Value: rec.Value})
51+ }
52+ _ = json.NewEncoder(w).Encode(map[string]any{"records": raw})
53+ })
54+ server := httptest.NewServer(mux)
55+ t.Cleanup(server.Close)
56+ return server
57+}
58+
59+func TestSyncAnnotations_DedupsMirroredMarginNote(t *testing.T) {
60+ ctx := context.Background()
61+ dbs := setupSyncTestDB(t)
62+
63+ assert.NilError(t, dbs.Articles.UpsertFeed(ctx, &db.Feed{FeedURL: "https://a.com/feed"}))
64+
65+ now := time.Now().Format(time.RFC3339)
66+ annValue, err := json.Marshal(AnnotationRecord{
67+ CreatedAt: now, FeedURL: "https://a.com/feed", ArticleURL: "https://a.com/1",
68+ Quote: "selected text", Note: "my note",
69+ })
70+ assert.NilError(t, err)
71+
72+ marginRec := NewMarginNoteRecord("https://a.com/1", "selected text", "my note", nil, now)
73+ marginValue, err := json.Marshal(marginRec)
74+ assert.NilError(t, err)
75+
76+ server := newSyncTestServer(t, map[string][]Record{
77+ CollectionAnnotation: {
78+ {URI: "at://did:test:u1/at.glean.annotation/rkey1", CID: "cid-a1", Value: annValue},
79+ },
80+ CollectionMarginNote: {
81+ {URI: "at://did:test:u1/at.margin.note/rkey1", CID: "cid-m1", Value: marginValue},
82+ },
83+ })
84+
85+ sync := NewSync(dbs.Articles, dbs.Users, NewUnauthenticatedClient(server.URL), slog.Default())
86+ assert.NilError(t, sync.syncAnnotations(ctx, "did:test:u1"))
87+
88+ anns, err := dbs.Articles.ListAnnotations(ctx, "", "", "", 100, 0)
89+ assert.NilError(t, err)
90+ assert.Equal(t, len(anns), 1, "mirrored margin note should not produce a duplicate row")
91+ if len(anns) == 1 {
92+ assert.Equal(t, anns[0].URI, "at://did:test:u1/at.glean.annotation/rkey1")
93+ }
94+}
95+
96+func TestSyncAnnotations_KeepsExternalMarginNote(t *testing.T) {
97+ ctx := context.Background()
98+ dbs := setupSyncTestDB(t)
99+
100+ assert.NilError(t, dbs.Articles.UpsertFeed(ctx, &db.Feed{FeedURL: "https://a.com/feed"}))
101+
102+ now := time.Now().Format(time.RFC3339)
103+ marginRec := NewMarginNoteRecord("https://a.com/1", "some quote", "some note", nil, now)
104+ marginValue, err := json.Marshal(marginRec)
105+ assert.NilError(t, err)
106+
107+ server := newSyncTestServer(t, map[string][]Record{
108+ CollectionAnnotation: {}, // no glean annotation
109+ CollectionMarginNote: {
110+ {URI: "at://did:test:u1/at.margin.note/rkey1", CID: "cid-m1", Value: marginValue},
111+ },
112+ })
113+
114+ sync := NewSync(dbs.Articles, dbs.Users, NewUnauthenticatedClient(server.URL), slog.Default())
115+ assert.NilError(t, sync.syncAnnotations(ctx, "did:test:u1"))
116+
117+ anns, err := dbs.Articles.ListAnnotations(ctx, "", "", "", 100, 0)
118+ assert.NilError(t, err)
119+ assert.Equal(t, len(anns), 1, "external margin note should be kept")
120+ assert.Equal(t, anns[0].URI, "at://did:test:u1/at.margin.note/rkey1")
121+}
122+
123+func TestSyncAnnotations_DedupsNoteOnlyMarginNote(t *testing.T) {
124+ ctx := context.Background()
125+ dbs := setupSyncTestDB(t)
126+
127+ assert.NilError(t, dbs.Articles.UpsertFeed(ctx, &db.Feed{FeedURL: "https://a.com/feed"}))
128+
129+ now := time.Now().Format(time.RFC3339)
130+ annValue, err := json.Marshal(AnnotationRecord{
131+ CreatedAt: now, FeedURL: "https://a.com/feed", ArticleURL: "https://a.com/1",
132+ Note: "just a comment",
133+ })
134+ assert.NilError(t, err)
135+
136+ marginRec := NewMarginNoteRecord("https://a.com/1", "", "just a comment", nil, now)
137+ marginValue, err := json.Marshal(marginRec)
138+ assert.NilError(t, err)
139+
140+ server := newSyncTestServer(t, map[string][]Record{
141+ CollectionAnnotation: {
142+ {URI: "at://did:test:u1/at.glean.annotation/rkey1", CID: "cid-a1", Value: annValue},
143+ },
144+ CollectionMarginNote: {
145+ {URI: "at://did:test:u1/at.margin.note/rkey1", CID: "cid-m1", Value: marginValue},
146+ },
147+ })
148+
149+ sync := NewSync(dbs.Articles, dbs.Users, NewUnauthenticatedClient(server.URL), slog.Default())
150+ assert.NilError(t, sync.syncAnnotations(ctx, "did:test:u1"))
151+
152+ anns, err := dbs.Articles.ListAnnotations(ctx, "", "", "", 100, 0)
153+ assert.NilError(t, err)
154+ assert.Equal(t, len(anns), 1, "note-only mirrored margin note should not duplicate")
155+}
156+
157+func TestDeleteMirroredMarginNote_DeletesMatchingContent(t *testing.T) {
158+ ctx := context.Background()
159+ now := time.Now().Format(time.RFC3339)
160+
161+ matching := NewMarginNoteRecord("https://a.com/1", "selected text", "my note", nil, now)
162+ matchingValue, err := json.Marshal(matching)
163+ assert.NilError(t, err)
164+ other := NewMarginNoteRecord("https://a.com/2", "other", "other note", nil, now)
165+ otherValue, err := json.Marshal(other)
166+ assert.NilError(t, err)
167+
168+ var deleted []string
169+ mux := http.NewServeMux()
170+ mux.HandleFunc("/xrpc/com.atproto.repo.listRecords", func(w http.ResponseWriter, r *http.Request) {
171+ w.Header().Set("Content-Type", "application/json")
172+ _ = json.NewEncoder(w).Encode(map[string]any{"records": []rawRecord{
173+ {URI: "at://did:test:u1/at.margin.note/match", CID: "c1", Value: matchingValue},
174+ {URI: "at://did:test:u1/at.margin.note/other", CID: "c2", Value: otherValue},
175+ }})
176+ })
177+ mux.HandleFunc("/xrpc/com.atproto.repo.deleteRecord", func(w http.ResponseWriter, r *http.Request) {
178+ var body map[string]any
179+ _ = json.NewDecoder(r.Body).Decode(&body)
180+ deleted = append(deleted, body["rkey"].(string))
181+ w.WriteHeader(http.StatusOK)
182+ _ = json.NewEncoder(w).Encode(map[string]any{})
183+ })
184+ server := httptest.NewServer(mux)
185+ t.Cleanup(server.Close)
186+
187+ client := NewUnauthenticatedClient(server.URL)
188+ err = DeleteMirroredMarginNotes(ctx, client, "did:test:u1", "https://a.com/1", "selected text", "my note")
189+ assert.NilError(t, err)
190+
191+ assert.Equal(t, len(deleted), 1)
192+ assert.Equal(t, deleted[0], "match")
193+}
194+
195+func TestDeleteMirroredMarginNote_NoMatchIsNoop(t *testing.T) {
196+ ctx := context.Background()
197+ now := time.Now().Format(time.RFC3339)
198+
199+ other := NewMarginNoteRecord("https://a.com/2", "other", "other note", nil, now)
200+ otherValue, err := json.Marshal(other)
201+ assert.NilError(t, err)
202+
203+ deleteCalled := false
204+ mux := http.NewServeMux()
205+ mux.HandleFunc("/xrpc/com.atproto.repo.listRecords", func(w http.ResponseWriter, r *http.Request) {
206+ w.Header().Set("Content-Type", "application/json")
207+ _ = json.NewEncoder(w).Encode(map[string]any{"records": []rawRecord{
208+ {URI: "at://did:test:u1/at.margin.note/other", CID: "c2", Value: otherValue},
209+ }})
210+ })
211+ mux.HandleFunc("/xrpc/com.atproto.repo.deleteRecord", func(w http.ResponseWriter, r *http.Request) {
212+ deleteCalled = true
213+ w.WriteHeader(http.StatusOK)
214+ })
215+ server := httptest.NewServer(mux)
216+ t.Cleanup(server.Close)
217+
218+ client := NewUnauthenticatedClient(server.URL)
219+ err = DeleteMirroredMarginNotes(ctx, client, "did:test:u1", "https://a.com/1", "selected text", "my note")
220+ assert.NilError(t, err)
221+ assert.Equal(t, deleteCalled, false)
222+}
new file mode 100644
@@ -0,0 +1,222 @@
1+package atproto
2+
3+import (
4+ "context"
5+ "encoding/json"
6+ "log/slog"
7+ "net/http"
8+ "net/http/httptest"
9+ "os"
10+ "testing"
11+ "time"
12+
13+ "gotest.tools/v3/assert"
14+
15+ "pkg.rbrt.fr/glean/internal/db"
16+)
17+
18+func setupSyncTestDB(t *testing.T) *db.Store {
19+ t.Helper()
20+ f, err := os.CreateTemp("", "glean-sync-test-*.db")
21+ assert.NilError(t, err)
22+ assert.NilError(t, f.Close())
23+ path := f.Name()
24+ t.Cleanup(func() {
25+ for _, suffix := range []string{"", "_users", "_users-shm", "_users-wal", "_articles", "_articles-shm", "_articles-wal", "_recs", "_recs-shm", "_recs-wal"} {
26+ _ = os.Remove(path + suffix)
27+ }
28+ })
29+ dbs, err := db.Open(path)
30+ assert.NilError(t, err)
31+ t.Cleanup(func() { _ = dbs.Close() })
32+ return dbs
33+}
34+
35+type rawRecord struct {
36+ URI string `json:"uri"`
37+ CID string `json:"cid"`
38+ Value json.RawMessage `json:"value"`
39+}
40+
41+// newSyncTestServer returns an httptest PDS that replies to listRecords for the
42+// given collection -> records map.
43+func newSyncTestServer(t *testing.T, byCollection map[string][]Record) *httptest.Server {
44+ t.Helper()
45+ mux := http.NewServeMux()
46+ mux.HandleFunc("/xrpc/", func(w http.ResponseWriter, r *http.Request) {
47+ w.Header().Set("Content-Type", "application/json")
48+ var raw []rawRecord
49+ for _, rec := range byCollection[r.URL.Query().Get("collection")] {
50+ raw = append(raw, rawRecord{URI: rec.URI, CID: rec.CID, Value: rec.Value})
51+ }
52+ _ = json.NewEncoder(w).Encode(map[string]any{"records": raw})
53+ })
54+ server := httptest.NewServer(mux)
55+ t.Cleanup(server.Close)
56+ return server
57+}
58+
59+func TestSyncAnnotations_DedupsMirroredMarginNote(t *testing.T) {
60+ ctx := context.Background()
61+ dbs := setupSyncTestDB(t)
62+
63+ assert.NilError(t, dbs.Articles.UpsertFeed(ctx, &db.Feed{FeedURL: "https://a.com/feed"}))
64+
65+ now := time.Now().Format(time.RFC3339)
66+ annValue, err := json.Marshal(AnnotationRecord{
67+ CreatedAt: now, FeedURL: "https://a.com/feed", ArticleURL: "https://a.com/1",
68+ Quote: "selected text", Note: "my note",
69+ })
70+ assert.NilError(t, err)
71+
72+ marginRec := NewMarginNoteRecord("https://a.com/1", "selected text", "my note", nil, now)
73+ marginValue, err := json.Marshal(marginRec)
74+ assert.NilError(t, err)
75+
76+ server := newSyncTestServer(t, map[string][]Record{
77+ CollectionAnnotation: {
78+ {URI: "at://did:test:u1/at.glean.annotation/rkey1", CID: "cid-a1", Value: annValue},
79+ },
80+ CollectionMarginNote: {
81+ {URI: "at://did:test:u1/at.margin.note/rkey1", CID: "cid-m1", Value: marginValue},
82+ },
83+ })
84+
85+ sync := NewSync(dbs.Articles, dbs.Users, NewUnauthenticatedClient(server.URL), slog.Default())
86+ assert.NilError(t, sync.syncAnnotations(ctx, "did:test:u1"))
87+
88+ anns, err := dbs.Articles.ListAnnotations(ctx, "", "", "", 100, 0)
89+ assert.NilError(t, err)
90+ assert.Equal(t, len(anns), 1, "mirrored margin note should not produce a duplicate row")
91+ if len(anns) == 1 {
92+ assert.Equal(t, anns[0].URI, "at://did:test:u1/at.glean.annotation/rkey1")
93+ }
94+}
95+
96+func TestSyncAnnotations_KeepsExternalMarginNote(t *testing.T) {
97+ ctx := context.Background()
98+ dbs := setupSyncTestDB(t)
99+
100+ assert.NilError(t, dbs.Articles.UpsertFeed(ctx, &db.Feed{FeedURL: "https://a.com/feed"}))
101+
102+ now := time.Now().Format(time.RFC3339)
103+ marginRec := NewMarginNoteRecord("https://a.com/1", "some quote", "some note", nil, now)
104+ marginValue, err := json.Marshal(marginRec)
105+ assert.NilError(t, err)
106+
107+ server := newSyncTestServer(t, map[string][]Record{
108+ CollectionAnnotation: {}, // no glean annotation
109+ CollectionMarginNote: {
110+ {URI: "at://did:test:u1/at.margin.note/rkey1", CID: "cid-m1", Value: marginValue},
111+ },
112+ })
113+
114+ sync := NewSync(dbs.Articles, dbs.Users, NewUnauthenticatedClient(server.URL), slog.Default())
115+ assert.NilError(t, sync.syncAnnotations(ctx, "did:test:u1"))
116+
117+ anns, err := dbs.Articles.ListAnnotations(ctx, "", "", "", 100, 0)
118+ assert.NilError(t, err)
119+ assert.Equal(t, len(anns), 1, "external margin note should be kept")
120+ assert.Equal(t, anns[0].URI, "at://did:test:u1/at.margin.note/rkey1")
121+}
122+
123+func TestSyncAnnotations_DedupsNoteOnlyMarginNote(t *testing.T) {
124+ ctx := context.Background()
125+ dbs := setupSyncTestDB(t)
126+
127+ assert.NilError(t, dbs.Articles.UpsertFeed(ctx, &db.Feed{FeedURL: "https://a.com/feed"}))
128+
129+ now := time.Now().Format(time.RFC3339)
130+ annValue, err := json.Marshal(AnnotationRecord{
131+ CreatedAt: now, FeedURL: "https://a.com/feed", ArticleURL: "https://a.com/1",
132+ Note: "just a comment",
133+ })
134+ assert.NilError(t, err)
135+
136+ marginRec := NewMarginNoteRecord("https://a.com/1", "", "just a comment", nil, now)
137+ marginValue, err := json.Marshal(marginRec)
138+ assert.NilError(t, err)
139+
140+ server := newSyncTestServer(t, map[string][]Record{
141+ CollectionAnnotation: {
142+ {URI: "at://did:test:u1/at.glean.annotation/rkey1", CID: "cid-a1", Value: annValue},
143+ },
144+ CollectionMarginNote: {
145+ {URI: "at://did:test:u1/at.margin.note/rkey1", CID: "cid-m1", Value: marginValue},
146+ },
147+ })
148+
149+ sync := NewSync(dbs.Articles, dbs.Users, NewUnauthenticatedClient(server.URL), slog.Default())
150+ assert.NilError(t, sync.syncAnnotations(ctx, "did:test:u1"))
151+
152+ anns, err := dbs.Articles.ListAnnotations(ctx, "", "", "", 100, 0)
153+ assert.NilError(t, err)
154+ assert.Equal(t, len(anns), 1, "note-only mirrored margin note should not duplicate")
155+}
156+
157+func TestDeleteMirroredMarginNote_DeletesMatchingContent(t *testing.T) {
158+ ctx := context.Background()
159+ now := time.Now().Format(time.RFC3339)
160+
161+ matching := NewMarginNoteRecord("https://a.com/1", "selected text", "my note", nil, now)
162+ matchingValue, err := json.Marshal(matching)
163+ assert.NilError(t, err)
164+ other := NewMarginNoteRecord("https://a.com/2", "other", "other note", nil, now)
165+ otherValue, err := json.Marshal(other)
166+ assert.NilError(t, err)
167+
168+ var deleted []string
169+ mux := http.NewServeMux()
170+ mux.HandleFunc("/xrpc/com.atproto.repo.listRecords", func(w http.ResponseWriter, r *http.Request) {
171+ w.Header().Set("Content-Type", "application/json")
172+ _ = json.NewEncoder(w).Encode(map[string]any{"records": []rawRecord{
173+ {URI: "at://did:test:u1/at.margin.note/match", CID: "c1", Value: matchingValue},
174+ {URI: "at://did:test:u1/at.margin.note/other", CID: "c2", Value: otherValue},
175+ }})
176+ })
177+ mux.HandleFunc("/xrpc/com.atproto.repo.deleteRecord", func(w http.ResponseWriter, r *http.Request) {
178+ var body map[string]any
179+ _ = json.NewDecoder(r.Body).Decode(&body)
180+ deleted = append(deleted, body["rkey"].(string))
181+ w.WriteHeader(http.StatusOK)
182+ _ = json.NewEncoder(w).Encode(map[string]any{})
183+ })
184+ server := httptest.NewServer(mux)
185+ t.Cleanup(server.Close)
186+
187+ client := NewUnauthenticatedClient(server.URL)
188+ err = DeleteMirroredMarginNotes(ctx, client, "did:test:u1", "https://a.com/1", "selected text", "my note")
189+ assert.NilError(t, err)
190+
191+ assert.Equal(t, len(deleted), 1)
192+ assert.Equal(t, deleted[0], "match")
193+}
194+
195+func TestDeleteMirroredMarginNote_NoMatchIsNoop(t *testing.T) {
196+ ctx := context.Background()
197+ now := time.Now().Format(time.RFC3339)
198+
199+ other := NewMarginNoteRecord("https://a.com/2", "other", "other note", nil, now)
200+ otherValue, err := json.Marshal(other)
201+ assert.NilError(t, err)
202+
203+ deleteCalled := false
204+ mux := http.NewServeMux()
205+ mux.HandleFunc("/xrpc/com.atproto.repo.listRecords", func(w http.ResponseWriter, r *http.Request) {
206+ w.Header().Set("Content-Type", "application/json")
207+ _ = json.NewEncoder(w).Encode(map[string]any{"records": []rawRecord{
208+ {URI: "at://did:test:u1/at.margin.note/other", CID: "c2", Value: otherValue},
209+ }})
210+ })
211+ mux.HandleFunc("/xrpc/com.atproto.repo.deleteRecord", func(w http.ResponseWriter, r *http.Request) {
212+ deleteCalled = true
213+ w.WriteHeader(http.StatusOK)
214+ })
215+ server := httptest.NewServer(mux)
216+ t.Cleanup(server.Close)
217+
218+ client := NewUnauthenticatedClient(server.URL)
219+ err = DeleteMirroredMarginNotes(ctx, client, "did:test:u1", "https://a.com/1", "selected text", "my note")
220+ assert.NilError(t, err)
221+ assert.Equal(t, deleteCalled, false)
222+}
modified internal/db/social.go +14 -0
@@ -84,6 +84,20 @@ func (s *ArticleStore) DeleteAnnotation(ctx context.Context, uri string) error {
8484 return err
8585 }
8686
87+// DeleteAnnotationsByContent removes every annotation a user has for a given
88+// article + quote + note. A glean annotation and its at.margin.note mirror share
89+// this content, so deleting by content clears both rows even though they have
90+// different URIs.
91+func (s *ArticleStore) DeleteAnnotationsByContent(ctx context.Context, authorDID, articleURL, quote, note string) error {
92+ _, err := s.db.ExecContext(ctx, `
93+ DELETE FROM articles.annotations
94+ WHERE author_did = ? AND article_url = ?
95+ AND COALESCE(quote, '') = COALESCE(?, '')
96+ AND COALESCE(note, '') = COALESCE(?, '')
97+ `, authorDID, articleURL, quote, note)
98+ return err
99+}
100+
87101 func (s *ArticleStore) AnnotationExists(ctx context.Context, uri string) (bool, error) {
88102 var exists int
89103 err := s.db.QueryRowContext(ctx, `SELECT 1 FROM articles.annotations WHERE uri = ?`, uri).Scan(&exists)
@@ -84,6 +84,20 @@ func (s *ArticleStore) DeleteAnnotation(ctx context.Context, uri string) error {
84 return err84 return err
85 }85 }
86 86
87+// DeleteAnnotationsByContent removes every annotation a user has for a given
88+// article + quote + note. A glean annotation and its at.margin.note mirror share
89+// this content, so deleting by content clears both rows even though they have
90+// different URIs.
91+func (s *ArticleStore) DeleteAnnotationsByContent(ctx context.Context, authorDID, articleURL, quote, note string) error {
92+ _, err := s.db.ExecContext(ctx, `
93+ DELETE FROM articles.annotations
94+ WHERE author_did = ? AND article_url = ?
95+ AND COALESCE(quote, '') = COALESCE(?, '')
96+ AND COALESCE(note, '') = COALESCE(?, '')
97+ `, authorDID, articleURL, quote, note)
98+ return err
99+}
100+
87 func (s *ArticleStore) AnnotationExists(ctx context.Context, uri string) (bool, error) {101 func (s *ArticleStore) AnnotationExists(ctx context.Context, uri string) (bool, error) {
88 var exists int102 var exists int
89 err := s.db.QueryRowContext(ctx, `SELECT 1 FROM articles.annotations WHERE uri = ?`, uri).Scan(&exists)103 err := s.db.QueryRowContext(ctx, `SELECT 1 FROM articles.annotations WHERE uri = ?`, uri).Scan(&exists)
modified internal/server/annotations_handler.go +9 -1
@@ -196,10 +196,18 @@ func (s *Server) handleDeleteAnnotation(w http.ResponseWriter, r *http.Request)
196196 return
197197 }
198198 }
199+
200+ // The annotation is mirrored to at.margin.note on creation; delete the
201+ // mirror too, otherwise the next sync resurrects it as a margin note.
202+ if mirrorErr := atproto.DeleteMirroredMarginNotes(ctx, client, user.DID, annotation.ArticleURL, annotation.Quote.String, annotation.Note.String); mirrorErr != nil {
203+ s.logger.Error("failed to delete mirrored margin note", "error", mirrorErr)
204+ }
199205 }
200206 }
201207
202- if err := s.dbs.Articles.DeleteAnnotation(ctx, annotation.URI); err != nil {
208+ // Delete by content so any duplicate row created by the mirror (different URI,
209+ // identical content) is removed alongside the canonical annotation.
210+ if err := s.dbs.Articles.DeleteAnnotationsByContent(ctx, user.DID, annotation.ArticleURL, annotation.Quote.String, annotation.Note.String); err != nil {
203211 http.Error(w, err.Error(), http.StatusInternalServerError)
204212 return
205213 }
@@ -196,10 +196,18 @@ func (s *Server) handleDeleteAnnotation(w http.ResponseWriter, r *http.Request)
196 return196 return
197 }197 }
198 }198 }
199+
200+ // The annotation is mirrored to at.margin.note on creation; delete the
201+ // mirror too, otherwise the next sync resurrects it as a margin note.
202+ if mirrorErr := atproto.DeleteMirroredMarginNotes(ctx, client, user.DID, annotation.ArticleURL, annotation.Quote.String, annotation.Note.String); mirrorErr != nil {
203+ s.logger.Error("failed to delete mirrored margin note", "error", mirrorErr)
204+ }
199 }205 }
200 }206 }
201 207
202- if err := s.dbs.Articles.DeleteAnnotation(ctx, annotation.URI); err != nil {208+ // Delete by content so any duplicate row created by the mirror (different URI,
209+ // identical content) is removed alongside the canonical annotation.
210+ if err := s.dbs.Articles.DeleteAnnotationsByContent(ctx, user.DID, annotation.ArticleURL, annotation.Quote.String, annotation.Note.String); err != nil {
203 http.Error(w, err.Error(), http.StatusInternalServerError)211 http.Error(w, err.Error(), http.StatusInternalServerError)
204 return212 return
205 }213 }