fix(brain_edit): snapshot, rollback and lock file go through the brain root (#192)

brain_edit did its own I/O by path around notes.Edit:
- the pre-edit snapshot used os.ReadFile, which hung on a FIFO note held
  open by a writer;
- the rollback (when the provenance stamp fails) used os.WriteFile;
- the edit lock "<note>.lock" was opened with O_CREATE by path, so a
  planted dangling .lock symlink made a successful edit create a file at
  the link's target, in the brain or outside it.

Snapshot is brainfs ReadRegular, rollback WriteRegular, and the lock is
opened and removed through new brainfs OpenLock/RemoveLock: clean path,
real parent directories, an existing lock must be regular, and the opened
handle must be the file at the path (SameFile).

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

ingestion/internal/api/write_keeps_learning_test.go

@@ -0,0 +1,34 @@
+package api_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mathiasbq/hyperguild/ingestion/internal/api"
+)
+
+// hyperguild#178: an unreadable directory under wiki/ made the link check
+// fail and brain_write refuse the note, losing the learning. The note is now
+// written with its links as given.
+func TestWriteNoteKeepsTheLearningWhenTheLinkCheckCannotIndex(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root ignores directory permissions")
+	}
+	brain := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w", "facts"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(brain, "wiki", "w", "facts", "real.md"), []byte("x"), 0o644))
+	locked := filepath.Join(brain, "wiki", "other", "locked")
+	require.NoError(t, os.MkdirAll(locked, 0o755))
+	require.NoError(t, os.Chmod(locked, 0o000))
+	t.Cleanup(func() { _ = os.Chmod(locked, 0o755) })
+
+	rel, err := api.WriteNote(brain, api.WriteNoteOptions{Wing: "w", Hall: "facts", Filename: "n", Content: "learning [[real]] and [[gone]]"})
+	require.NoError(t, err)
+	got, err := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
+	require.NoError(t, err)
+	assert.Contains(t, string(got), "[[gone]]", "links kept as given when the index is partial")
+}

ingestion/internal/brain/linkcheck.go

@@ -47,7 +47,12 @@ var (
 func StripUnresolvedLinks(brainDir, content string) (string, []string, error) {
 	index, err := entryIndex(brainDir)
 	if err != nil {
-		return "", nil, err
+		// A partial index cannot tell an invented link from one into the
+		// part it could not read, so unwrap nothing, and do not refuse the
+		// write either: this runs before every brain_write, and a refused
+		// write is a lost learning (hyperguild#178).
+		slog.Warn("link check skipped: cannot index the brain", "err", err)
+		return content, nil, nil
 	}
 	if len(index) == 0 {
 		return content, nil, nil

ingestion/internal/brain/linkcheck_fallback_test.go

@@ -0,0 +1,41 @@
+package brain_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
+)
+
+// hyperguild#178: StripUnresolvedLinks runs before every brain_write and
+// brain_update. An unreadable directory under knowledge/ or wiki/ made it
+// error, the write was refused and the learning lost. When the link index
+// cannot be built, the content is now returned unchanged (no link is
+// unwrapped, none is invented) and no error stops the write.
+func TestStripUnresolvedLinksKeepsContentWhenTheIndexCannotBeBuilt(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root ignores directory permissions")
+	}
+	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))
+	locked := filepath.Join(b, "wiki", "other", "locked")
+	require.NoError(t, os.MkdirAll(locked, 0o755))
+	require.NoError(t, os.Chmod(locked, 0o000))
+	t.Cleanup(func() { _ = os.Chmod(locked, 0o755) })
+
+	in := "learning [[real]] and [[gone]]"
+	out, unlinked, err := brain.StripUnresolvedLinks(b, in)
+	require.NoError(t, err)
+	assert.Equal(t, in, out, "content unchanged: no link unwrapped on a partial index")
+	assert.Empty(t, unlinked)
+
+	require.NoError(t, os.Chmod(locked, 0o755))
+	out, _, err = brain.StripUnresolvedLinks(b, in)
+	require.NoError(t, err)
+	assert.NotContains(t, out, "[[gone]]", "positive control: with a readable tree the strip runs")
+}

ingestion/internal/brainfs/brainfs.go

@@ -136,6 +136,49 @@ 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

@@ -0,0 +1,61 @@
+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,7 +118,14 @@ 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.
-	prior, err := os.ReadFile(target.Abs)
+	// Read through the brain root, regular files only (hyperguild#192): a
+	// 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)
 	}
@@ -131,7 +138,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 := os.WriteFile(target.Abs, prior, 0o644); werr != nil {
+		if werr := bfs.WriteRegular(target.Rel, prior); 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.Abs)
+	unlock, err := flockFile(t)
 	if err != nil {
 		return Result{}, err
 	}
@@ -220,20 +220,32 @@ func lockFor(abs string) *sync.Mutex {
 }
 
 // flockFile takes an advisory lock covering other processes sharing the
-// hostPath — during a RollingUpdate two ingestion pods do.
-func flockFile(abs string) (func(), error) {
-	f, err := os.OpenFile(abs+".lock", os.O_CREATE|os.O_RDWR, 0o644)
+// hostPath — during a RollingUpdate two ingestion pods do. The lock file is
+// opened and removed through the brain root, never through a symlink
+// (hyperguild#192): a planted dangling "<note>.lock" link made a
+// 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", filepath.Base(abs), err)
+		return nil, fmt.Errorf("notes: lock %s: %w", lockRel, 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", filepath.Base(abs), err)
+		return nil, fmt.Errorf("notes: flock %s: %w", lockRel, err)
 	}
 	return func() {
 		_ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
 		_ = f.Close()
-		_ = os.Remove(abs + ".lock")
+		if b, err := brainfs.Open(t.Brain); err == nil {
+			_ = b.RemoveLock(lockRel)
+			_ = b.Close()
+		}
 	}, nil
 }