From 4b7d5fd0a2d10e9f148cf9c835e52f36d00d2353 Mon Sep 17 00:00:00 2001 From: hongming Date: Sun, 24 May 2026 02:50:13 -0700 Subject: [PATCH] feat(mcp): #1754 broadcast ACTIVITY_LOGGED on MCP memory writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canvas Memory tab (MemoryInspectorPanel.tsx, post-#1749) subscribes to ACTIVITY_LOGGED WS events filtered to memory-write activity_types so it can live-refresh when an agent commits a memory. But the MCP tool paths bypass LogActivity, so the panel only refreshes via manual reload when an agent commits via commit_memory or commit_memory_v2. Fix adds ACTIVITY_LOGGED broadcasts in the three MCP write tools: - toolCommitMemoryV2 → activity_type='memory_write' - toolCommitSummary → activity_type='memory_summary_write' - toolForgetMemory → activity_type='memory_delete' Reads (search_memory / recall_memory) intentionally do NOT broadcast — too noisy and not useful for live UI refresh, matching the issue's proposed scope. The legacy commit_memory shim is covered transitively: it calls toolCommitMemoryV2 internally so the broadcast fires once at the v2-routed write point. Implementation notes: - New helper logMemoryMCPActivity wraps LogActivity with a nil-broadcaster guard. Takes *events.Broadcaster (concrete) NOT events.EventEmitter (interface): passing a nil concrete pointer through an interface parameter creates a typed-nil interface that fails to compare equal to nil — and would derefence inside LogActivity. The typed-nil trap caught me on first attempt; the test suite panicked when test fixtures passed nil. The concrete-type signature makes the nil-check reliable. - Fire-and-forget: broadcast failure must never roll back a successful plugin write. LogActivity already logs+swallows INSERT errors. - target_id carries the affected memory id so the canvas can do optimistic add/remove without a full refetch. Closes #1754. --- .../internal/handlers/mcp_tools_memory_v2.go | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/workspace-server/internal/handlers/mcp_tools_memory_v2.go b/workspace-server/internal/handlers/mcp_tools_memory_v2.go index 00c991523..f076d5171 100644 --- a/workspace-server/internal/handlers/mcp_tools_memory_v2.go +++ b/workspace-server/internal/handlers/mcp_tools_memory_v2.go @@ -31,6 +31,7 @@ import ( "strings" "time" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/events" "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/client" "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract" "github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/namespace" @@ -154,6 +155,14 @@ func (h *MCPHandler) toolCommitMemoryV2(ctx context.Context, workspaceID string, } } + // Broadcast ACTIVITY_LOGGED so the canvas Memory tab live-refreshes + // (issue #1754). Fire-and-forget — broadcast failure must not roll + // back a successful plugin write. Source=agent because this is the + // MCP path (vs canvas user-driven POST /memories which the legacy + // handler logs with `memory_write_global`). + summary := "commit_memory to " + ns + logMemoryMCPActivity(ctx, h.broadcaster, workspaceID, "memory_write", resp.ID, ns, &summary) + out, _ := json.Marshal(resp) return string(out), nil } @@ -258,6 +267,11 @@ func (h *MCPHandler) toolCommitSummary(ctx context.Context, workspaceID string, if err != nil { return "", fmt.Errorf("plugin commit: %w", err) } + + // Broadcast ACTIVITY_LOGGED for live canvas refresh (#1754). + summary := "commit_summary to " + ns + logMemoryMCPActivity(ctx, h.broadcaster, workspaceID, "memory_summary_write", resp.ID, ns, &summary) + out, _ := json.Marshal(resp) return string(out), nil } @@ -320,6 +334,13 @@ func (h *MCPHandler) toolForgetMemory(ctx context.Context, workspaceID string, a }); err != nil { return "", fmt.Errorf("plugin forget: %w", err) } + + // Broadcast ACTIVITY_LOGGED for live canvas refresh (#1754). + // On forget, target_id carries the forgotten memory's id so the + // canvas can remove the row optimistically. + summary := "forget_memory " + memID + " from " + ns + logMemoryMCPActivity(ctx, h.broadcaster, workspaceID, "memory_delete", memID, ns, &summary) + return `{"forgotten":true}`, nil } @@ -393,3 +414,43 @@ func pickStringSlice(args map[string]interface{}, key string) []string { } return nil } + +// logMemoryMCPActivity fires the ACTIVITY_LOGGED broadcast for MCP-tool +// memory writes/deletes (issue #1754). The canvas Memory tab subscribes +// to memory_* activity_types via the WS feed; without this call, the +// panel only refreshes when the user hits the manual reload button. +// +// Fire-and-forget: broadcast failure must never roll back a successful +// plugin write. The underlying LogActivity helper already logs insert +// errors and swallows them — wrap with a nil-broadcaster guard so test +// fixtures without an events.Broadcaster don't nil-deref. +// +// IMPORTANT: takes *events.Broadcaster (concrete pointer), not +// events.EventEmitter (interface). Passing a nil *Broadcaster as +// EventEmitter creates a typed-nil interface that does NOT compare +// equal to nil — and would derefence inside LogActivity. Keeping the +// concrete type here lets the nil-check work reliably. +// +// Args: +// - activityType: "memory_write" / "memory_summary_write" / "memory_delete" +// - targetID: the affected memory's id (the canvas uses this for +// optimistic add/remove without a full refetch) +// - namespace: the v2 namespace the row landed in (canvas filters +// its display by readable-namespace intersection) +func logMemoryMCPActivity(ctx context.Context, broadcaster *events.Broadcaster, workspaceID, activityType, targetID, namespace string, summary *string) { + if broadcaster == nil { + return + } + source := workspaceID + target := targetID + method := namespace + LogActivity(ctx, broadcaster, ActivityParams{ + WorkspaceID: workspaceID, + ActivityType: activityType, + SourceID: &source, + TargetID: &target, + Method: &method, + Summary: summary, + Status: "ok", + }) +} -- 2.52.0