From 22d0653b6196146f3f8c1c9e57afeadf83f4b52e Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 23:47:24 +0200 Subject: [PATCH] test(watcher): a marker link planted during the run is refused and not retried (#158) Review round 2 on PR #195: the marker writes had no test, so reverting them to by-path writes, or the refusal to log-only, stayed green. The link is planted from inside the fake model call, between the walk's Lstat and the marker write, for both markers and both an outside and an in-brain relative target; red on all four flips. The marker-entry skip is now checked for .failed too. Also pins cleanRel's acceptance of a backslash as a filename byte in brainfs itself, and stops the buildAndWrite control claiming writes it does not count. Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/brainfs/brainfs_test.go | 17 ++++ ingestion/internal/pipeline/root_test.go | 2 +- .../internal/watcher/writes_redirect_test.go | 78 ++++++++++++++++--- 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index 0f5570b..5ac4f4d 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -529,3 +529,20 @@ func TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories(t *testing.T) { got, _ = os.ReadFile(filepath.Join(dir, "wiki", "new", "facts", "n.md")) assert.Equal(t, "one", string(got)) } + +// On Linux a backslash is an ordinary filename byte, not a separator, so a +// name containing one stays a single in-root component (hyperguild#158: the +// watcher retried such a note forever while cleanRel refused it). +func TestReadAndPutRegularTreatABackslashAsAFilenameByte(t *testing.T) { + dir := t.TempDir() + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + require.NoError(t, b.PutRegular(`a\..\b.md`, []byte("x"))) + got, err := os.ReadFile(filepath.Join(dir, `a\..\b.md`)) + require.NoError(t, err, "the name was not kept as one component") + assert.Equal(t, "x", string(got)) + got, err = b.ReadRegular(`a\..\b.md`) + require.NoError(t, err) + assert.Equal(t, "x", string(got)) +} diff --git a/ingestion/internal/pipeline/root_test.go b/ingestion/internal/pipeline/root_test.go index a727e2d..9b92cde 100644 --- a/ingestion/internal/pipeline/root_test.go +++ b/ingestion/internal/pipeline/root_test.go @@ -213,5 +213,5 @@ func TestBuildAndWriteNeverWritesThroughAPageSwappedForASymlink(t *testing.T) { } got, _ := os.ReadFile(target) assert.Equal(t, "untouched", string(got), "buildAndWrite wrote through a swapped-in symlink") - assert.Positive(t, ok, "positive control: some runs wrote the page") + assert.Positive(t, ok, "positive control: some runs returned without error") } diff --git a/ingestion/internal/watcher/writes_redirect_test.go b/ingestion/internal/watcher/writes_redirect_test.go index fe4efed..dc06188 100644 --- a/ingestion/internal/watcher/writes_redirect_test.go +++ b/ingestion/internal/watcher/writes_redirect_test.go @@ -82,17 +82,73 @@ func TestProcessDirDoesNotAppendTheLogThroughASymlink(t *testing.T) { // marker: the note is skipped and nothing is created at the link's target. // (os.Stat followed the link, saw nothing, and processed the note again.) func TestProcessDirTreatsAnyEntryAtTheMarkerPathAsAMarker(t *testing.T) { - brainDir := setupBrainDir(t) - outside := filepath.Join(t.TempDir(), "created") - require.NoError(t, os.WriteFile(filepath.Join(brainDir, "raw", "n.md"), []byte("note"), 0o644)) - require.NoError(t, os.Symlink(outside, filepath.Join(brainDir, "raw", "n.md.processed"))) - calls := 0 - cfg := Config{BrainDir: brainDir, Interval: time.Hour, Pipeline: pipeline.Config{ - Complete: func(context.Context, string, string) (string, error) { calls++; return "[]", nil }, Schema: "# Schema"}} - _ = processDir(context.Background(), cfg, "2026-10-05") - assert.Zero(t, calls, "the note was processed despite an entry at its marker path") - _, err := os.Lstat(outside) - assert.True(t, os.IsNotExist(err), "a file was created at the marker link's target") + for _, kind := range []string{"processed", "failed"} { + t.Run(kind, func(t *testing.T) { + brainDir := setupBrainDir(t) + outside := filepath.Join(t.TempDir(), "created") + require.NoError(t, os.WriteFile(filepath.Join(brainDir, "raw", "n.md"), []byte("note"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(brainDir, "raw", "n.md."+kind))) + calls := 0 + cfg := Config{BrainDir: brainDir, Interval: time.Hour, Pipeline: pipeline.Config{ + Complete: func(context.Context, string, string) (string, error) { calls++; return "[]", nil }, Schema: "# Schema"}} + _ = processDir(context.Background(), cfg, "2026-10-05") + assert.Zero(t, calls, "the note was processed despite an entry at its marker path") + _, err := os.Lstat(outside) + assert.True(t, os.IsNotExist(err), "a file was created at the marker link's target") + }) + } +} + +// The marker write itself is reachable while pipeline.Run is in flight +// (in production, an LLM call): a link planted at the marker path then is +// refused, and the note is remembered, so removing the link (as +// symlinkguard does) does not make the watcher call the model again. +func TestProcessDirRefusesAMarkerLinkPlantedDuringTheRunAndDoesNotRetry(t *testing.T) { + for _, kind := range []string{"processed", "failed"} { + for _, target := range []string{"outside", "in-brain"} { + t.Run(kind+"/"+target, func(t *testing.T) { + brainDir := setupBrainDir(t) + raw := filepath.Join(brainDir, "raw") + marker := filepath.Join(raw, "n.md."+kind) + require.NoError(t, os.WriteFile(filepath.Join(raw, "n.md"), []byte("note"), 0o644)) + // The in-brain target is relative, so os.Root itself would + // follow it; only the regular-file check refuses it. + victim := filepath.Join(raw, "victim") + link := filepath.Join(t.TempDir(), "created") + if target == "in-brain" { + require.NoError(t, os.WriteFile(victim, []byte("untouched"), 0o644)) + link = "victim" + } + ok := okPipeline().Complete + calls := 0 + cfg := Config{BrainDir: brainDir, Interval: time.Hour, Pipeline: pipeline.Config{Schema: "# Schema", + Complete: func(ctx context.Context, sys, user string) (string, error) { + calls++ + require.NoError(t, os.Symlink(link, marker)) + if kind == "failed" { + return "", errors.New("model down") + } + return ok(ctx, sys, user) + }}} + for i := 0; i < 3; i++ { + errs := processDir(context.Background(), cfg, "2026-10-05") + if i == 0 { + require.NotEmpty(t, errs, "the planted link was not refused") + } + _ = os.Remove(marker) + } + assert.Equal(t, 1, calls, "the note was retried after its marker write was refused") + if target == "in-brain" { + got, err := os.ReadFile(victim) + require.NoError(t, err) + assert.Equal(t, "untouched", string(got), "the marker was written through the link") + } else { + _, err := os.Lstat(link) + assert.True(t, os.IsNotExist(err), "the marker was written through the link") + } + }) + } + } } // A refused archive copy must not retry every interval: the note is