Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions internal/memory/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,13 @@ func TestMergeDuplicatesAndConflictsReport(t *testing.T) {
}
}

func TestMarkOutdatedDownranksMemory(t *testing.T) {
// TestMarkOutdatedHidesSupersededMemoryFromRecall pins issue #18. Marking an
// entry outdated WITH a successor now removes it from semantic recall instead
// of merely downranking it: the old vector is unchanged, so downranking alone
// still let the dead copy compete with its successor. The downrank path stays
// in force for MarkOutdated without a successor — see
// TestOutdatedWithoutSuccessorStaysRecallable.
func TestMarkOutdatedHidesSupersededMemoryFromRecall(t *testing.T) {
store := newTestStore(t)

current := &Memory{
Expand Down Expand Up @@ -998,12 +1004,15 @@ func TestMarkOutdatedDownranksMemory(t *testing.T) {
if err != nil {
t.Fatalf("Recall: %v", err)
}
if len(results) < 2 {
t.Fatalf("expected 2 results, got %d", len(results))
if len(results) != 1 {
t.Fatalf("expected 1 result, got %d", len(results))
}
if results[0].Memory.ID != current.ID {
t.Fatalf("top result = %s, want %s", results[0].Memory.ID, current.ID)
}
if _, err := store.Get(old.ID); err != nil {
t.Fatalf("superseded entry must stay retrievable by id: %v", err)
}
outdated, err := store.Get(old.ID)
if err != nil {
t.Fatalf("Get old: %v", err)
Expand Down
20 changes: 20 additions & 0 deletions internal/memory/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ func (ms *Store) snapshotReadonlyMemories() []*cachedMemory {
return snapshot
}

// hasMemory reports whether the given id is present in the cache. Used to tell
// a live supersession pointer from a dangling one.
func (ms *Store) hasMemory(id string) bool {
ms.mu.RLock()
defer ms.mu.RUnlock()
_, ok := ms.memories[id]
return ok
}

// snapshotForContext returns a read-only snapshot pre-filtered by context.
func (ms *Store) snapshotForContext(ctx string) []*cachedMemory {
if ctx == "" {
Expand Down Expand Up @@ -197,6 +206,17 @@ func (ms *Store) Recall(ctx context.Context, query string, filters Filters, limi
continue
}

// Superseded entries (temporal replacement, e.g. after a merge or
// MarkOutdated) are invisible to semantic recall — the successor
// carries the current knowledge, while the old vector is unchanged and
// keeps out-ranking it. They stay visible to List/ListLightweight so
// maintenance tools still see the temporal history. The successor is
// looked up rather than trusted: Delete does not clear superseded_by on
// predecessors, and a dangling pointer would bury the entry forever.
if m.SupersededBy != "" && ms.hasMemory(m.SupersededBy) {
continue
}

// T48 layer-aware filtering: when the flag is on, surface memories
// are invisible outside their originating Context. This prevents
// session scratch state from leaking into unrelated recall calls.
Expand Down
160 changes: 160 additions & 0 deletions internal/memory/superseded_recall_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package memory

import (
"context"
"path/filepath"
"testing"

"github.com/ipiton/agent-memory-mcp/internal/embedder"
"go.uber.org/zap"
)

// newSupersessionTestStore builds a store over a stub embedder that returns the
// same vector for every input — recall then ranks purely on the metadata
// weights, so the assertions below are about the filter, not about similarity.
func newSupersessionTestStore(t *testing.T) *Store {
t.Helper()

server := newEmbeddingTestServer(t, []float64{1, 0, 0, 0})
t.Cleanup(server.Close)

emb, err := embedder.New(embedder.Config{
OpenAIToken: "test-token",
OpenAIBaseURL: server.URL,
OpenAIModel: "test-model",
Dimension: 4,
}, zap.NewNop())
if err != nil {
t.Fatalf("New embedder: %v", err)
}

store, err := NewStore(filepath.Join(t.TempDir(), "superseded.db"), emb, zap.NewNop())
if err != nil {
t.Fatalf("NewStore: %v", err)
}
t.Cleanup(func() { _ = store.Close() })

return store
}

func storeSupersessionPair(t *testing.T, store *Store) {
t.Helper()

ctx := context.Background()

old := &Memory{
ID: "old-1",
Title: "Deploy runbook",
Content: "Deploy runbook: scale the api deployment to three replicas.",
Type: TypeSemantic,
Context: "payments",
}
successor := &Memory{
ID: "new-1",
Title: "Deploy runbook",
Content: "Deploy runbook: scale the api deployment to five replicas.",
Type: TypeSemantic,
Context: "payments",
}
for _, m := range []*Memory{old, successor} {
if err := store.Store(ctx, m); err != nil {
t.Fatalf("Store %s: %v", m.ID, err)
}
}

if err := store.SetTemporalFields(ctx, old.ID, nil, nil, successor.ID, ""); err != nil {
t.Fatalf("SetTemporalFields: %v", err)
}
}

func recalledIDs(t *testing.T, store *Store) map[string]bool {
t.Helper()

results, err := store.Recall(context.Background(), "deploy runbook replicas", Filters{Context: "payments"}, 10)
if err != nil {
t.Fatalf("Recall: %v", err)
}
ids := make(map[string]bool, len(results))
for _, r := range results {
ids[r.Memory.ID] = true
}
return ids
}

// TestSupersededMemoryExcludedFromRecall pins issue #18: an entry whose
// superseded_by points at a live successor stays out of semantic recall (its
// vector is unchanged, so it kept out-ranking the successor), while remaining
// visible to the List-based maintenance views.
func TestSupersededMemoryExcludedFromRecall(t *testing.T) {
store := newSupersessionTestStore(t)
storeSupersessionPair(t, store)

ids := recalledIDs(t, store)
if ids["old-1"] {
t.Error("superseded entry leaked into semantic recall")
}
if !ids["new-1"] {
t.Error("successor missing from recall")
}

listed, err := store.List(context.Background(), Filters{Context: "payments"}, 10)
if err != nil {
t.Fatalf("List: %v", err)
}
found := false
for _, m := range listed {
if m.ID == "old-1" {
found = true
}
}
if !found {
t.Error("superseded entry must stay visible to List for temporal history")
}

lightweight := store.ListLightweight(Filters{Context: "payments"})
found = false
for _, m := range lightweight {
if m.ID == "old-1" {
found = true
}
}
if !found {
t.Error("superseded entry must stay visible to ListLightweight")
}
}

// TestOutdatedWithoutSuccessorStaysRecallable guards the boundary of the new
// filter: MarkOutdated with an empty supersededBy leaves superseded_by unset,
// so the entry keeps the pre-existing downrank treatment (lower importance,
// archived metadata) and stays in recall. Nothing replaced it — hiding it would
// simply lose the knowledge.
func TestOutdatedWithoutSuccessorStaysRecallable(t *testing.T) {
store := newSupersessionTestStore(t)
storeSupersessionPair(t, store)

if _, err := store.MarkOutdated(context.Background(), "new-1", "just stale", ""); err != nil {
t.Fatalf("MarkOutdated: %v", err)
}

if !recalledIDs(t, store)["new-1"] {
t.Error("outdated entry without a successor must stay in recall")
}
}

// TestSupersededByDanglingPointerStaysRecallable covers the other side of the
// filter: Delete does not clear superseded_by on predecessors, so once the
// successor is gone the pointer dangles. An archived entry beats no entry at
// all — the old memory must come back into recall rather than stay buried.
func TestSupersededByDanglingPointerStaysRecallable(t *testing.T) {
store := newSupersessionTestStore(t)
storeSupersessionPair(t, store)

if err := store.Delete(context.Background(), "new-1"); err != nil {
t.Fatalf("Delete successor: %v", err)
}

ids := recalledIDs(t, store)
if !ids["old-1"] {
t.Error("entry with a dangling superseded_by pointer stayed buried in recall")
}
}
4 changes: 4 additions & 0 deletions internal/rag/rag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,10 @@ func TestClassifySourceType(t *testing.T) {
{path: "k8s/ingress.yaml", want: "k8s"},
{path: "dead_ends/why-we-avoid-async-migration.md", want: "dead_end"},
{path: "notes/why-we-avoid-shared-mutable-state.md", want: "dead_end"},
// Issue #19: plain markdown outside docs/ and without a "# " heading
// used to classify as "" and get silently dropped from the index.
{path: "knowledge/mahoo-architecture.md", want: "docs"},
{path: "plans/analysis.md", want: "docs"},
}

for _, tc := range tests {
Expand Down
89 changes: 88 additions & 1 deletion internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,98 @@ func (s *MCPServer) dispatch(req rpcRequest) (any, *rpcError) {
case "tools/call":
return s.handleToolsCall(req.Params)
default:
s.logUnknownMethod(req)
return nil, &rpcError{Code: rpcErrMethodNotFound, Message: "method not found"}
}
}

func (s *MCPServer) handleInitialize(_ json.RawMessage) (any, *rpcError) {
// requestMeta is the subset of a request's `_meta` we inspect. Under revision
// 2026-07-28 clients carry their protocol version on every request; under the
// revision we implement (2025-11-25) the field is simply absent.
type requestMeta struct {
Meta struct {
ProtocolVersion string `json:"io.modelcontextprotocol/protocolVersion"`
} `json:"_meta"`
}

// logUnknownMethod records calls to methods we do not implement.
//
// This is the migration tripwire for MCP-PROTOCOL-MIGRATION-2026-07-28, and it
// exists because the obvious place to watch — initialize — turned out to be
// blind. Measured 2026-08-10 against a live client: Claude Code reconnects to a
// restarted HTTP server by going straight to tools/call and never re-sends
// initialize, which our dispatch accepts because no handshake is required. A
// detector on initialize therefore never fires for the case it was built for.
//
// An unknown method is the reliable signal instead: a client that has moved to
// 2026-07-28 calls server/discover, a mandatory RPC we do not implement, and
// today that returns method-not-found silently. The protocol version is read
// from _meta when present, so the line says which revision the caller speaks
// rather than only that something unknown was asked for.
func (s *MCPServer) logUnknownMethod(req rpcRequest) {
if s.fileLogger == nil {
return
}

var m requestMeta
if len(req.Params) > 0 {
// A parse failure is not worth reporting separately: the method is
// unknown either way, and that is the fact we are recording.
_ = json.Unmarshal(req.Params, &m)
}

s.fileLogger.Warn("unknown method",
zap.String("method", req.Method),
zap.String("client_protocol_version", m.Meta.ProtocolVersion),
zap.String("server_protocol_version", protocolVersion),
)
}

// initializeParams is the subset of the client's initialize request we inspect.
// Everything outside these fields is deliberately ignored.
type initializeParams struct {
ProtocolVersion string `json:"protocolVersion"`
ClientInfo struct {
Name string `json:"name"`
Version string `json:"version"`
} `json:"clientInfo"`
}

// logClientProtocolVersion records which protocol revision the client asked for.
//
// The server does not negotiate: handleInitialize always answers with its own
// protocolVersion regardless of the request. Without this log line a divergence
// between client and server is unobservable, so we would learn that the client
// moved to a newer revision from a failure rather than from telemetry. Logging
// only — the response is unchanged, and a mismatch is not an error here.
func (s *MCPServer) logClientProtocolVersion(params json.RawMessage) {
if s.fileLogger == nil {
return
}

var p initializeParams
if len(params) > 0 {
if err := json.Unmarshal(params, &p); err != nil {
s.fileLogger.Warn("initialize: failed to parse params",
zap.String("server_protocol_version", protocolVersion),
zap.Error(err),
)
return
}
}

s.fileLogger.Info("initialize: client protocol version",
zap.String("client_protocol_version", p.ProtocolVersion),
zap.String("server_protocol_version", protocolVersion),
zap.Bool("protocol_version_match", p.ProtocolVersion == protocolVersion),
zap.String("client_name", p.ClientInfo.Name),
zap.String("client_version", p.ClientInfo.Version),
)
}

func (s *MCPServer) handleInitialize(params json.RawMessage) (any, *rpcError) {
s.logClientProtocolVersion(params)

return map[string]any{
"protocolVersion": protocolVersion,
"capabilities": map[string]any{
Expand Down
Loading
Loading