From 8e488b6e4ba29dcaa736d5391669e9a12c7e7395 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:22:20 +0200 Subject: [PATCH] fix(pipeline): a slug used in two places is refused even when one is the wing's own (#208) FindSlug returned the first hit, so a wing's own page of a name masked another page of the same name (the retired dirs are searched before the wings, and wings alphabetically). FindSlugs returns every match and callers refuse any that is not their own. Found by the legacy-move planner's own test. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/pipeline/pipeline.go | 15 +++++-- .../internal/pipeline/wing_layout_test.go | 14 ++++++ ingestion/internal/wiki/findslug_test.go | 30 +++++++------ ingestion/internal/wiki/inventory.go | 45 ++++++++++--------- 4 files changed, 65 insertions(+), 39 deletions(-) diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index cd4ec03..eab1e3e 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -53,11 +53,18 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR var err error if cfg.Wing != "" { // A source slug is a page's graph identity: refuse to take over a slug - // that lives anywhere but this wing's own sources/. - if existing, found, ferr := wiki.FindSlug(brainDir, wiki.Slug(source)); ferr != nil { + // that lives anywhere but this wing's own sources/ — including when + // this wing's own page of that name also exists. + slug := wiki.Slug(source) + own := "wiki/" + cfg.Wing + "/sources/" + slug + ".md" + existing, ferr := wiki.FindSlugs(brainDir, slug) + if ferr != nil { return Result{}, fmt.Errorf("check source slug: %w", ferr) - } else if own := "wiki/" + cfg.Wing + "/sources/" + wiki.Slug(source) + ".md"; found && existing != own { - return Result{}, fmt.Errorf("source slug %q is already used by %s", wiki.Slug(source), existing) + } + for _, p := range existing { + if p != own { + return Result{}, fmt.Errorf("source slug %q is already used by %s", slug, p) + } } inventory, err = wiki.LoadWingInventory(brainDir, cfg.Wing) } else { diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index 5f5b240..d53b514 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -140,3 +140,17 @@ func listAll(t *testing.T, dir string) []string { _ = filepath.WalkDir(dir, func(p string, _ os.DirEntry, _ error) error { out = append(out, p); return nil }) return out } + +// The first match must not hide a second one: this wing's own page of the name +// exists AND a page of the same name lives elsewhere. +func TestRun_ASlugUsedInTwoPlacesIsRefusedEvenWhenTheOwnPageIsOneOfThem(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/web-clip/sources"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/web-clip/sources/talk.md"), []byte("---\ntitle: Talk\n---\nown"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/entities"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/entities/talk.md"), []byte("---\ntitle: Talk\n---\nan entity"), 0o644)) + + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "already used") +} diff --git a/ingestion/internal/wiki/findslug_test.go b/ingestion/internal/wiki/findslug_test.go index bb89795..92202a3 100644 --- a/ingestion/internal/wiki/findslug_test.go +++ b/ingestion/internal/wiki/findslug_test.go @@ -16,37 +16,41 @@ func put(t *testing.T, dir, rel string) { require.NoError(t, os.WriteFile(full, []byte("---\ntitle: x\n---\n"), 0o644)) } -func TestFindSlug(t *testing.T) { +func TestFindSlugs(t *testing.T) { dir := t.TempDir() put(t, dir, "wiki/sources/legacy-src.md") put(t, dir, "wiki/concepts/a-concept.md") put(t, dir, "wiki/entities/an-entity.md") put(t, dir, "wiki/homelab/failures/in-a-hall.md") put(t, dir, "wiki/web-clip/sources/a-clip.md") + // One slug in two places: the first hit must not hide the second. + put(t, dir, "wiki/sources/shared-slug.md") + put(t, dir, "wiki/entities/shared-slug.md") + put(t, dir, "wiki/aaa/facts/shared-slug.md") tests := []struct { slug string - want string + want []string }{ - {"legacy-src", "wiki/sources/legacy-src.md"}, - {"a-concept", "wiki/concepts/a-concept.md"}, - {"an-entity", "wiki/entities/an-entity.md"}, - {"in-a-hall", "wiki/homelab/failures/in-a-hall.md"}, - {"a-clip", "wiki/web-clip/sources/a-clip.md"}, - {"nothing-here", ""}, + {"legacy-src", []string{"wiki/sources/legacy-src.md"}}, + {"a-concept", []string{"wiki/concepts/a-concept.md"}}, + {"an-entity", []string{"wiki/entities/an-entity.md"}}, + {"in-a-hall", []string{"wiki/homelab/failures/in-a-hall.md"}}, + {"a-clip", []string{"wiki/web-clip/sources/a-clip.md"}}, + {"shared-slug", []string{"wiki/aaa/facts/shared-slug.md", "wiki/entities/shared-slug.md", "wiki/sources/shared-slug.md"}}, + {"nothing-here", nil}, } for _, tt := range tests { t.Run(tt.slug, func(t *testing.T) { - got, found, err := FindSlug(dir, tt.slug) + got, err := FindSlugs(dir, tt.slug) require.NoError(t, err) - assert.Equal(t, tt.want != "", found) assert.Equal(t, tt.want, got) }) } } -func TestFindSlug_NoWikiDirIsNotAFinding(t *testing.T) { - _, found, err := FindSlug(t.TempDir(), "x") +func TestFindSlugs_NoWikiDirFindsNothing(t *testing.T) { + got, err := FindSlugs(t.TempDir(), "x") require.NoError(t, err) - assert.False(t, found) + assert.Empty(t, got) } diff --git a/ingestion/internal/wiki/inventory.go b/ingestion/internal/wiki/inventory.go index b87f5f8..44f8be1 100644 --- a/ingestion/internal/wiki/inventory.go +++ b/ingestion/internal/wiki/inventory.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io/fs" + "sort" "strings" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" @@ -133,45 +134,46 @@ func LoadWingInventory(brainDir, wing string) (map[PageType][]Entry, error) { return result, nil } -// FindSlug returns the brain-relative path of an existing page whose basename is -// slug, searching the retired dirs (wiki/{sources,concepts,entities}/) and every -// wing hall (wiki///). The slug is a page's graph identity, so two -// pages must not share one. -func FindSlug(brainDir, slug string) (string, bool, error) { +// FindSlugs returns the brain-relative path of EVERY existing page whose +// basename is slug, searching the retired dirs (wiki/{sources,concepts, +// entities}/) and every wing hall (wiki///), sorted. The slug is a +// page's graph identity, so two pages must not share one; returning all of +// them (not the first) is what lets a caller tell "only my own page" from "my +// own page and somebody else's". +func FindSlugs(brainDir, slug string) ([]string, error) { bfs, err := brainfs.Open(brainDir) if err != nil { - return "", false, fmt.Errorf("open brain: %w", err) + return nil, fmt.Errorf("open brain: %w", err) } defer func() { _ = bfs.Close() }() file := slug + ".md" - has := func(dir string) (bool, error) { + var found []string + check := func(dir string) error { names, err := bfs.ReadDirRegular(dir) if errors.Is(err, fs.ErrNotExist) { - return false, nil + return nil } if err != nil { - return false, err + return err } for _, n := range names { if n == file { - return true, nil + found = append(found, dir+"/"+file) } } - return false, nil + return nil } for _, d := range []string{"wiki/sources", "wiki/concepts", "wiki/entities"} { - if ok, err := has(d); err != nil { - return "", false, err - } else if ok { - return d + "/" + file, true, nil + if err := check(d); err != nil { + return nil, err } } wings, err := bfs.ReadDirDirs("wiki") if errors.Is(err, fs.ErrNotExist) { - return "", false, nil + return found, nil } if err != nil { - return "", false, err + return nil, err } for _, w := range wings { if w == "sources" || w == "concepts" || w == "entities" { @@ -182,12 +184,11 @@ func FindSlug(brainDir, slug string) (string, bool, error) { continue } for _, h := range halls { - if ok, err := has("wiki/" + w + "/" + h); err != nil { - return "", false, err - } else if ok { - return "wiki/" + w + "/" + h + "/" + file, true, nil + if err := check("wiki/" + w + "/" + h); err != nil { + return nil, err } } } - return "", false, nil + sort.Strings(found) + return found, nil }