fix(watcher): park through the brain root, keep a note edited mid-resolve, honour only real fences (#205)

From the second independent review of #207:
- parkForReview now skips the delete when the note changed since it was read
  (an LLM call sits in that window; Syncthing can deliver a newer version)
- parked copies are created via brainfs (real dirs only, O_EXCL, no symlink
  following), so a planted raw/needs-review symlink cannot redirect or hide a
  note; this also removes the need for the guard allowlist entry added in
  1e14c56, which is reverted
- a '----' line no longer closes the proposal block
Each has a test that failed first and a mutant that fails it.

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

ingestion/internal/extract/callsites_test.go

@@ -89,9 +89,6 @@ func TestNoPackageReferencesExtractTextOutsideExtract(t *testing.T) {
 // on a path that is not a raw/ read.
 var osOpenAllowed = map[string]bool{
 	"internal/watcher/watcher.go:appendWatcherLog": true, // ingest log append, not a raw/ read
-	// Claims a free name in raw/needs-review/ with O_EXCL so a parked note is
-	// never overwritten (#205). It creates a file; it reads nothing from raw/.
-	"internal/watcher/watcher.go:reserveParkedName": true,
 	// Text reads by path; outside this package only TextIn's temp copy reaches
 	// it (TestNoPackageReferencesExtractTextOutsideExtract enforces that).
 	"internal/extract/extract.go:Text": true,

ingestion/internal/watcher/park.go

@@ -0,0 +1,44 @@
+package watcher
+
+import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"path/filepath"
+	"strings"
+
+	"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
+)
+
+// Kept out of watcher.go on purpose: that file is covered by the raw-reader
+// guard (internal/extract/callsites_test.go), which forbids any .Open selector
+// there. This function reads nothing from raw/; it creates a file under the
+// brain root, which refuses symlinked directories and never follows a final
+// symlink.
+
+// createParked writes data to raw/needs-review/<filename> through the brain
+// root, never replacing a parked note: on a name clash it tries -2, -3, ...
+// A needs-review that is a symlink, or any non-directory, is refused.
+func createParked(brainDir, filename string, data []byte) error {
+	bfs, err := brainfs.Open(brainDir)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = bfs.Close() }()
+	ext := filepath.Ext(filename)
+	stem := strings.TrimSuffix(filename, ext)
+	for n := 1; n < 10000; n++ {
+		name := filename
+		if n > 1 {
+			name = fmt.Sprintf("%s-%d%s", stem, n, ext)
+		}
+		err := bfs.CreateRegular("raw/needs-review/"+name, data)
+		if err == nil {
+			return nil
+		}
+		if !errors.Is(err, fs.ErrExist) {
+			return err
+		}
+	}
+	return fmt.Errorf("no free name for %q in raw/needs-review", filename)
+}

ingestion/internal/watcher/watcher.go

@@ -3,10 +3,8 @@ package watcher
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
-	"io/fs"
 	"log/slog"
 	"os"
 	"path/filepath"
@@ -178,7 +176,7 @@ func processFile(ctx context.Context, cfg Config, root *os.Root, rawDir, path, d
 	if cfg.Resolver != nil {
 		d := cfg.Resolver.Resolve(ctx, wing.Note{Dir: topDir(rel), FrontmatterWing: frontmatterWing(content), Text: content})
 		if d.Source == wing.Exception {
-			return parkForReview(cfg, root, rawDir, rel, path, filename, content, d, date)
+			return parkForReview(cfg, root, rel, path, filename, content, d, date, before)
 		}
 		pcfg.Wing = d.Wing
 		slog.Info("watcher: wing resolved", "file", filename, "wing", d.Wing, "how", d.Source, "reason", d.Reason)
@@ -352,58 +350,43 @@ func frontmatterWing(content string) string {
 
 // parkForReview moves a note the resolver could not place into
 // raw/needs-review/, recording the proposal in its frontmatter so a human can
-// approve it (brain_promote). Nothing is extracted or written to the wiki. The
+// approve it by moving it into raw/<wing>/. Nothing is extracted or written to
-// original is removed only after the parked copy is written.
+// the wiki.
-func parkForReview(cfg Config, root *os.Root, rawDir, rel, path, filename, content string, d wing.Decision, date string) error {
+//
-	dir := filepath.Join(rawDir, "needs-review")
+// The parked copy is created through the brain root (real directories only,
-	if err := os.MkdirAll(dir, 0o755); err != nil {
+// O_EXCL, never overwriting) and is the only copy once the original is retired,
-		return fmt.Errorf("mkdir needs-review: %w", err)
+// so the original is removed only after that write succeeds and only if it has
-	}
+// not changed since it was read: a newer version delivered while the resolver
-	dest, err := reserveParkedName(dir, filename)
+// ran stays in raw/ for the next poll.
-	if err != nil {
+func parkForReview(cfg Config, root *os.Root, rel, path, filename, content string, d wing.Decision, date string, before os.FileInfo) error {
-		return fmt.Errorf("park note: %w", err)
+	var data []byte
-	}
 	if strings.ToLower(filepath.Ext(filename)) == ".pdf" {
 		// Binary: park the bytes untouched; the proposal is in log.md only.
-		if err := copyFromRoot(root, rel, dest); err != nil {
+		in, _, err := extract.OpenRegular(root, rel)
-			_ = os.Remove(dest)
+		if err != nil {
 			return fmt.Errorf("park pdf: %w", err)
 		}
-	} else if err := os.WriteFile(dest, []byte(withProposal(content, d)), 0o644); err != nil {
+		data, err = io.ReadAll(in)
-		_ = os.Remove(dest)
+		_ = in.Close()
+		if err != nil {
+			return fmt.Errorf("park pdf: %w", err)
+		}
+	} else {
+		data = []byte(withProposal(content, d))
+	}
+	if err := createParked(cfg.BrainDir, filename, data); err != nil {
 		return fmt.Errorf("park note: %w", err)
 	}
 	if err := appendWatcherLog(cfg.BrainDir, filename, fmt.Errorf("needs review (proposed wing %q): %s", d.Wing, d.Reason), date); err != nil {
 		slog.Error("watcher: failed to write log entry", "error", err)
 	}
 	slog.Info("watcher: parked for review", "file", filename, "proposed_wing", d.Wing, "reason", d.Reason)
-	retire(cfg, root, rel, path, ".processed", date)
+	if changedSince(root, rel, before) {
+		slog.Warn("watcher: note changed while it was being resolved; leaving the newer version in raw/", "file", filename)
 		return nil
 	}
-
+	retire(cfg, root, rel, path, ".processed", date)
-// reserveParkedName claims a free name in dir for filename, never reusing one:
+	return nil
-// the parked copy is the only copy once the original is retired, so a note
-// that shares a name with an earlier parked note must not replace it. The name
-// is created with O_EXCL; -2, -3, ... is appended until one is free.
-func reserveParkedName(dir, filename string) (string, error) {
-	ext := filepath.Ext(filename)
-	stem := strings.TrimSuffix(filename, ext)
-	for n := 1; n < 10000; n++ {
-		name := filename
-		if n > 1 {
-			name = fmt.Sprintf("%s-%d%s", stem, n, ext)
-		}
-		dest := filepath.Join(dir, name)
-		f, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
-		if err == nil {
-			_ = f.Close()
-			return dest, nil
-		}
-		if !errors.Is(err, fs.ErrExist) {
-			return "", err
-		}
-	}
-	return "", fmt.Errorf("no free name for %q in %s", filename, dir)
 }
 
 // proposalKeys are the frontmatter keys the resolver owns in a parked note.
@@ -429,7 +412,7 @@ func withProposal(content string, d wing.Decision) string {
 	if !strings.HasPrefix(content, "---\n") {
 		return "---\n" + kv.String() + "---\n" + content
 	}
-	block, rest, found := strings.Cut(content[4:], "\n---")
+	block, rest, found := cutFence(content[4:])
 	if !found {
 		return "---\n" + kv.String() + "---\n" + content
 	}
@@ -442,3 +425,15 @@ func withProposal(content string, d wing.Decision) string {
 	}
 	return "---\n" + kv.String() + strings.Join(kept, "\n") + "\n---" + rest
 }
+
+// cutFence splits s at the first closing fence: a line that is exactly "---".
+// "----" or "---x" is not a fence, matching how the frontmatter readers parse.
+func cutFence(s string) (block, rest string, found bool) {
+	if i := strings.Index(s, "\n---\n"); i >= 0 {
+		return s[:i], s[i+4:], true
+	}
+	if strings.HasSuffix(s, "\n---") {
+		return s[:len(s)-4], "", true
+	}
+	return "", "", false
+}

ingestion/internal/watcher/watcher_wing_test.go

@@ -271,3 +271,61 @@ func linesStartingWith(s, prefix string) int {
 	}
 	return n
 }
+
+// Resolving costs an LLM call. If Syncthing delivers a newer version of the note
+// meanwhile, deleting the original loses that version: the parked copy is stale.
+func TestProcessDir_ANoteEditedWhileTheResolverRunsIsNotDeleted(t *testing.T) {
+	var llmCalls atomic.Int32
+	var notePath string
+	clf := &editingClassifier{}
+	cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{}), &llmCalls)
+	notePath = filepath.Join(brainDir, "raw", "n.md")
+	clf.edit = func() { require.NoError(t, os.WriteFile(notePath, []byte("NEWER and longer version"), 0o644)) }
+	put(t, brainDir, "n.md", "ORIGINAL")
+
+	require.Empty(t, processDir(context.Background(), cfg, "2026-10-20"))
+
+	got, err := os.ReadFile(notePath)
+	require.NoError(t, err, "the edited note must still be in raw/")
+	assert.Equal(t, "NEWER and longer version", string(got))
+}
+
+type editingClassifier struct{ edit func() }
+
+func (c *editingClassifier) Classify(context.Context, string, []string) (wing.Suggestion, error) {
+	c.edit()
+	return wing.Suggestion{}, nil
+}
+
+// A planted symlink at raw/needs-review must not redirect a parked note out of
+// the brain, nor lose it from the review queue.
+func TestProcessDir_AParkedNoteIsNeverWrittenThroughASymlinkedNeedsReview(t *testing.T) {
+	var llmCalls atomic.Int32
+	cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls)
+	outside := t.TempDir()
+	require.NoError(t, os.Symlink(outside, filepath.Join(brainDir, "raw", "needs-review")))
+	put(t, brainDir, "n.md", "Note.")
+
+	errs := processDir(context.Background(), cfg, "2026-10-20")
+
+	assert.NotEmpty(t, errs, "parking is refused, loudly")
+	entries, _ := os.ReadDir(outside)
+	assert.Empty(t, entries, "nothing was written through the symlink")
+	assert.True(t, has(brainDir, "raw/n.md"), "the note stays where it was")
+}
+
+func TestWithProposal_ANonFenceDashLineDoesNotCloseTheBlock(t *testing.T) {
+	out := withProposal("---\nfoo: bar\n----\nBody", wing.Decision{Wing: "radar", Confidence: 0.4, Reason: "r"})
+	lines := strings.Split(out, "\n")
+	require.Equal(t, "---", lines[0])
+	closing := -1
+	for i := 1; i < len(lines); i++ {
+		if lines[i] == "---" {
+			closing = i
+			break
+		}
+	}
+	require.Positive(t, closing, "the proposal block is closed by a real fence")
+	assert.Contains(t, strings.Join(lines[:closing], "\n"), "proposal_reason:")
+	assert.NotContains(t, strings.Join(lines[:closing], "\n"), "foo: bar", "the unfenced note is body, not part of the block")
+}