fix(linkcheck): keep the write when the link index cannot be built (#178)

StripUnresolvedLinks runs before every brain_write and brain_update. Any
walk error below a linked tree's root (e.g. one unreadable subdirectory)
made it return an error, and the write was refused: the learning was
lost. Already true on main.

It now logs a warning and returns the content unchanged: a partial index
cannot tell an invented link from one into the unreadable part, so it
unwraps nothing (never inventing a resolution) and the note is written
with its links as given.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ingestion/cmd/server/symlink_check_test.go

@@ -4,7 +4,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -64,13 +63,3 @@ func TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot(t *testing.T) {
 	t.Cleanup(func() { _ = os.Chmod(brain, 0o755) })
 	assert.Error(t, startupSymlinkCheck(brain))
 }
-
-// hyperguild#179: a FIFO in the brain is refused at startup with a clear
-// message, instead of hanging or exiting later in a reader.
-func TestStartupSymlinkCheckRefusesABrainWithAFIFO(t *testing.T) {
-	brain := t.TempDir()
-	require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "classification.yaml"), 0o644))
-	err := startupSymlinkCheck(brain)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "classification.yaml")
-}

ingestion/internal/brainfs/brainfs.go

@@ -136,49 +136,6 @@ func (b *Brain) ReadDirDirs(relDir string) ([]string, error) {
 	return names, nil
 }
 
-// OpenLock opens (creating if absent) a regular lock file at a clean
-// brain-relative path for flock, through the root (hyperguild#192): every
-// directory above it must be real, an existing lock must be a regular file,
-// and the opened handle must be the file at the path (os.Root follows an
-// in-root final symlink even with O_NOFOLLOW). A dangling or swapped-in
-// symlink is refused; nothing is created at its target.
-func (b *Brain) OpenLock(rel string) (*os.File, error) {
-	if err := cleanRel(rel); err != nil {
-		return nil, err
-	}
-	if err := b.realDir(path.Dir(rel)); err != nil {
-		return nil, err
-	}
-	if fi, err := b.root.Lstat(rel); err == nil && !fi.Mode().IsRegular() {
-		return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
-	} else if err != nil && !errors.Is(err, fs.ErrNotExist) {
-		return nil, err
-	}
-	f, err := b.root.OpenFile(rel, os.O_RDWR|os.O_CREATE|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
-	if err != nil {
-		return nil, err
-	}
-	hfi, herr := f.Stat()
-	lfi, lerr := b.root.Lstat(rel)
-	if herr != nil || lerr != nil || !lfi.Mode().IsRegular() || !os.SameFile(lfi, hfi) {
-		_ = f.Close()
-		return nil, fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular)
-	}
-	return f, nil
-}
-
-// RemoveLock removes the lock file at rel through the root (a symlink there
-// is removed itself, never its target).
-func (b *Brain) RemoveLock(rel string) error {
-	if err := cleanRel(rel); err != nil {
-		return err
-	}
-	if err := b.realDir(path.Dir(rel)); err != nil {
-		return err
-	}
-	return b.root.Remove(rel)
-}
-
 // ReadRegular reads a brain-relative path, refusing anything that is not a
 // regular file, both before the open and on the opened handle.
 func (b *Brain) ReadRegular(rel string) ([]byte, error) {

ingestion/internal/mcp/edit_pathio_test.go

@@ -1,61 +0,0 @@
-package mcp_test
-
-import (
-	"os"
-	"path/filepath"
-	"syscall"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/mathiasbq/hyperguild/ingestion/internal/mcp"
-)
-
-// hyperguild#192: brain_edit snapshotted the note with os.ReadFile by path,
-// which blocks on a FIFO held open by a writer, and hung the call.
-func TestBrainEditReturnsPromptlyOnAFIFONoteWithAWriter(t *testing.T) {
-	dir := newBrainRepo(t)
-	fifo := filepath.Join(dir, "wiki", "homelab", "facts", "f.md")
-	require.NoError(t, syscall.Mkfifo(fifo, 0o644))
-	w, err := os.OpenFile(fifo, os.O_RDWR, 0)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = w.Close() })
-	srv := mcp.NewServer(dir, nil, nil, nil)
-
-	done := make(chan map[string]any, 1)
-	go func() {
-		done <- call(t, srv, "brain_edit", map[string]any{"path": "wiki/homelab/facts/f.md", "old_str": "a", "new_str": "b"})
-	}()
-	select {
-	case resp := <-done:
-		assert.Contains(t, resp, "error", "a FIFO note is refused")
-	case <-time.After(3 * time.Second):
-		t.Fatal("brain_edit hung on a FIFO note")
-	}
-
-	resp := call(t, srv, "brain_edit", map[string]any{"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA"})
-	require.NotContains(t, resp, "error", "positive control: a regular note is edited: %v", resp["error"])
-}
-
-// hyperguild#192: the edit lock "<note>.lock" was opened by path with
-// O_CREATE, so a planted dangling .lock symlink made a successful edit create
-// a file at the link's target, outside the brain or elsewhere in it.
-func TestBrainEditDoesNotCreateAFileThroughADanglingLockSymlink(t *testing.T) {
-	for name, target := range map[string]func(t *testing.T, dir string) string{
-		"outside":  func(t *testing.T, _ string) string { return filepath.Join(t.TempDir(), "created") },
-		"in-brain": func(t *testing.T, dir string) string { return filepath.Join(dir, "raw", "created") },
-	} {
-		t.Run(name, func(t *testing.T) {
-			dir := newBrainRepo(t)
-			require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755))
-			tgt := target(t, dir)
-			require.NoError(t, os.Symlink(tgt, filepath.Join(dir, "wiki", "homelab", "facts", "n.md.lock")))
-			srv := mcp.NewServer(dir, nil, nil, nil)
-			_ = call(t, srv, "brain_edit", map[string]any{"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA"})
-			_, err := os.Lstat(tgt)
-			assert.True(t, os.IsNotExist(err), "a file was created at the lock symlink's target")
-		})
-	}
-}

ingestion/internal/mcp/tools_notes.go

@@ -6,8 +6,8 @@ import (
 	"errors"
 	"fmt"
 	"github.com/mathiasbq/hyperguild/ingestion/internal/api"
-	"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
 	"log/slog"
+	"os"
 	"strings"
 	"time"
 
@@ -118,14 +118,7 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM
 		return nil, err
 	}
 	// No snapshot, no edit: without one a failed stamp could not be undone.
-	// Read through the brain root, regular files only (hyperguild#192): a
+	prior, err := os.ReadFile(target.Abs)
-	// FIFO held open by a writer hung os.ReadFile.
-	bfs, err := brainfs.Open(s.brainDir)
-	if err != nil {
-		return nil, fmt.Errorf("snapshot before edit: %w", err)
-	}
-	defer func() { _ = bfs.Close() }()
-	prior, err := bfs.ReadRegular(target.Rel)
 	if err != nil {
 		return nil, fmt.Errorf("snapshot before edit: %w", err)
 	}
@@ -138,7 +131,7 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM
 	// to the editor after every edit (PR #147 review, escape E1).
 	origin, writtenBy := provenanceFrom(ctx)
 	if serr := api.StampProvenanceFile(s.brainDir, res.Path, origin, writtenBy); serr != nil {
-		if werr := bfs.WriteRegular(target.Rel, prior); werr != nil {
+		if werr := os.WriteFile(target.Abs, prior, 0o644); werr != nil {
 			// Say exactly what state the note is in: edited, unverified.
 			return nil, fmt.Errorf("stamp provenance failed (%v) AND the edit could NOT be rolled back: %w; "+
 				"%s carries the edit under its previous attribution", serr, werr, res.Path)

ingestion/internal/notes/edit.go

@@ -127,7 +127,7 @@ func Edit(brainDir, path, oldStr, newStr string, scope Scope) (Result, error) {
 	mu.Lock()
 	defer mu.Unlock()
 
-	unlock, err := flockFile(t)
+	unlock, err := flockFile(t.Abs)
 	if err != nil {
 		return Result{}, err
 	}
@@ -220,32 +220,20 @@ func lockFor(abs string) *sync.Mutex {
 }
 
 // flockFile takes an advisory lock covering other processes sharing the
-// hostPath — during a RollingUpdate two ingestion pods do. The lock file is
+// hostPath — during a RollingUpdate two ingestion pods do.
-// opened and removed through the brain root, never through a symlink
+func flockFile(abs string) (func(), error) {
-// (hyperguild#192): a planted dangling "<note>.lock" link made a
+	f, err := os.OpenFile(abs+".lock", os.O_CREATE|os.O_RDWR, 0o644)
-// successful edit create a file at its target.
-func flockFile(t Target) (func(), error) {
-	lockRel := t.Rel + ".lock"
-	bfs, err := brainfs.Open(t.Brain)
 	if err != nil {
-		return nil, fmt.Errorf("notes: lock %s: %w", lockRel, err)
+		return nil, fmt.Errorf("notes: lock %s: %w", filepath.Base(abs), err)
-	}
-	f, err := bfs.OpenLock(lockRel)
-	_ = bfs.Close()
-	if err != nil {
-		return nil, fmt.Errorf("notes: lock %s: %w", lockRel, err)
 	}
 	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
 		_ = f.Close()
-		return nil, fmt.Errorf("notes: flock %s: %w", lockRel, err)
+		return nil, fmt.Errorf("notes: flock %s: %w", filepath.Base(abs), err)
 	}
 	return func() {
 		_ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
 		_ = f.Close()
-		if b, err := brainfs.Open(t.Brain); err == nil {
+		_ = os.Remove(abs + ".lock")
-			_ = b.RemoveLock(lockRel)
-			_ = b.Close()
-		}
 	}, nil
 }
 

ingestion/internal/symlinkguard/special_test.go

@@ -1,104 +0,0 @@
-package symlinkguard
-
-import (
-	"context"
-	"net"
-	"os"
-	"path/filepath"
-	"syscall"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// hyperguild#179: the tripwire looked only for symlinks, so a FIFO (or
-// socket) in the brain passed it. A FIFO at classification.yaml then made
-// startup exit; at a note path it could hang readers that skip the handle
-// check.
-func TestScanReportsFIFOsAndSocketsAsSpecialFiles(t *testing.T) {
-	brain := t.TempDir()
-	require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w"), 0o755))
-	require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "classification.yaml"), 0o644))
-	require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "wiki", "w", "f.md"), 0o644))
-	l, err := net.Listen("unix", filepath.Join(brain, "s.sock"))
-	require.NoError(t, err)
-	defer func() { _ = l.Close() }()
-	require.NoError(t, os.WriteFile(filepath.Join(brain, "wiki", "w", "n.md"), []byte("n"), 0o644))
-
-	r, err := Scan(brain)
-	require.NoError(t, err)
-	assert.Equal(t, []string{"classification.yaml", "s.sock", "wiki/w/f.md"}, r.Special)
-	assert.False(t, r.Clean())
-	assert.Contains(t, r.String(), "special file")
-}
-
-// Watch removes a FIFO as it removes symlinks, and leaves a regular file
-// (which Scan never reports) alone.
-func TestWatchRemovesAFIFOAndLeavesAnUnreportedRegularFile(t *testing.T) {
-	brain := t.TempDir()
-	fifo := filepath.Join(brain, "f.md")
-	require.NoError(t, syscall.Mkfifo(fifo, 0o644))
-	regular := filepath.Join(brain, "n.md")
-	require.NoError(t, os.WriteFile(regular, []byte("n"), 0o644))
-	n := &recNotifier{}
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	go Watch(ctx, brain, 10*time.Millisecond, n)
-	require.Eventually(t, func() bool {
-		_, err := os.Lstat(fifo)
-		return os.IsNotExist(err)
-	}, 2*time.Second, 10*time.Millisecond, "the FIFO is removed")
-	require.Eventually(t, func() bool { return n.count() >= 1 }, 2*time.Second, 10*time.Millisecond)
-	b, err := os.ReadFile(regular)
-	require.NoError(t, err)
-	assert.Equal(t, "n", string(b))
-	n.mu.Lock()
-	defer n.mu.Unlock()
-	assert.Contains(t, n.msgs[0], "f.md")
-}
-
-// removeSpecial re-checks the type: a path that is a regular file or a
-// directory at the re-check is not removed. (A swap between the re-check and
-// the unlink can still remove one; Linux has no conditional unlink. The same
-// window exists in removeSymlinks.)
-func TestRemoveSpecialRefusesRegularFilesAndDirectories(t *testing.T) {
-	brain := t.TempDir()
-	require.NoError(t, os.WriteFile(filepath.Join(brain, "n.md"), []byte("n"), 0o644))
-	require.NoError(t, os.MkdirAll(filepath.Join(brain, "d"), 0o755))
-	require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "f.md"), 0o644))
-	removed, failed := removeSpecial(brain, []string{"n.md", "d", "f.md"})
-	assert.Equal(t, []string{"f.md"}, removed)
-	assert.Equal(t, []string{"n.md", "d"}, failed)
-	_, err := os.Stat(filepath.Join(brain, "n.md"))
-	assert.NoError(t, err)
-}
-
-// A special file Watch cannot remove (its directory is read-only) stays
-// reported: the persistent-finding alert names it.
-func TestWatchReportsASpecialFileItCannotRemove(t *testing.T) {
-	if os.Geteuid() == 0 {
-		t.Skip("root ignores directory permissions")
-	}
-	brain := t.TempDir()
-	ro := filepath.Join(brain, "ro")
-	require.NoError(t, os.MkdirAll(ro, 0o755))
-	require.NoError(t, syscall.Mkfifo(filepath.Join(ro, "f.md"), 0o644))
-	require.NoError(t, os.Chmod(ro, 0o555))
-	t.Cleanup(func() { _ = os.Chmod(ro, 0o755) })
-	n := &recNotifier{}
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	go Watch(ctx, brain, 10*time.Millisecond, n)
-	require.Eventually(t, func() bool { return n.count() >= 1 }, 2*time.Second, 10*time.Millisecond)
-	_, err := os.Lstat(filepath.Join(ro, "f.md"))
-	require.NoError(t, err, "the FIFO could not be removed")
-	n.mu.Lock()
-	defer n.mu.Unlock()
-	assert.Contains(t, n.msgs[0], "ro/f.md")
-	assert.Contains(t, n.msgs[0], "special file")
-	for _, m := range n.msgs {
-		assert.NotContains(t, m, "removed", "a file it could not remove was reported as removed")
-	}
-}

ingestion/internal/symlinkguard/symlinkguard.go

@@ -46,16 +46,10 @@ type Notifier interface {
 type Result struct {
 	Symlinks   []string
 	Unreadable []string
-	// Special holds FIFOs, sockets and devices (hyperguild#179): a reader
-	// that opens one by path can hang, and a FIFO at classification.yaml
-	// stops startup.
-	Special []string
 }
 
 // Clean reports whether the scan found nothing to act on.
-func (r Result) Clean() bool {
+func (r Result) Clean() bool { return len(r.Symlinks) == 0 && len(r.Unreadable) == 0 }
-	return len(r.Symlinks) == 0 && len(r.Unreadable) == 0 && len(r.Special) == 0
-}
 
 // String describes the findings for a log line or alert. Paths are quoted, so
 // a name containing a newline cannot blur two findings together.
@@ -65,9 +59,6 @@ func (r Result) String() string {
 	if len(r.Symlinks) > 0 {
 		parts = append(parts, fmt.Sprintf("%d symlink(s): %s", len(r.Symlinks), q(r.Symlinks)))
 	}
-	if len(r.Special) > 0 {
-		parts = append(parts, fmt.Sprintf("%d special file(s) (FIFO, socket or device; delete them): %s", len(r.Special), q(r.Special)))
-	}
 	if len(r.Unreadable) > 0 {
 		parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable)))
 	}
@@ -122,16 +113,12 @@ func Scan(brainDir string) (Result, error) {
 		if d.IsDir() && p == rootGit {
 			return filepath.SkipDir
 		}
-		switch {
+		if d.Type()&os.ModeSymlink != 0 {
-		case d.Type()&os.ModeSymlink != 0:
 			r.Symlinks = append(r.Symlinks, rel)
-		case !d.IsDir() && !d.Type().IsRegular():
-			r.Special = append(r.Special, rel)
 		}
 		return nil
 	})
 	sort.Strings(r.Symlinks)
-	sort.Strings(r.Special)
 	sort.Strings(r.Unreadable)
 	if walkErr != nil {
 		// Only the root's own error reaches here (deeper errors are recorded
@@ -195,20 +182,14 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
 				q.push(fmt.Sprintf("brain symlink tripwire (hyperguild#157): removed %d symlink(s) from the brain dir: %s",
 					len(removed), Result{}.quoted(removed)))
 			}
-			sRemoved, sFailed := removeSpecial(brainDir, r.Special)
+			key = Result{Symlinks: failed, Unreadable: r.Unreadable}.String()
-			if len(sRemoved) > 0 {
-				slog.Error("brain symlink tripwire removed special files", "count", len(sRemoved))
-				q.push(fmt.Sprintf("brain symlink tripwire (hyperguild#179): removed %d special file(s) (FIFO, socket or device) from the brain dir: %s",
-					len(sRemoved), Result{}.quoted(sRemoved)))
-			}
-			key = Result{Symlinks: failed, Unreadable: r.Unreadable, Special: sFailed}.String()
 		}
 		if key != reported {
 			reported = key
 			if key != "" {
 				slog.Error("brain symlink tripwire", "finding", key)
-				q.push("brain symlink tripwire (hyperguild#157, #179): " + key +
+				q.push("brain symlink tripwire (hyperguild#157): " + key +
-					". Symlinks can lead readers out of the brain and special files can hang them; remove them.")
+					". Readers may follow symlinks out of the brain; fix them.")
 			}
 		}
 	}
@@ -341,40 +322,6 @@ func permanent(err error) bool {
 	return errors.As(err, &p) && p.Permanent()
 }
 
-// removeSpecial unlinks each brain-relative FIFO, socket or device through
-// an os.Root on the brain dir, after re-checking that it is still one: a
-// path that is a regular file, a directory or a symlink at the re-check is
-// left alone and reported as failed. The re-check narrows, but cannot close,
-// the window before the unlink (Linux has no conditional unlink); the same
-// window exists in removeSymlinks, and only a local writer can use it.
-func removeSpecial(brainDir string, rels []string) (removed, failed []string) {
-	if len(rels) == 0 {
-		return nil, nil
-	}
-	resolved, err := filepath.EvalSymlinks(brainDir)
-	if err != nil {
-		return nil, rels
-	}
-	root, err := os.OpenRoot(resolved)
-	if err != nil {
-		return nil, rels
-	}
-	defer func() { _ = root.Close() }()
-	for _, rel := range rels {
-		fi, err := root.Lstat(rel)
-		if err != nil || fi.IsDir() || fi.Mode().IsRegular() || fi.Mode()&os.ModeSymlink != 0 {
-			failed = append(failed, rel)
-			continue
-		}
-		if err := root.Remove(rel); err != nil {
-			failed = append(failed, rel)
-			continue
-		}
-		removed = append(removed, rel)
-	}
-	return removed, failed
-}
-
 // removeSymlinks unlinks each brain-relative symlink through an os.Root on
 // the brain dir, after re-checking it is still a symlink. Removing a symlink
 // never touches its target. It returns what was removed and what was not.