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)