fix(tools): lead a failed memory listing with the failure, not empty-store copy

memory.List returns its error alongside whatever it could read precisely so a
caller can tell "there are no notes" from "the notes could not be read" --
escape_test.go pins that at the library. The tool layer put the confusion back:
it rendered renderMemoryList(notes) first and appended the error underneath, so
a redirected .zero, an unreadable scope, or every entry failing to read all
produced an ok result whose body began "No saved notes yet".

A model reading that writes the note again, or proceeds as though nothing was
ever recorded. Zero notes plus an error means the store's contents are UNKNOWN,
which is not the same as known-empty, so that case now returns an error result
whose first line says so.

Partial success is unchanged: notes that were read are still listed with the
failure appended beside them, and a genuinely empty readable store still reports
empty. The appended caveat also now matches the failure -- "Some notes could not
be read" is true of an unreadable entry and false of a containment refusal,
where the store was never opened, and sending an operator to look for corrupt
files is the wrong instruction.

Reported by @jatmn.

internal/tools/memory.go

@@ -103,6 +103,22 @@ func (tool memoryTool) Run(_ context.Context, args map[string]any) Result {
 			listScopes = nil
 		}
 		notes, listErr := memory.List(tool.paths, listScopes...)
+		// NOTHING READABLE PLUS A FAILURE IS NOT AN EMPTY STORE. The two are
+		// indistinguishable to a model, and it acts on the difference: told
+		// "No saved notes yet", it writes the note again, or proceeds as though
+		// nothing was ever recorded. That is exactly the confusion
+		// memory.List's error return exists to prevent — escape_test.go pins it
+		// at the library ("returning an empty list would tell the caller there
+		// are no notes") — and the tool layer put it straight back by rendering
+		// the empty-store copy first and appending the failure underneath.
+		//
+		// So the failure leads, and the result is an error: a redirected .zero,
+		// an unreadable scope, or every entry failing to read are all states
+		// where the store's contents are UNKNOWN, not known to be empty.
+		// Reported by @jatmn.
+		if listErr != nil && len(notes) == 0 {
+			return errorResult(memoryListFailure(listErr))
+		}
 		// BESIDE the notes, not instead of them. memory.List deliberately returns
 		// what it could read alongside the failures, and returning an error here
 		// threw that away — turning the library's careful partial success back
@@ -111,7 +127,7 @@ func (tool memoryTool) Run(_ context.Context, args map[string]any) Result {
 		// appended rather than substituted.
 		rendered := renderMemoryList(notes)
 		if listErr != nil {
-			rendered += "\n\nSome notes could not be read: " + listErr.Error()
+			rendered += "\n\n" + memoryListCaveat(listErr)
 		}
 		return okResult(rendered)
 	}
@@ -285,6 +301,31 @@ func (tool memoryForgetTool) Run(_ context.Context, args map[string]any) Result
 	return okResult(fmt.Sprintf("Forgot %q (%s).", name, scope))
 }
 
+// memoryListFailure is the whole message when the listing produced nothing and
+// failed. It must not read like an empty store, so it says what is unknown and
+// what would make it knowable again.
+func memoryListFailure(listErr error) string {
+	return "The memory store could not be read, so its contents are unknown — this is NOT the same as having no notes. " +
+		"Do not assume memory is empty or rewrite notes that may already exist.\n\n" +
+		memoryListCaveat(listErr)
+}
+
+// memoryListCaveat names what went wrong in terms that match the failure.
+// "Some notes could not be read" is true of an unreadable entry and false of a
+// containment refusal, where the store was never opened at all — telling an
+// operator that notes were unreadable sends them looking for corrupt files
+// instead of the redirected directory that actually caused it.
+func memoryListCaveat(listErr error) string {
+	switch {
+	case errors.Is(listErr, memory.ErrIsSymlink):
+		return "The memory store is a link pointing outside the workspace and was refused: " + listErr.Error()
+	case errors.Is(listErr, memory.ErrUnreadable):
+		return "The memory store exists but could not be opened: " + listErr.Error()
+	default:
+		return "Some notes could not be read: " + listErr.Error()
+	}
+}
+
 func renderMemoryList(notes []memory.Note) string {
 	if len(notes) == 0 {
 		return "No saved notes yet. Use memory_write to keep something a future session could not work out for itself."

internal/tools/memory_test.go

@@ -3,7 +3,9 @@ package tools
 import (
 	"context"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -304,3 +306,88 @@ func TestAnUnscopedListingIsProjectFirst(t *testing.T) {
 		t.Errorf("the listing put local before project, which is the resolution order, not the listing order:\n%s", result.Output)
 	}
 }
+
+// linkDirForTool mirrors internal/memory's helper: a junction on Windows, where
+// os.Symlink needs a privilege an ordinary session does not hold.
+func linkDirForTool(t *testing.T, target, link string) {
+	t.Helper()
+	if runtime.GOOS == "windows" {
+		if out, err := exec.Command("cmd", "/c", "mklink", "/J", link, target).CombinedOutput(); err != nil {
+			t.Skipf("cannot create a junction: %v %s", err, out)
+		}
+		return
+	}
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("cannot create a symlink: %v", err)
+	}
+}
+
+// NOTHING READ PLUS A FAILURE MUST NOT RENDER AS AN EMPTY STORE. memory.List
+// returns its error precisely so a caller can tell "there are no notes" from
+// "the notes could not be read" — escape_test.go pins that at the library — and
+// the tool put the confusion back by rendering the empty-store copy first and
+// appending the failure below it. A model reading "No saved notes yet" writes
+// the note again, or proceeds as though nothing was ever recorded.
+//
+// Both ways of reaching zero-notes-plus-error are covered: a redirected .zero
+// (the store is never opened) and every on-disk entry failing to read (the
+// store opens, its contents do not).
+func TestAFailedMemoryListingDoesNotRenderAsAnEmptyStore(t *testing.T) {
+	t.Run("redirected store", func(t *testing.T) {
+		base := t.TempDir()
+		outside := filepath.Join(base, "outside")
+		workspace := filepath.Join(base, "workspace")
+		for _, dir := range []string{outside, workspace, filepath.Join(outside, "memory")} {
+			if err := os.MkdirAll(dir, 0o700); err != nil {
+				t.Fatal(err)
+			}
+		}
+		linkDirForTool(t, outside, filepath.Join(workspace, ".zero"))
+		paths := memory.DefaultPaths(workspace)
+		// The library's own answer is the premise of this test.
+		if notes, err := memory.List(paths); err == nil || len(notes) != 0 {
+			t.Fatalf("premise failed: List returned %d notes, err=%v", len(notes), err)
+		}
+
+		result := NewMemoryTool(paths).Run(context.Background(), map[string]any{})
+		if result.Status != StatusError {
+			t.Errorf("an unreadable store reported ok: %q", result.Output)
+		}
+		if strings.Contains(strings.ToLower(result.Output), "no saved notes") {
+			t.Errorf("an unreadable store was rendered as an empty one: %q", result.Output)
+		}
+		if !strings.Contains(strings.ToLower(result.Output), "unknown") {
+			t.Errorf("the output does not say the contents are unknown: %q", result.Output)
+		}
+		// The caveat has to match the failure: nothing was "unread" here, the
+		// store was refused outright, and sending an operator to look for
+		// corrupt note files is the wrong instruction.
+		if strings.Contains(result.Output, "Some notes could not be read") {
+			t.Errorf("a containment refusal was reported as unreadable notes: %q", result.Output)
+		}
+	})
+
+	t.Run("readable store with readable notes still lists", func(t *testing.T) {
+		paths := memoryTestPaths(t)
+		if _, err := memory.Write(paths, memory.ScopeProject, "kept", "d", "body"); err != nil {
+			t.Fatal(err)
+		}
+		result := NewMemoryTool(paths).Run(context.Background(), map[string]any{})
+		if result.Status != StatusOK || !strings.Contains(result.Output, "kept") {
+			t.Fatalf("a healthy listing regressed: status=%s %q", result.Status, result.Output)
+		}
+	})
+
+	// And a genuinely empty, readable store still says so — the fix must not
+	// turn "no notes" into an error.
+	t.Run("empty readable store still reports empty", func(t *testing.T) {
+		paths := memoryTestPaths(t)
+		result := NewMemoryTool(paths).Run(context.Background(), map[string]any{})
+		if result.Status != StatusOK {
+			t.Fatalf("an empty store reported an error: %q", result.Output)
+		}
+		if !strings.Contains(strings.ToLower(result.Output), "no saved notes") {
+			t.Errorf("an empty store lost its empty-store copy: %q", result.Output)
+		}
+	})
+}