From 6b2ee6aeeb7234287607e9bead45cb64b463760a Mon Sep 17 00:00:00 2001 From: Aaron Martell Date: Tue, 25 Aug 2026 11:14:14 -0500 Subject: [PATCH 1/4] feat: add gc/restore to deleted book marks. --- cmd/book/gc.go | 51 +++++++++ cmd/book/main.go | 61 +++++++++- cmd/book/mark.go | 75 ++++++++++++- internal/book/types.go | 105 +++++++++++++++-- internal/book/types_test.go | 200 ++++++++++++++++++++++++++------- internal/catalog/index.go | 66 +++++++++++ internal/catalog/index_test.go | 51 +++++++++ internal/model/mark_model.go | 2 +- 8 files changed, 560 insertions(+), 51 deletions(-) create mode 100644 cmd/book/gc.go diff --git a/cmd/book/gc.go b/cmd/book/gc.go new file mode 100644 index 0000000..7f9e0e3 --- /dev/null +++ b/cmd/book/gc.go @@ -0,0 +1,51 @@ +package cmd + +import ( + "fmt" + "time" + + "github.com/polymorcodeus/book/internal/book" + "github.com/polymorcodeus/book/internal/catalog" +) + +// gc purges soft-deleted marks older than retentionDays from the shelf TOML +// files and reconciles the derived index. +func gc(config *book.Config, retentionDays int) error { + if !config.Autoconfirm { + return fmt.Errorf("set --confirm to run gc") + } + if retentionDays < 0 { + return fmt.Errorf("--retention-days must be non-negative") + } + + var shelves book.BookShelves + if err := catalog.LoadShelves(&shelves, config); err != nil { + return err + } + + cutoff := time.Now().UTC().AddDate(0, 0, -retentionDays) + + var purged, changed int + for i := range shelves { + shelf := &shelves[i] + n := shelf.PurgeDeletedMarks(cutoff) + if n == 0 { + continue + } + if err := catalog.UpdateShelfFile(shelf); err != nil { + return err + } + purged += n + changed++ + } + + if changed > 0 { + // Reconcile the derived index so purged marks disappear from search. + if _, err := syncIndex(config); err != nil { + return err + } + } + + fmt.Printf("purged %d mark(s) from %d shelf(s)\n", purged, changed) + return nil +} diff --git a/cmd/book/main.go b/cmd/book/main.go index f3cf0e9..f418ea4 100644 --- a/cmd/book/main.go +++ b/cmd/book/main.go @@ -73,6 +73,10 @@ func Main() { var markTags string var markTitle string var searchTags string + var restoreURL string + var restoreID string + var trash bool + var retentionDays int cmd := &cli.Command{ Name: "book", @@ -390,6 +394,11 @@ func Main() { Usage: "collection selection for mark", Destination: &collection, }, + &cli.BoolFlag{ + Name: "trash", + Usage: "list soft-deleted marks instead of active ones", + Destination: &trash, + }, }, Before: func(ctx context.Context, cmd *cli.Command) (context.Context, error) { if !config.Interactive && format == "" { @@ -398,7 +407,7 @@ func Main() { return ctx, nil }, Action: func(ctx context.Context, cmd *cli.Command) error { - if err := marks(&bookShelves, shelf, collection, format, config); err != nil { + if err := marks(&bookShelves, shelf, collection, format, trash, config); err != nil { return cli.Exit(config.StyledError(err), 1) } return nil @@ -448,6 +457,38 @@ func Main() { return nil }, }, + { + Name: "restore", + Usage: "restore a soft-deleted bookmark", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "id", + Usage: "catalog_id of the trashed mark to restore (preferred)", + Destination: &restoreID, + }, + &cli.StringFlag{ + Name: "shelf", + Usage: "shelf containing the trashed mark", + Destination: &shelf, + }, + &cli.StringFlag{ + Name: "collection", + Usage: "collection containing the trashed mark", + Destination: &collection, + }, + &cli.StringFlag{ + Name: "url", + Usage: "url of the trashed mark to restore", + Destination: &restoreURL, + }, + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + if err := restoreMark(&bookShelves, restoreID, shelf, collection, restoreURL); err != nil { + return cli.Exit(config.StyledError(err), 1) + } + return nil + }, + }, }, }, { @@ -460,6 +501,24 @@ func Main() { return nil }, }, + { + Name: "gc", + Usage: "purge soft-deleted marks older than the retention window", + Flags: []cli.Flag{ + &cli.IntFlag{ + Name: "retention-days", + Value: 30, + Usage: "purge marks soft-deleted more than this many days ago", + Destination: &retentionDays, + }, + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + if err := gc(config, retentionDays); err != nil { + return cli.Exit(config.StyledError(err), 1) + } + return nil + }, + }, { Name: "index", Usage: "manage the derived SQLite search index", diff --git a/cmd/book/mark.go b/cmd/book/mark.go index b0940f6..510b4c7 100644 --- a/cmd/book/mark.go +++ b/cmd/book/mark.go @@ -16,7 +16,29 @@ func mark(bs *book.BookShelves, config *book.Config) error { return runProgram(markRootScreen(bs, &book.Mark{}, "get", config)) } -func marks(bs *book.BookShelves, shelfName string, collectionName string, format string, config *book.Config) error { +func marks(bs *book.BookShelves, shelfName string, collectionName string, format string, trash bool, config *book.Config) error { + // Trash listing reads the derived index and is always non-interactive. + if trash { + if format == "" { + return fmt.Errorf("set --format=[json|toml] to list trashed marks") + } + idx, err := syncIndex(config) + if err != nil { + return err + } + deleted, err := idx.DeletedMarks(shelfName, collectionName) + if err != nil { + return err + } + if format == "toml" { + wrapped := struct { + Marks []catalog.SearchResult `toml:"marks"` + }{deleted} + return book.PrintCatalog(wrapped, format) + } + return book.PrintCatalog(deleted, format) + } + // Non-interactive path: all required flags provided if shelfName != "" && collectionName != "" && !config.Interactive { idx, err := syncIndex(config) @@ -142,6 +164,57 @@ func removeMark(bs *book.BookShelves, config *book.Config) error { return runProgram(markRootScreen(bs, &book.Mark{}, "delete", config)) } +func restoreMark(bs *book.BookShelves, id string, shelfName string, collectionName string, url string) error { + // --id is the preferred path: IDs are globally unique, so no shelf or + // collection scoping is needed. + if id != "" { + target := bs.SoftDeletedByID(id) + if target == nil { + return fmt.Errorf("no trashed mark with id %q", id) + } + return clearSoftDelete(target.Shelf, target.Collection, target) + } + + if shelfName == "" || collectionName == "" || url == "" { + return fmt.Errorf("restore requires --id, or --shelf/--collection/--url") + } + + shelf := bs.Shelf(shelfName) + if book.StructIsEmpty(shelf) { + return fmt.Errorf("shelf %q not found", shelfName) + } + collection := shelf.Collection(collectionName) + if book.StructIsEmpty(collection) { + return fmt.Errorf("collection %q not found in shelf %q", collectionName, shelfName) + } + + var target *book.Mark + for _, m := range collection.Marks { + if m.URL == url && m.IsDeleted() { + target = m + break + } + } + if target == nil { + return fmt.Errorf("no trashed mark with url %q in %q/%q", url, shelfName, collectionName) + } + return clearSoftDelete(shelf, collection, target) +} + +// clearSoftDelete restores a soft-deleted mark in place and persists its shelf. +func clearSoftDelete(shelf *book.Shelf, collection *book.Collection, target *book.Mark) error { + now := book.NowTimestamp() + target.DeletedAt = "" + target.UpdatedAt = now + if collection.UpdatedAt != "" { + collection.UpdatedAt = now + } + if shelf.IsV2() { + shelf.UpdatedAt = now + } + return catalog.UpdateShelfFile(shelf) +} + func markRootScreen(bs *book.BookShelves, mark *book.Mark, action string, config *book.Config) model.RootScreen { if book.StructIsEmpty(mark) { screen := model.GetMarkForm(bs, &book.Mark{}, config, action) diff --git a/internal/book/types.go b/internal/book/types.go index 5178e35..7f7bec6 100644 --- a/internal/book/types.go +++ b/internal/book/types.go @@ -166,12 +166,46 @@ func (bs *BookShelves) LoadParents() { } // VerifyUniqueURL returns an error if the given ID already exists in any mark. +// A collision with a soft-deleted mark points at `book mark restore` rather +// than re-adding the URL. func (bs *BookShelves) VerifyUniqueURL(id string) error { for _, b := range *bs { for _, c := range b.Collections { for _, m := range c.Marks { - if m.ID == id { - return fmt.Errorf("duplicate URL Found!\n\n%s", m.FullDetail()) + if m.ID != id { + continue + } + if m.IsDeleted() { + return fmt.Errorf("URL already trashed!\n\n%s\n\nrestore it with:\nbook mark restore --shelf %s --collection %s --url %s", + m.FullDetail(), m.Shelf.Name, m.Collection.Name, m.URL) + } + return fmt.Errorf("duplicate URL Found!\n\n%s", m.FullDetail()) + } + } + } + return nil +} + +// PurgeDeletedMarks hard-removes soft-deleted marks older than cutoff from every +// shelf, returning the number of marks removed. +func (bs *BookShelves) PurgeDeletedMarks(cutoff time.Time) int { + total := 0 + for i := range *bs { + total += (*bs)[i].PurgeDeletedMarks(cutoff) + } + return total +} + +// SoftDeletedByID returns the soft-deleted mark whose ID matches, or nil. IDs +// are globally unique, so no shelf or collection scoping is needed. The +// returned mark retains its Shelf and Collection back-pointers after +// LoadParents. +func (bs *BookShelves) SoftDeletedByID(id string) *Mark { + for i := range *bs { + for _, c := range (*bs)[i].Collections { + for _, m := range c.Marks { + if m.ID == id && m.IsDeleted() { + return m } } } @@ -222,6 +256,16 @@ func (s *Shelf) AddCollection(c *Collection) { s.Collections[c.Name] = c } +// PurgeDeletedMarks hard-removes soft-deleted marks older than cutoff from every +// collection in the shelf, returning the number of marks removed. +func (s *Shelf) PurgeDeletedMarks(cutoff time.Time) int { + total := 0 + for _, c := range s.Collections { + total += c.PurgeDeletedMarks(cutoff) + } + return total +} + // CollectionsNames returns the names of all collections in the shelf, sorted. func (s *Shelf) CollectionsNames() []string { names := make([]string, 0, len(s.Collections)) @@ -243,29 +287,45 @@ type Collection struct { Marks []*Mark `toml:"marks" json:"marks"` } -// MarksNames returns the names of all marks in the collection. +// MarksNames returns the names of all non-deleted marks in the collection. func (c *Collection) MarksNames() []string { markNames := make([]string, 0, len(c.Marks)) for _, m := range c.Marks { + if m.IsDeleted() { + continue + } markNames = append(markNames, m.Name) } return markNames } -// AllTags returns every tag across all marks in the collection, sorted and deduplicated. +// HasActiveMarks reports whether the collection contains any non-deleted mark. +func (c *Collection) HasActiveMarks() bool { + for _, m := range c.Marks { + if !m.IsDeleted() { + return true + } + } + return false +} + +// AllTags returns every tag across all non-deleted marks in the collection, sorted. func (c *Collection) AllTags() []string { var tags []string for _, m := range c.Marks { + if m.IsDeleted() { + continue + } tags = append(tags, m.Tags...) } slices.Sort(tags) return tags } -// Mark returns a mark by name from the collection. +// Mark returns a non-deleted mark by name from the collection. func (c *Collection) Mark(m string) *Mark { for _, n := range c.Marks { - if n.Name == m { + if n.Name == m && !n.IsDeleted() { return n } } @@ -277,11 +337,33 @@ func (c *Collection) AddMark(m *Mark) { c.Marks = append(c.Marks, m) } -// DeleteMark removes the given mark from the collection. +// DeleteMark soft-deletes the given mark by stamping its DeletedAt field. The +// mark remains in the collection until gc purges it, so accidental removals can +// be restored. func (c *Collection) DeleteMark(m *Mark) { - c.Marks = slices.DeleteFunc(c.Marks, func(d *Mark) bool { - return d == m + if m == nil || m.DeletedAt != "" { + return + } + m.DeletedAt = NowTimestamp() +} + +// PurgeDeletedMarks hard-removes soft-deleted marks whose DeletedAt timestamp is +// strictly before cutoff, returning the number of marks removed. Marks with an +// empty or unparseable DeletedAt are retained. +func (c *Collection) PurgeDeletedMarks(cutoff time.Time) int { + removed := 0 + c.Marks = slices.DeleteFunc(c.Marks, func(m *Mark) bool { + if m == nil || m.DeletedAt == "" { + return false + } + t, err := time.Parse(time.RFC3339, m.DeletedAt) + if err != nil || !t.Before(cutoff) { + return false + } + removed++ + return true }) + return removed } // Mark is a single bookmark with a title, URL, tags, and back-references. @@ -303,6 +385,11 @@ func (m *Mark) Description() string { return fmt.Sprintf("Title: %s\nURL: %s\nTags: %s", m.Name, m.URL, strings.Join(m.Tags, ",")) } +// IsDeleted reports whether the mark has been soft-deleted. +func (m *Mark) IsDeleted() bool { + return m.DeletedAt != "" +} + // FullDetail returns a verbose summary including shelf, collection, title, URL, and tags. func (m *Mark) FullDetail() string { return fmt.Sprintf("Shelf: %s\nCollection: %s\nTitle: %s\nURL: %s\nTags: %s", m.Shelf.Name, m.Collection.Name, m.Name, m.URL, strings.Join(m.Tags, ",")) diff --git a/internal/book/types_test.go b/internal/book/types_test.go index d665482..c7476ef 100644 --- a/internal/book/types_test.go +++ b/internal/book/types_test.go @@ -87,6 +87,7 @@ func TestVerifyUniqueURL(t *testing.T) { Name: "col-1", Marks: []*Mark{ {ID: "abc12345", Name: "first", URL: "https://example.com/first"}, + {ID: "aabbccdd", Name: "trashed", URL: "https://example.com/trashed", DeletedAt: "2026-08-01T00:00:00Z"}, }, }, }, @@ -106,9 +107,10 @@ func TestVerifyUniqueURL(t *testing.T) { bs.LoadParents() tests := []struct { - name string - id string - wantErr bool + name string + id string + wantErr bool + wantRestore bool }{ { name: "unique id passes", @@ -125,6 +127,12 @@ func TestVerifyUniqueURL(t *testing.T) { id: "def67890", wantErr: true, }, + { + name: "trashed collision suggests restore", + id: "aabbccdd", + wantErr: true, + wantRestore: true, + }, } for _, tt := range tests { @@ -136,6 +144,9 @@ func TestVerifyUniqueURL(t *testing.T) { if !tt.wantErr && err != nil { t.Errorf("VerifyUniqueURL(%q) unexpected error: %v", tt.id, err) } + if tt.wantRestore && (err == nil || !strings.Contains(err.Error(), "restore")) { + t.Errorf("VerifyUniqueURL(%q) error = %v, want restore hint", tt.id, err) + } }) } } @@ -176,56 +187,167 @@ func TestAllTags(t *testing.T) { func TestDeleteMark(t *testing.T) { markA := &Mark{Name: "a"} markB := &Mark{Name: "b"} - markC := &Mark{Name: "c"} + + col := &Collection{Marks: []*Mark{markA, markB}} + col.DeleteMark(markB) + + if len(col.Marks) != 2 { + t.Fatalf("len(Marks) = %d, want 2 (soft delete retains the mark)", len(col.Marks)) + } + if markB.DeletedAt == "" { + t.Errorf("DeleteMark did not stamp DeletedAt") + } + if _, err := time.Parse(time.RFC3339, markB.DeletedAt); err != nil { + t.Errorf("DeleteMark DeletedAt = %q, not RFC3339: %v", markB.DeletedAt, err) + } + if markA.DeletedAt != "" { + t.Errorf("DeleteMark stamped the wrong mark") + } + + // Deleting again is a no-op that preserves the original timestamp. + first := markB.DeletedAt + col.DeleteMark(markB) + if markB.DeletedAt != first { + t.Errorf("DeleteMark not idempotent: %q -> %q", first, markB.DeletedAt) + } + + // Deleting a nil mark is a no-op. + col.DeleteMark(nil) + if len(col.Marks) != 2 { + t.Errorf("len(Marks) = %d, want 2 after nil delete", len(col.Marks)) + } +} + +func TestIsDeleted(t *testing.T) { + if (&Mark{}).IsDeleted() { + t.Errorf("empty mark reported deleted") + } + if !(&Mark{DeletedAt: "x"}).IsDeleted() { + t.Errorf("mark with DeletedAt reported not deleted") + } +} + +func TestMarksNamesExcludesDeleted(t *testing.T) { + col := &Collection{Marks: []*Mark{ + {Name: "a"}, + {Name: "b", DeletedAt: "x"}, + {Name: "c"}, + }} + want := []string{"a", "c"} + got := col.MarksNames() + if !slices.Equal(got, want) { + t.Errorf("MarksNames() = %v, want %v", got, want) + } +} + +func TestMarkSkipsDeleted(t *testing.T) { + col := &Collection{Marks: []*Mark{ + {Name: "a", DeletedAt: "x"}, + {Name: "a"}, + }} + got := col.Mark("a") + if got == nil || got.DeletedAt != "" { + t.Errorf("Mark() returned a soft-deleted mark or nil: %+v", got) + } + if col.Mark("missing") != nil { + t.Errorf("Mark() returned a mark for an unknown name") + } +} + +func TestAllTagsExcludesDeleted(t *testing.T) { + col := &Collection{Marks: []*Mark{ + {Tags: []string{"z", "a"}}, + {Tags: []string{"deleted-only"}, DeletedAt: "x"}, + {Tags: []string{"b", "a"}}, + }} + want := []string{"a", "a", "b", "z"} + got := col.AllTags() + if !slices.Equal(got, want) { + t.Errorf("AllTags() = %v, want %v", got, want) + } +} + +func TestPurgeDeletedMarks(t *testing.T) { + cutoff := time.Now().UTC().Add(-30 * 24 * time.Hour) + old := cutoff.Add(-24 * time.Hour).Format(time.RFC3339) + recent := time.Now().UTC().Add(-time.Hour).Format(time.RFC3339) tests := []struct { - name string - start []*Mark - remove *Mark - wantNames []string - wantLength int + name string + col *Collection + want int }{ { - name: "removes by pointer identity", - start: []*Mark{markA, markB, markC}, - remove: markB, - wantNames: []string{"a", "c"}, - wantLength: 2, - }, - { - name: "removing absent mark is no-op", - start: []*Mark{markA, markC}, - remove: markB, - wantNames: []string{"a", "c"}, - wantLength: 2, + name: "purges only old soft-deleted marks", + col: &Collection{Marks: []*Mark{ + {Name: "active"}, + {Name: "old", DeletedAt: old}, + {Name: "recent", DeletedAt: recent}, + {Name: "bad", DeletedAt: "not-a-timestamp"}, + }}, + want: 1, }, { - name: "removes only exact pointer match", - start: []*Mark{markA, {Name: "a"}}, - remove: markA, - wantNames: []string{"a"}, - wantLength: 1, + name: "no soft-deleted marks", + col: &Collection{Marks: []*Mark{ + {Name: "active"}, + }}, + want: 0, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - col := &Collection{Marks: tt.start} - col.DeleteMark(tt.remove) - - if len(col.Marks) != tt.wantLength { - t.Errorf("len(Marks) = %d, want %d", len(col.Marks), tt.wantLength) - } - - gotNames := make([]string, len(col.Marks)) - for i, m := range col.Marks { - gotNames[i] = m.Name - } - if !slices.Equal(gotNames, tt.wantNames) { - t.Errorf("remaining marks = %v, want %v", gotNames, tt.wantNames) + got := tt.col.PurgeDeletedMarks(cutoff) + if got != tt.want { + t.Fatalf("PurgeDeletedMarks() = %d, want %d", got, tt.want) } }) } + + // Verify the first case retains the right marks. + col := &Collection{Marks: []*Mark{ + {Name: "active"}, + {Name: "old", DeletedAt: old}, + {Name: "recent", DeletedAt: recent}, + {Name: "bad", DeletedAt: "not-a-timestamp"}, + }} + col.PurgeDeletedMarks(cutoff) + var remaining []string + for _, m := range col.Marks { + remaining = append(remaining, m.Name) + } + want := []string{"active", "recent", "bad"} + if !slices.Equal(remaining, want) { + t.Errorf("remaining marks = %v, want %v", remaining, want) + } +} + +func TestSoftDeletedByID(t *testing.T) { + bs := BookShelves{ + { + Name: "shelf-a", + Collections: map[string]*Collection{ + "col-1": { + Name: "col-1", + Marks: []*Mark{ + {ID: "abc12345", Name: "active", URL: "https://example.com"}, + {ID: "aabbccdd", Name: "trashed", URL: "https://example.com/trashed", DeletedAt: "2026-08-01T00:00:00Z"}, + }, + }, + }, + }, + } + + if got := bs.SoftDeletedByID("aabbccdd"); got == nil || got.Name != "trashed" { + t.Errorf("SoftDeletedByID(trashed) = %+v, want trashed mark", got) + } + if got := bs.SoftDeletedByID("abc12345"); got != nil { + t.Errorf("SoftDeletedByID(active) = %+v, want nil", got) + } + if got := bs.SoftDeletedByID("missing"); got != nil { + t.Errorf("SoftDeletedByID(missing) = %+v, want nil", got) + } } func TestGenerateID(t *testing.T) { diff --git a/internal/catalog/index.go b/internal/catalog/index.go index 11fdb3e..9ce7ea3 100644 --- a/internal/catalog/index.go +++ b/internal/catalog/index.go @@ -352,6 +352,7 @@ func (ix *Index) Collection(shelfName, collectionName string) (*book.Collection, // SearchResult is a single match from a full-text search over the index. type SearchResult struct { + ID string `json:"catalog_id" toml:"catalog_id"` Shelf string `json:"shelf" toml:"shelf"` Collection string `json:"collection" toml:"collection"` Title string `json:"title" toml:"title"` @@ -441,6 +442,71 @@ func (ix *Index) Search(query, shelfName, collectionName string, tagClauses [][] return nil, err } results = append(results, SearchResult{ + ID: m.id, + Shelf: m.shelf, + Collection: m.collection, + Title: m.title, + URL: m.url, + Tags: mark.Tags, + }) + } + return results, nil +} + +// DeletedMarks returns every soft-deleted mark, optionally filtered by shelf and +// collection name, ordered by shelf, collection, and title. +func (ix *Index) DeletedMarks(shelfName, collectionName string) ([]SearchResult, error) { + sqlQuery := ` + SELECT m.catalog_id, m.title, m.url, c.name, s.name + FROM marks m + JOIN collections c ON c.collection_id = m.collection_id + JOIN shelves s ON s.shelf_id = c.shelf_id + WHERE m.deleted_at != ''` + var args []any + + if shelfName != "" { + sqlQuery += " AND s.name = ?" + args = append(args, shelfName) + } + if collectionName != "" { + sqlQuery += " AND c.name = ?" + args = append(args, collectionName) + } + sqlQuery += " ORDER BY s.name, c.name, m.title" + + rows, err := ix.db.Query(sqlQuery, args...) + if err != nil { + return nil, err + } + + type match struct { + id, title, url, collection, shelf string + } + var matches []match + for rows.Next() { + var m match + if err := rows.Scan(&m.id, &m.title, &m.url, &m.collection, &m.shelf); err != nil { + _ = rows.Close() + return nil, err + } + matches = append(matches, m) + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return nil, err + } + if err := rows.Close(); err != nil { + return nil, err + } + + results := make([]SearchResult, 0, len(matches)) + for _, m := range matches { + mark := &book.Mark{ID: m.id} + if err := ix.loadTags(mark); err != nil { + return nil, err + } + results = append(results, SearchResult{ + ID: m.id, Shelf: m.shelf, Collection: m.collection, Title: m.title, diff --git a/internal/catalog/index_test.go b/internal/catalog/index_test.go index 72b4245..7b19abf 100644 --- a/internal/catalog/index_test.go +++ b/internal/catalog/index_test.go @@ -404,3 +404,54 @@ func TestSearchExcludesSoftDeleted(t *testing.T) { t.Fatalf("Search excluded soft-deleted = %+v, want only the non-deleted mark", results) } } + +func TestDeletedMarks(t *testing.T) { + cfg := testConfig(t) + + v2 := 2 + s := &book.Shelf{ + SchemaVersion: &v2, + ID: book.GenerateShelfID("work"), + Name: "work", + Collections: map[string]*book.Collection{ + "golang": { + ID: book.GenerateCollectionID("work", "golang"), + Name: "golang", + Marks: []*book.Mark{ + {ID: book.GenerateID("https://go.dev"), Name: "The Go Programming Language", URL: "https://go.dev", Tags: []string{"lang"}}, + {ID: book.GenerateID("https://pkg.go.dev"), Name: "Golang patterns", URL: "https://pkg.go.dev", Tags: []string{"docs"}, DeletedAt: book.NowTimestamp()}, + }, + }, + }, + } + writeShelfFile(t, cfg, s) + + ix, err := OpenIndex(cfg) + if err != nil { + t.Fatalf("OpenIndex: %v", err) + } + defer func() { _ = ix.Close() }() + + if _, err := ix.Rebuild(cfg); err != nil { + t.Fatalf("Rebuild: %v", err) + } + + deleted, err := ix.DeletedMarks("", "") + if err != nil { + t.Fatalf("DeletedMarks: %v", err) + } + if len(deleted) != 1 || deleted[0].Title != "Golang patterns" { + t.Fatalf("DeletedMarks = %+v, want only the soft-deleted mark", deleted) + } + if deleted[0].ID != book.GenerateID("https://pkg.go.dev") { + t.Fatalf("DeletedMarks ID = %q, want %q", deleted[0].ID, book.GenerateID("https://pkg.go.dev")) + } + + filtered, err := ix.DeletedMarks("other", "") + if err != nil { + t.Fatalf("DeletedMarks filtered: %v", err) + } + if len(filtered) != 0 { + t.Fatalf("DeletedMarks(other) = %+v, want empty", filtered) + } +} diff --git a/internal/model/mark_model.go b/internal/model/mark_model.go index dc965bf..25fc27b 100644 --- a/internal/model/mark_model.go +++ b/internal/model/mark_model.go @@ -284,7 +284,7 @@ func GetMarkForm(bs *book.BookShelves, mark *book.Mark, config *book.Config, act // Prevent empty collections from being loaded var opts []string for _, col := range bs.Shelf(chosenShelf).Collections { - if (len(col.Marks) > 0 && action != "add") || action == "add" { + if (col.HasActiveMarks() && action != "add") || action == "add" { opts = append(opts, col.Name) } } From a71856e757e4973c39324cb66424a03a59ebacac Mon Sep 17 00:00:00 2001 From: Aaron Martell Date: Tue, 25 Aug 2026 11:48:13 -0500 Subject: [PATCH 2/4] feat: add doctor for mult-machine mook mark reconcilliation --- cmd/book/doctor.go | 168 ++++++++++++++++++++++++++ cmd/book/main.go | 33 +++++- internal/book/doctor.go | 130 ++++++++++++++++++++ internal/book/doctor_test.go | 203 ++++++++++++++++++++++++++++++++ internal/book/types.go | 7 ++ internal/catalog/doctor.go | 44 +++++++ internal/catalog/doctor_test.go | 141 ++++++++++++++++++++++ internal/catalog/index.go | 57 +++++++++ 8 files changed, 782 insertions(+), 1 deletion(-) create mode 100644 cmd/book/doctor.go create mode 100644 internal/book/doctor.go create mode 100644 internal/book/doctor_test.go create mode 100644 internal/catalog/doctor.go create mode 100644 internal/catalog/doctor_test.go diff --git a/cmd/book/doctor.go b/cmd/book/doctor.go new file mode 100644 index 0000000..8e80635 --- /dev/null +++ b/cmd/book/doctor.go @@ -0,0 +1,168 @@ +package cmd + +import ( + "fmt" + + "github.com/polymorcodeus/book/internal/book" + "github.com/polymorcodeus/book/internal/catalog" +) + +// doctor inspects the catalog for post-merge problems: duplicate marks, schema +// drift, index staleness, and stray debris. With fix it auto-resolves merge +// duplicates and rewrites the affected shelf files. +func doctor(config *book.Config, fix bool) error { + var shelves book.BookShelves + if err := catalog.LoadShelves(&shelves, config); err != nil { + return err + } + + var duplicates, conflicts []book.MarkConflict + for _, c := range shelves.DetectDuplicates() { + if c.TrueConflict { + conflicts = append(conflicts, c) + } else { + duplicates = append(duplicates, c) + } + } + + v1Files, err := catalog.V1ShelfFiles(config.ShelfRoot, config.CatalogFormat) + if err != nil { + return err + } + debris, err := catalog.StrayDebris(config.ShelfRoot, config.CatalogFormat) + if err != nil { + return err + } + stale, err := indexStaleFiles(config) + if err != nil { + return err + } + + report := doctorReport{ + Duplicates: duplicates, + Conflicts: conflicts, + V1Files: v1Files, + Debris: debris, + Stale: stale, + } + + if fix { + if !config.Autoconfirm { + return fmt.Errorf("set --confirm to fix merge duplicates") + } + removed, changed := shelves.ResolveDuplicates() + report.Fixed = removed + for _, s := range changed { + if err := catalog.UpdateShelfFile(s); err != nil { + return err + } + report.FixedFiles = append(report.FixedFiles, s.FilePath) + } + // Only reconcile the index when no duplicate IDs remain: the index's + // primary key on catalog_id cannot represent unresolved conflicts. + if len(changed) > 0 && len(shelves.DetectDuplicates()) == 0 { + if _, err := syncIndex(config); err != nil { + return err + } + } + } + + printDoctorReport(report) + return nil +} + +// indexStaleFiles returns shelf paths whose index entries are out of date, or +// nil when the index has not been built yet. +func indexStaleFiles(config *book.Config) ([]string, error) { + exists, err := catalog.VerifyExists(catalog.IndexPath(config)) + if err != nil { + return nil, err + } + if !exists { + return nil, nil + } + + idx, err := catalog.OpenIndex(config) + if err != nil { + return nil, err + } + defer func() { _ = idx.Close() }() + return idx.StaleFiles(config) +} + +type doctorReport struct { + Duplicates []book.MarkConflict + Conflicts []book.MarkConflict + V1Files []string + Debris []string + Stale []string + Fixed int + FixedFiles []string +} + +func printDoctorReport(r doctorReport) { + clean := len(r.Duplicates) == 0 && len(r.Conflicts) == 0 && + len(r.V1Files) == 0 && len(r.Debris) == 0 && len(r.Stale) == 0 && r.Fixed == 0 + + fmt.Println("book doctor") + fmt.Println() + + if clean { + fmt.Println("catalog is clean") + return + } + + if len(r.Duplicates) > 0 { + fmt.Printf("duplicate marks (%d)\n", len(r.Duplicates)) + for _, d := range r.Duplicates { + fmt.Printf(" %s %s (%d copies)\n", d.ID, d.URL, len(d.Marks)) + for _, m := range d.Marks { + fmt.Printf(" - %s / %s\n", m.Shelf.Name, m.Collection.Name) + } + } + if r.Fixed == 0 { + fmt.Println(" run `book doctor --fix` to auto-merge these") + } + fmt.Println() + } + + if len(r.Conflicts) > 0 { + fmt.Printf("conflicting marks (%d, manual resolution needed)\n", len(r.Conflicts)) + for _, c := range r.Conflicts { + fmt.Printf(" %s %s\n", c.ID, c.URL) + for _, m := range c.Marks { + fmt.Printf(" - %s / %s title=%q tags=%v deleted=%t\n", + m.Shelf.Name, m.Collection.Name, m.Name, m.Tags, m.IsDeleted()) + } + } + fmt.Println() + } + + if len(r.V1Files) > 0 { + fmt.Println("v1 schema files (run `book migrate`)") + for _, f := range r.V1Files { + fmt.Printf(" %s\n", f) + } + fmt.Println() + } + + if len(r.Stale) > 0 { + fmt.Println("stale index entries (run `book index sync`)") + for _, f := range r.Stale { + fmt.Printf(" %s\n", f) + } + fmt.Println() + } + + if len(r.Debris) > 0 { + fmt.Println("stray debris") + for _, f := range r.Debris { + fmt.Printf(" %s\n", f) + } + fmt.Println() + } + + if r.Fixed > 0 { + fmt.Printf("fixed: removed %d duplicate mark(s) from %d shelf file(s)\n", r.Fixed, len(r.FixedFiles)) + } +} diff --git a/cmd/book/main.go b/cmd/book/main.go index f418ea4..fad6cf9 100644 --- a/cmd/book/main.go +++ b/cmd/book/main.go @@ -77,6 +77,7 @@ func Main() { var restoreID string var trash bool var retentionDays int + var fix bool cmd := &cli.Command{ Name: "book", @@ -188,8 +189,20 @@ func Main() { // when only a subcommand is given (e.g. "book shelf"), urfave/cli // will auto-render the help text. We skip catalog loading so help // renders quickly without reading the filesystem. + // Load Book Shelves only for the data commands (shelf, collection, + // mark) and only when a subcommand is given. This skips `mark + // search` (which reads the SQLite index) and the catalog admin + // tools (migrate, gc, doctor, index, catalog), which load their + // own data. When only a subcommand is given (e.g. "book shelf"), + // urfave/cli auto-renders help, so we skip loading to keep help + // fast. Note the command name must be checked explicitly: a + // top-level tool's own flags (e.g. "doctor --fix") would otherwise + // leak into Args() and trigger an unwanted load. isSearch := cmd.Args().First() == "mark" && cmd.Args().Get(1) == "search" - if cmd.Args().Len() > 1 && !isSearch { + needsCatalog := cmd.Args().First() == "shelf" || + cmd.Args().First() == "collection" || + cmd.Args().First() == "mark" + if cmd.Args().Len() > 1 && needsCatalog && !isSearch { if err := catalog.LoadCatalog(&bookShelves, config, config.Interactive); err != nil { return ctx, cli.Exit(config.StyledError(err), 1) } @@ -519,6 +532,24 @@ func Main() { return nil }, }, + { + Name: "doctor", + Aliases: []string{"sync"}, + Usage: "detect and fix post-merge catalog problems", + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "fix", + Usage: "auto-merge duplicate marks (requires --confirm)", + Destination: &fix, + }, + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + if err := doctor(config, fix); err != nil { + return cli.Exit(config.StyledError(err), 1) + } + return nil + }, + }, { Name: "index", Usage: "manage the derived SQLite search index", diff --git a/internal/book/doctor.go b/internal/book/doctor.go new file mode 100644 index 0000000..d952710 --- /dev/null +++ b/internal/book/doctor.go @@ -0,0 +1,130 @@ +package book + +import "slices" + +// MarkConflict groups marks that share a catalog_id. When catalog_id is empty +// (an unmigrated v1 mark) the ID is derived from the URL, matching the identity +// `book migrate` and the index assign. +type MarkConflict struct { + ID string // the shared catalog_id (or derived ID) + URL string // the mark URL (identical across the group unless IDs collide) + Marks []*Mark // every mark sharing the ID, in deterministic catalog order + + // TrueConflict reports whether the group holds the same URL with differing + // content (title, tags, or deleted state). A true conflict cannot be + // auto-merged; a false one is a harmless duplicate from a git merge. + TrueConflict bool +} + +// effectiveID returns the mark's catalog_id, deriving it from the URL when the +// field is empty. This keeps v1 (unmigrated) marks comparable to v2 marks. +func effectiveID(m *Mark) string { + if m.ID != "" { + return m.ID + } + return GenerateID(m.URL) +} + +// DetectDuplicates scans every shelf and collection for marks that share a +// catalog_id. Results are returned in stable order (by ID). Collection order is +// resolved by name so the outcome is deterministic across runs despite the +// map-keyed collections. +func (bs *BookShelves) DetectDuplicates() []MarkConflict { + groups := make(map[string]*MarkConflict) + var order []string + + for i := range *bs { + shelf := &(*bs)[i] + for _, name := range shelf.CollectionsNames() { + c := shelf.Collections[name] + for _, m := range c.Marks { + id := effectiveID(m) + g, ok := groups[id] + if !ok { + g = &MarkConflict{ID: id, URL: m.URL} + groups[id] = g + order = append(order, id) + } + g.Marks = append(g.Marks, m) + } + } + } + + slices.Sort(order) + result := make([]MarkConflict, 0, len(order)) + for _, id := range order { + g := groups[id] + if len(g.Marks) < 2 { + continue + } + g.TrueConflict = !marksIdentical(g.Marks) + result = append(result, *g) + } + return result +} + +// ResolveDuplicates removes marks that are exact duplicates (same catalog_id +// and identical content) of an earlier mark, keeping the first occurrence. True +// conflicts are left untouched for manual resolution. It returns the number of +// marks removed and the shelves whose collections were modified. +func (bs *BookShelves) ResolveDuplicates() (removed int, changed []*Shelf) { + changedSet := make(map[*Shelf]struct{}) + for _, conflict := range bs.DetectDuplicates() { + if conflict.TrueConflict { + continue + } + for _, dup := range conflict.Marks[1:] { + if dup.Collection == nil { + continue + } + dup.Collection.RemoveMark(dup) + removed++ + if dup.Shelf != nil { + changedSet[dup.Shelf] = struct{}{} + } + } + } + + changed = make([]*Shelf, 0, len(changedSet)) + for shelf := range changedSet { + changed = append(changed, shelf) + } + // Deterministic ordering for the caller's report. + slices.SortFunc(changed, func(a, b *Shelf) int { + if a.Name < b.Name { + return -1 + } + if a.Name > b.Name { + return 1 + } + return 0 + }) + return removed, changed +} + +// marksIdentical reports whether every mark in a non-empty group has the same +// title, URL, tags, and deleted state. +func marksIdentical(marks []*Mark) bool { + first := marks[0] + for _, m := range marks[1:] { + if m.Name != first.Name || m.URL != first.URL || m.DeletedAt != first.DeletedAt { + return false + } + if !equalTags(m.Tags, first.Tags) { + return false + } + } + return true +} + +// equalTags compares two tag slices as sets, ignoring order. +func equalTags(a, b []string) bool { + if len(a) != len(b) { + return false + } + sa := slices.Clone(a) + sb := slices.Clone(b) + slices.Sort(sa) + slices.Sort(sb) + return slices.Equal(sa, sb) +} diff --git a/internal/book/doctor_test.go b/internal/book/doctor_test.go new file mode 100644 index 0000000..ff2b532 --- /dev/null +++ b/internal/book/doctor_test.go @@ -0,0 +1,203 @@ +package book + +import ( + "testing" +) + +// mark returns a Mark with back-pointers wired to the given shelf and collection. +func testMark(s *Shelf, c *Collection, id, name, url string, tags []string) *Mark { + return &Mark{ + Shelf: s, + Collection: c, + ID: id, + Name: name, + URL: url, + Tags: tags, + } +} + +// fixture builds a BookShelves with a single shelf and collection holding marks. +func fixture(t *testing.T, marks ...*Mark) BookShelves { + t.Helper() + shelf := &Shelf{ + ID: "s1", + Name: "work", + Collections: map[string]*Collection{ + "dev": {ID: "c1", Name: "dev"}, + }, + } + col := shelf.Collections["dev"] + for _, m := range marks { + m.Shelf = shelf + m.Collection = col + col.Marks = append(col.Marks, m) + } + return BookShelves{*shelf} +} + +func TestDetectDuplicates(t *testing.T) { + t.Run("no duplicates", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "b", "two", "https://b", []string{"y"}), + ) + if got := bs.DetectDuplicates(); len(got) != 0 { + t.Fatalf("DetectDuplicates() = %v, want none", got) + } + }) + + t.Run("identical duplicate is not a conflict", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + ) + got := bs.DetectDuplicates() + if len(got) != 1 { + t.Fatalf("got %d conflicts, want 1", len(got)) + } + if got[0].TrueConflict { + t.Error("identical marks flagged as conflict") + } + if len(got[0].Marks) != 2 { + t.Errorf("group has %d marks, want 2", len(got[0].Marks)) + } + }) + + t.Run("differing tags is a conflict", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "a", "one", "https://a", []string{"y"}), + ) + got := bs.DetectDuplicates() + if len(got) != 1 || !got[0].TrueConflict { + t.Fatalf("DetectDuplicates() = %v, want 1 true conflict", got) + } + }) + + t.Run("differing title is a conflict", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "a", "two", "https://a", []string{"x"}), + ) + got := bs.DetectDuplicates() + if len(got) != 1 || !got[0].TrueConflict { + t.Fatalf("DetectDuplicates() = %v, want 1 true conflict", got) + } + }) + + t.Run("differing deleted state is a conflict", func(t *testing.T) { + active := testMark(nil, nil, "a", "one", "https://a", []string{"x"}) + deleted := testMark(nil, nil, "a", "one", "https://a", []string{"x"}) + deleted.DeletedAt = "2026-08-01T00:00:00Z" + bs := fixture(t, active, deleted) + got := bs.DetectDuplicates() + if len(got) != 1 || !got[0].TrueConflict { + t.Fatalf("DetectDuplicates() = %v, want 1 true conflict", got) + } + }) + + t.Run("derives ID from URL for v1 marks", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "", "one", "https://a", nil), + testMark(nil, nil, "", "one", "https://a", nil), + ) + got := bs.DetectDuplicates() + if len(got) != 1 { + t.Fatalf("got %d conflicts, want 1", len(got)) + } + if got[0].ID != GenerateID("https://a") { + t.Errorf("ID = %q, want %q", got[0].ID, GenerateID("https://a")) + } + }) + + t.Run("tags compared as sets", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x", "y"}), + testMark(nil, nil, "a", "one", "https://a", []string{"y", "x"}), + ) + got := bs.DetectDuplicates() + if len(got) != 1 || got[0].TrueConflict { + t.Fatalf("DetectDuplicates() = %v, want 1 non-conflict", got) + } + }) +} + +func TestResolveDuplicates(t *testing.T) { + t.Run("removes identical duplicates and keeps first", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "b", "two", "https://b", []string{"y"}), + ) + removed, changed := bs.ResolveDuplicates() + if removed != 1 { + t.Errorf("removed = %d, want 1", removed) + } + if len(changed) != 1 { + t.Fatalf("changed shelves = %d, want 1", len(changed)) + } + col := bs[0].Collections["dev"] + if len(col.Marks) != 2 { + t.Fatalf("marks after resolve = %d, want 2", len(col.Marks)) + } + if col.Marks[0].ID != "a" || col.Marks[1].ID != "b" { + t.Errorf("marks = %q, %q; want a then b", col.Marks[0].ID, col.Marks[1].ID) + } + }) + + t.Run("leaves true conflicts untouched", func(t *testing.T) { + bs := fixture(t, + testMark(nil, nil, "a", "one", "https://a", []string{"x"}), + testMark(nil, nil, "a", "two", "https://a", []string{"x"}), + ) + removed, changed := bs.ResolveDuplicates() + if removed != 0 || len(changed) != 0 { + t.Fatalf("removed=%d changed=%d, want 0/0", removed, len(changed)) + } + col := bs[0].Collections["dev"] + if len(col.Marks) != 2 { + t.Fatalf("marks after resolve = %d, want 2", len(col.Marks)) + } + }) + + t.Run("duplicate across collections dedupes second occurrence", func(t *testing.T) { + shelf := &Shelf{ + ID: "s1", + Name: "work", + Collections: map[string]*Collection{ + "dev": {ID: "c1", Name: "dev"}, + "docs": {ID: "c2", Name: "docs"}, + }, + } + dev := shelf.Collections["dev"] + docs := shelf.Collections["docs"] + m1 := testMark(shelf, dev, "a", "one", "https://a", []string{"x"}) + m2 := testMark(shelf, docs, "a", "one", "https://a", []string{"x"}) + dev.Marks = []*Mark{m1} + docs.Marks = []*Mark{m2} + bs := BookShelves{*shelf} + + removed, _ := bs.ResolveDuplicates() + if removed != 1 { + t.Fatalf("removed = %d, want 1", removed) + } + if len(docs.Marks) != 0 { + t.Errorf("docs marks = %d, want 0", len(docs.Marks)) + } + if len(dev.Marks) != 1 { + t.Errorf("dev marks = %d, want 1", len(dev.Marks)) + } + }) +} + +func TestEqualTags(t *testing.T) { + if !equalTags([]string{"a", "b"}, []string{"b", "a"}) { + t.Error("equalTags should treat order-insensitive sets as equal") + } + if equalTags([]string{"a"}, []string{"a", "b"}) { + t.Error("equalTags should reject different lengths") + } + if equalTags(nil, []string{"a"}) { + t.Error("equalTags should reject nil vs non-nil") + } +} diff --git a/internal/book/types.go b/internal/book/types.go index 7f7bec6..37e7216 100644 --- a/internal/book/types.go +++ b/internal/book/types.go @@ -347,6 +347,13 @@ func (c *Collection) DeleteMark(m *Mark) { m.DeletedAt = NowTimestamp() } +// RemoveMark hard-removes the given mark from the collection without stamping +// deleted_at. It is used by book doctor to drop merge-duplicated marks and must +// not be used for user-initiated removal (which should soft-delete instead). +func (c *Collection) RemoveMark(m *Mark) { + c.Marks = slices.DeleteFunc(c.Marks, func(x *Mark) bool { return x == m }) +} + // PurgeDeletedMarks hard-removes soft-deleted marks whose DeletedAt timestamp is // strictly before cutoff, returning the number of marks removed. Marks with an // empty or unparseable DeletedAt are retained. diff --git a/internal/catalog/doctor.go b/internal/catalog/doctor.go new file mode 100644 index 0000000..4a8af35 --- /dev/null +++ b/internal/catalog/doctor.go @@ -0,0 +1,44 @@ +package catalog + +import ( + "fmt" + "path/filepath" + + "github.com/BurntSushi/toml" + "github.com/polymorcodeus/book/internal/book" +) + +// V1ShelfFiles returns the shelf TOML files in dir that still use the v1 schema +// (no schema_version key). These should be upgraded with `book migrate`. +func V1ShelfFiles(dir, catalogFormat string) ([]string, error) { + files, err := filepath.Glob(fmt.Sprintf("%s/*.%s", dir, catalogFormat)) + if err != nil { + return nil, err + } + + var v1 []string + for _, file := range files { + var shelf book.Shelf + if _, err := toml.DecodeFile(file, &shelf); err != nil { + return nil, err + } + if !shelf.IsV2() { + v1 = append(v1, file) + } + } + return v1, nil +} + +// StrayDebris returns leftover temp and backup files in dir that a crashed +// write or `book migrate` may have left behind. +func StrayDebris(dir, catalogFormat string) ([]string, error) { + var debris []string + for _, suffix := range []string{"tmp", "bak"} { + matches, err := filepath.Glob(fmt.Sprintf("%s/*.%s.%s", dir, catalogFormat, suffix)) + if err != nil { + return nil, err + } + debris = append(debris, matches...) + } + return debris, nil +} diff --git a/internal/catalog/doctor_test.go b/internal/catalog/doctor_test.go new file mode 100644 index 0000000..3cdcfe0 --- /dev/null +++ b/internal/catalog/doctor_test.go @@ -0,0 +1,141 @@ +package catalog + +import ( + "os" + "path/filepath" + "slices" + "testing" + + "github.com/polymorcodeus/book/internal/book" +) + +const v1Fixture = `shelf_name = "work" + +[Collections] + [Collections.dev] + collection_name = "dev" + + [[Collections.dev.marks]] + title = "one" + url = "https://a" +` + +const v2Fixture = `schema_version = 2 +shelf_id = "12345678" +shelf_name = "work" + +[Collections] + [Collections.dev] + collection_id = "87654321" + collection_name = "dev" +` + +func TestV1ShelfFiles(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "work.toml"), []byte(v1Fixture), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "home.toml"), []byte(v2Fixture), 0644); err != nil { + t.Fatal(err) + } + + got, err := V1ShelfFiles(dir, "toml") + if err != nil { + t.Fatalf("V1ShelfFiles: %v", err) + } + want := []string{filepath.Join(dir, "work.toml")} + if !slices.Equal(got, want) { + t.Errorf("V1ShelfFiles() = %v, want %v", got, want) + } +} + +func TestV1ShelfFilesNone(t *testing.T) { + dir := t.TempDir() + got, err := V1ShelfFiles(dir, "toml") + if err != nil { + t.Fatalf("V1ShelfFiles: %v", err) + } + if len(got) != 0 { + t.Errorf("V1ShelfFiles() = %v, want none", got) + } +} + +func TestStrayDebris(t *testing.T) { + dir := t.TempDir() + for _, name := range []string{"a.toml.tmp", "b.toml.bak", "c.toml"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte("x"), 0644); err != nil { + t.Fatal(err) + } + } + + got, err := StrayDebris(dir, "toml") + if err != nil { + t.Fatalf("StrayDebris: %v", err) + } + want := []string{ + filepath.Join(dir, "a.toml.tmp"), + filepath.Join(dir, "b.toml.bak"), + } + slices.Sort(got) + slices.Sort(want) + if !slices.Equal(got, want) { + t.Errorf("StrayDebris() = %v, want %v", got, want) + } +} + +func TestIndexStaleFiles(t *testing.T) { + cfg := testConfig(t) + ix, err := OpenIndex(cfg) + if err != nil { + t.Fatalf("OpenIndex: %v", err) + } + defer func() { _ = ix.Close() }() + + s := sampleShelf() + writeShelfFile(t, cfg, s) + + // Not yet indexed: the file should be reported stale. + stale, err := ix.StaleFiles(cfg) + if err != nil { + t.Fatalf("StaleFiles before sync: %v", err) + } + if !slices.Equal(stale, []string{s.FilePath}) { + t.Fatalf("StaleFiles before sync = %v, want %v", stale, []string{s.FilePath}) + } + + // After a sync the index is current. + if _, err := ix.Sync(cfg); err != nil { + t.Fatalf("Sync: %v", err) + } + stale, err = ix.StaleFiles(cfg) + if err != nil { + t.Fatalf("StaleFiles after sync: %v", err) + } + if len(stale) != 0 { + t.Fatalf("StaleFiles after sync = %v, want empty", stale) + } + + // Modifying the file marks it stale again. + s.Collections["golang"].Marks = append(s.Collections["golang"].Marks, + &book.Mark{ID: book.GenerateID("https://example.com"), Name: "New", URL: "https://example.com"}) + writeShelfFile(t, cfg, s) + stale, err = ix.StaleFiles(cfg) + if err != nil { + t.Fatalf("StaleFiles after modify: %v", err) + } + if !slices.Equal(stale, []string{s.FilePath}) { + t.Fatalf("StaleFiles after modify = %v, want %v", stale, []string{s.FilePath}) + } + + // Removing the file is also reported as stale. + if err := os.Remove(s.FilePath); err != nil { + t.Fatal(err) + } + stale, err = ix.StaleFiles(cfg) + if err != nil { + t.Fatalf("StaleFiles after remove: %v", err) + } + if !slices.Equal(stale, []string{s.FilePath}) { + t.Fatalf("StaleFiles after remove = %v, want %v", stale, []string{s.FilePath}) + } +} diff --git a/internal/catalog/index.go b/internal/catalog/index.go index 9ce7ea3..b5bad44 100644 --- a/internal/catalog/index.go +++ b/internal/catalog/index.go @@ -10,6 +10,7 @@ import ( "io" "os" "path/filepath" + "sort" "strings" "github.com/BurntSushi/toml" @@ -216,6 +217,62 @@ func (ix *Index) Sync(config *book.Config) (*SyncReport, error) { return report, nil } +// StaleFiles returns the shelf file paths whose index entries are out of date: +// files whose content changed since indexing, files never indexed, and paths +// that were indexed but no longer exist on disk. An empty result means the +// index is current. +func (ix *Index) StaleFiles(config *book.Config) ([]string, error) { + files, err := shelfFilePaths(config) + if err != nil { + return nil, err + } + + tx, err := ix.db.Begin() + if err != nil { + return nil, err + } + defer func() { _ = tx.Rollback() }() + + var stale []string + seen := make(map[string]bool, len(files)) + for _, file := range files { + seen[file] = true + changed, err := ix.fileChanged(tx, file) + if err != nil { + return nil, err + } + if changed { + stale = append(stale, file) + } + } + + // Paths indexed but no longer present on disk. + rows, err := tx.Query(`SELECT path FROM file_meta`) + if err != nil { + return nil, err + } + for rows.Next() { + var path string + if err := rows.Scan(&path); err != nil { + _ = rows.Close() + return nil, err + } + if !seen[path] { + stale = append(stale, path) + } + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return nil, err + } + if err := rows.Close(); err != nil { + return nil, err + } + + sort.Strings(stale) + return stale, nil +} + // UpsertShelf writes a single shelf (and its collections, marks, and tags) into // the index. The caller must have already persisted the shelf to TOML. func (ix *Index) UpsertShelf(s *book.Shelf) error { From 99fa05a3b4a7e4bf6906e89f1b6e66acc6f759ee Mon Sep 17 00:00:00 2001 From: Aaron Martell Date: Tue, 25 Aug 2026 11:56:54 -0500 Subject: [PATCH 3/4] chore: version bump --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 56130fb..79127d8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v1.1.1 +v1.2.0 From 826ecb306ccde9035141a4aba9aa276decd52f9f Mon Sep 17 00:00:00 2001 From: Aaron Martell Date: Tue, 25 Aug 2026 12:01:02 -0500 Subject: [PATCH 4/4] docs: updated README --- README.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0f557da..b3eda4a 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Terminal bookmark manager with hierarchical organization (Shelf → Collection → Mark). Bookmarks are persisted as plain TOML, enabling version control, clean diffs, and [dotfile manager](https://github.com/polymorcodeus/lnk) integration. Supports both interactive TUI and non-interactive CLI modes for scripting. -The roadmap includes search, lazy loading, stable identifiers, and atomic shelf-collection operations. +Bookmarks ship with stable identifiers (`catalog_id`), full-text search (`book mark search`), atomic writes, schema migration (`book migrate`), and soft-delete recovery (`book mark restore`, `book gc`). ## Quick Demo @@ -115,6 +115,8 @@ collection_desc = "language and framework docs" - `catalog_id` is a stable URL hash — duplicates are rejected across the entire catalog. - Collections are keyed by name inside the `[Collections]` table. - Marks are inline arrays-of-tables per collection. +- Optional RFC3339 timestamps (`created_at`, `updated_at`, `deleted_at`) track each entity's lifecycle; `deleted_at` marks a soft-deleted mark. +- `mark remove` soft-deletes by setting `deleted_at`; the mark is hidden from `list`/`get`/`search` until `book gc` purges it or `mark restore` brings it back. - `schema_version` is the on-disk data format version (`2`), independent of the tool's release version (v1.x). `book migrate` upgrades older v1 files in place. ## Commands @@ -130,8 +132,15 @@ collection_desc = "language and framework docs" | `mark add ` | Add a bookmark (optionally non-interactive) | | `mark edit` | Edit an existing bookmark (TUI) | | `mark get` | Browse bookmarks and open one (TUI) | -| `mark list` | List bookmarks in a collection | -| `mark remove` | Remove a bookmark (TUI) | +| `mark list` | List bookmarks in a collection (`--trash` lists soft-deleted) | +| `mark search ` | Full-text search by title, URL, or tags | +| `mark remove` | Soft-delete a bookmark (TUI) | +| `mark restore` | Restore a soft-deleted bookmark (`--id`, or `--shelf`/`--collection`/`--url`) | +| `migrate` | Upgrade v1 shelf files to the v2 schema | +| `gc` | Purge soft-deleted marks past the retention window | +| `index rebuild` | Rebuild the derived search index | +| `index sync` | Reconcile the index with shelf changes | +| `doctor` | Detect post-merge duplicates and conflicts (`--fix` auto-merges; alias `sync`) | | `catalog theme` | Generate `theme.json` with default TUI theme | | `catalog template` | Generate `template.json` with default TUI templates | | `catalog config` | Create the config file if missing |