From 8fdf7284aa46228e68438e9a9646dd8e15f1a9cb Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 07:17:43 +0200 Subject: [PATCH] fix(linkcheck): skip a linked tree that is not a real directory (#157) Review round 1 on PR #175: - entryIndex returned WalkRegular's walk-root ErrNotRegular as an error, and StripUnresolvedLinks runs before every brain_write: a symlinked, dangling or regular-file knowledge/ (or wiki/) made every write fail and lose the learning. main's filepath.WalkDir skipped such a root. It is now skipped with a warning; other walk errors still abort, as on main. New table test over the three shapes. - The symlinked-hall assertion passed with no index written at all. It now indexes a real hall next to the symlinked one and asserts no error, the index exists and lists the real note; a new test does the same for BuildAllWingIndexes with a symlinked wing next to a real one. - The test comment no longer says this PR fixed the FIFO hang on every brain_write: that path is tunnel.DetectTunnels (slice 5b). Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/brain/index_root_test.go | 65 ++++++++++++++++++--- ingestion/internal/brain/linkcheck.go | 8 +++ 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/ingestion/internal/brain/index_root_test.go b/ingestion/internal/brain/index_root_test.go index 1a794e8..6efb2cc 100644 --- a/ingestion/internal/brain/index_root_test.go +++ b/ingestion/internal/brain/index_root_test.go @@ -21,7 +21,7 @@ func readIndex(t *testing.T, brainDir, wing string) string { // hyperguild#157 slice 5: BuildWingIndex listed wiki/// with // os.ReadDir and opened each note by path, so a symlinked note put the -// target's title into _index.md, and a FIFO hung every brain_write. +// target's title into _index.md, and a FIFO hung the index build. func TestBuildWingIndexRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) { b := t.TempDir() facts := filepath.Join(b, "wiki", "w", "facts") @@ -48,24 +48,44 @@ func TestBuildWingIndexRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) { brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) } -// A wing or hall directory that is a symlink is not listed, and nothing is -// written into its target. -func TestBuildWingIndexDoesNotFollowASymlinkedWingOrHall(t *testing.T) { +// A symlinked hall is skipped while a real hall next to it is indexed; a +// symlinked wing is refused and nothing is written into its target. +func TestBuildWingIndexSkipsASymlinkedHallAndRefusesASymlinkedWing(t *testing.T) { b := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "facts"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(b, "raw", "facts", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644)) - require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "decisions"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "decisions", "d.md"), []byte("---\ntitle: zqrealhall\n---\n"), 0o644)) require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "facts"), filepath.Join(b, "wiki", "w", "facts"))) require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(b, "wiki", "alias"))) - _ = brain.BuildWingIndex(b, "w") - assert.NotContains(t, readIndex(t, b, "w"), "zqrawonly", "symlinked hall") + require.NoError(t, brain.BuildWingIndex(b, "w")) + idx, err := os.ReadFile(filepath.Join(b, "wiki", "w", "_index.md")) + require.NoError(t, err, "the wing index is written") + assert.Contains(t, string(idx), "zqrealhall", "the real hall is indexed") + assert.NotContains(t, string(idx), "zqrawonly", "the symlinked hall is not") assert.Error(t, brain.BuildWingIndex(b, "alias")) - _, err := os.Stat(filepath.Join(b, "raw", "_index.md")) + _, err = os.Stat(filepath.Join(b, "raw", "_index.md")) assert.True(t, os.IsNotExist(err), "an index was written into the symlinked wing's target") } +// BuildAllWingIndexes indexes the real wings and skips a symlinked one. +func TestBuildAllWingIndexesSkipsASymlinkedWing(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "facts", "n.md"), []byte("---\ntitle: zqrealwing\n---\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "facts"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(b, "wiki", "alias"))) + + require.NoError(t, brain.BuildAllWingIndexes(b)) + idx, err := os.ReadFile(filepath.Join(b, "wiki", "w", "_index.md")) + require.NoError(t, err) + assert.Contains(t, string(idx), "zqrealwing") + _, err = os.Stat(filepath.Join(b, "raw", "_index.md")) + assert.True(t, os.IsNotExist(err)) +} + // A link to a symlinked note does not resolve: only regular files count as // entries, so StripUnresolvedLinks unwraps it. func TestStripUnresolvedLinksDoesNotCountASymlinkedNoteAsAnEntry(t *testing.T) { @@ -81,3 +101,32 @@ func TestStripUnresolvedLinksDoesNotCountASymlinkedNoteAsAnEntry(t *testing.T) { assert.NotContains(t, out, "[[ghost]]") assert.Contains(t, unlinked, "ghost") } + +// A linked tree (knowledge/ or wiki/) that is not a real directory is +// skipped, not an error: StripUnresolvedLinks runs before every brain_write, +// and an error there loses the learning. +func TestStripUnresolvedLinksSkipsALinkedTreeThatIsNotARealDirectory(t *testing.T) { + for name, mk := range map[string]func(t *testing.T, b string){ + "symlink to an in-brain dir": func(t *testing.T, b string) { + require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "k"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("raw", "k"), filepath.Join(b, "knowledge"))) + }, + "dangling symlink": func(t *testing.T, b string) { + require.NoError(t, os.Symlink("nowhere", filepath.Join(b, "knowledge"))) + }, + "regular file": func(t *testing.T, b string) { + require.NoError(t, os.WriteFile(filepath.Join(b, "knowledge"), []byte("x"), 0o644)) + }, + } { + t.Run(name, func(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "facts", "real.md"), []byte("x"), 0o644)) + mk(t, b) + out, _, err := brain.StripUnresolvedLinks(b, "learning [[real]] and [[gone]]") + require.NoError(t, err) + assert.Contains(t, out, "[[real]]", "links into wiki/ still resolve") + assert.NotContains(t, out, "[[gone]]") + }) + } +} diff --git a/ingestion/internal/brain/linkcheck.go b/ingestion/internal/brain/linkcheck.go index 7f44817..1553015 100644 --- a/ingestion/internal/brain/linkcheck.go +++ b/ingestion/internal/brain/linkcheck.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io/fs" + "log/slog" "path" "regexp" "sort" @@ -121,6 +122,13 @@ func entryIndex(brainDir string) (map[string]struct{}, error) { index[strings.TrimPrefix(strings.TrimSuffix(rel, ".md"), "wiki/")] = struct{}{} return nil }, func(rel string, err error) { + // A linked tree that is not a real directory (a symlink, a + // file) is skipped, as filepath.WalkDir did: this runs before + // every brain_write, and an error here loses the learning. + if rel == dir && errors.Is(err, brainfs.ErrNotRegular) { + slog.Warn("link check: skipping a linked tree that is not a real directory", "dir", dir) + return + } if walkErr == nil { walkErr = fmt.Errorf("%s: %w", rel, err) }