From 1e57f495950e6c3064a6af89db992837b201f020 Mon Sep 17 00:00:00 2001
From: Mathias <mthbqv@gmail.com>
Date: Tue, 22 Sep 2026 23:18:43 +0200
Subject: [PATCH] fix(mcp): name raw argument bytes when brain_edit refuses an
 empty old_str
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

hyperguild#115: a session reported "old_str is empty" with old_str that
was never empty on their end. Server-side round-trip re-verified clean
(fresh brain_edit call, real content, succeeds) — the code path itself
isn't broken, but its error left no way to tell "client sent empty" apart
from "something between the client and here lost the field" without a
second investigation. The refusal now names the raw argument byte count.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 ingestion/internal/mcp/tools_notes.go      |  8 ++++++++
 ingestion/internal/mcp/tools_notes_test.go | 23 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go
index fe0e813..866ffde 100644
--- a/ingestion/internal/mcp/tools_notes.go
+++ b/ingestion/internal/mcp/tools_notes.go
@@ -3,6 +3,7 @@ package mcp
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"strings"
@@ -180,6 +181,13 @@ func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (notes.R
 	}
 	res, err := notes.Edit(s.brainDir, a.Path, a.OldStr, a.NewStr, scope)
 	if err != nil {
+		if errors.Is(err, notes.ErrDegenerate) && a.OldStr == "" {
+			// hyperguild#115: "old_str is empty" alone doesn't say whether the
+			// caller sent it empty or a hop between the caller and here lost
+			// it. Naming the raw argument byte count makes that distinguishable
+			// without a second investigation.
+			return notes.Result{}, nil, fmt.Errorf("%w (raw arguments were %d bytes)", err, len(args))
+		}
 		return notes.Result{}, nil, err
 	}
 
diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go
index 540a03d..b936e75 100644
--- a/ingestion/internal/mcp/tools_notes_test.go
+++ b/ingestion/internal/mcp/tools_notes_test.go
@@ -70,6 +70,29 @@ func TestBrainReadAndEdit_RoundTripThroughJSONRPC(t *testing.T) {
 		"only the matched bytes may change")
 }
 
+func TestBrainEdit_EmptyOldStrErrorNamesRawArgumentBytes(t *testing.T) {
+	// hyperguild#115: a session reported "old_str is empty" with an old_str
+	// that, on their end, was never empty. The server-side path was
+	// unreachable to reproduce (verified separately: a fresh brain_edit call
+	// with real content round-trips cleanly), which points at something
+	// between a specific client and this call losing the field. The fix this
+	// issue actually asked for isn't a behavior change — old_str=="" must
+	// still refuse — it's that the refusal should say how many raw argument
+	// bytes it received, so "client sent empty" is distinguishable from
+	// "server received empty after some hop lost it" without a second
+	// session's worth of guessing.
+	dir := newBrainRepo(t)
+	srv := mcp.NewServer(dir, nil, nil, nil)
+
+	resp := call(t, srv, "brain_edit", map[string]any{
+		"path": "wiki/homelab/facts/n.md", "old_str": "", "new_str": "BETA",
+	})
+	errMsg, ok := resp["error"].(map[string]any)["message"].(string)
+	require.True(t, ok, "%v", resp)
+	assert.Contains(t, errMsg, "old_str is empty")
+	assert.Contains(t, errMsg, "raw arguments were", "%v", errMsg)
+}
+
 func TestBrainEdit_RefusesGovernanceFilesFromTheModelFacingTool(t *testing.T) {
 	dir := newBrainRepo(t)
 	srv := mcp.NewServer(dir, nil, nil, nil)