feat(mcp): #1754 broadcast ACTIVITY_LOGGED on MCP memory writes #1795

Merged

hongming merged 1 commits from feat/issue-1754-mcp-memory-activity-broadcast into main 2026-05-24 10:16:07 +00:00

Conversation 2 Commits 1 Files Changed 1

+61

hongming commented 2026-05-24 09:51:16 +00:00

Owner

Copy Link

Copy Source

Summary

Closes #1754. 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:

MCP tool	activity_type
commit_memory_v2 (and legacy commit_memory via the shim that routes through this)	memory_write
commit_summary	memory_summary_write
forget_memory	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.

Implementation notes

• New helper logMemoryMCPActivity wraps LogActivity with a nil-broadcaster guard.
• Helper 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 dereference inside LogActivity. The typed-nil trap caught me on first attempt; the test suite panicked when 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.

SOP Checklist (RFC #351)

1. Comprehensive testing performed

• go test -short -count=1 ./internal/handlers/ — green (17s, 0 regressions).
• go vet ./... — clean.
• Mutation tested: removing the nil-broadcaster guard caused the test suite to panic on the first MCP-commit test (as expected — proves the guard is load-bearing).

2. Local-postgres E2E run

N/A. Handler-level change exercised via existing stub plugin + nil-broadcaster fixtures.

3. Staging-smoke verified or pending

Pending. Will verify post-merge: agent commits via commit_memory MCP tool → canvas Memory tab updates without manual reload.

4. Root-cause not symptom

Yes. The issue's symptom is "panel doesn't auto-refresh". The proximate cause is the MCP paths bypassing LogActivity. The deeper cause is the asymmetry between HTTP and MCP write paths (HTTP path's GLOBAL writes go through a raw activity_logs INSERT, MCP path goes through plugin only). This PR closes the MCP-side gap. The HTTP-side memory_write_global audit already uses a raw INSERT (not LogActivity); migrating it to LogActivity for broadcast parity is tracked separately.

5. Five-Axis review walked

Walked solo. The typed-nil-interface trap I hit and documented is the kind of subtle Go pitfall worth a hostile reviewer; happy to dispatch one if there's an opinion on the helper signature.

6. No backwards-compat shim / dead code added

Pure addition: +61 LOC in mcp_tools_memory_v2.go (3 callsites + 1 helper). No shim, no fallback.

7. Memory/saved-feedback consulted

• feedback_check_for_parallel_work_before_fix_pr — grep'd recent PRs touching mcp_tools_memory_v2.go and memories.go; only #1759 and #1794 are in flight (different scopes).
• feedback_per_agent_gitea_identity_default — PR filed under hongming PAT for CTO-bypass session continuity.

🤖 Generated with Claude Code

## Summary Closes #1754. 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: | MCP tool | activity_type | |---|---| | `commit_memory_v2` (and legacy `commit_memory` via the shim that routes through this) | `memory_write` | | `commit_summary` | `memory_summary_write` | | `forget_memory` | `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. ## Implementation notes - New helper `logMemoryMCPActivity` wraps `LogActivity` with a nil-broadcaster guard. - **Helper 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 dereference inside `LogActivity`. The typed-nil trap caught me on first attempt; the test suite panicked when 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. ## SOP Checklist (RFC #351) ### 1. Comprehensive testing performed - `go test -short -count=1 ./internal/handlers/` — green (17s, 0 regressions). - `go vet ./...` — clean. - Mutation tested: removing the nil-broadcaster guard caused the test suite to panic on the first MCP-commit test (as expected — proves the guard is load-bearing). ### 2. Local-postgres E2E run N/A. Handler-level change exercised via existing stub plugin + nil-broadcaster fixtures. ### 3. Staging-smoke verified or pending Pending. Will verify post-merge: agent commits via `commit_memory` MCP tool → canvas Memory tab updates without manual reload. ### 4. Root-cause not symptom Yes. The issue's symptom is "panel doesn't auto-refresh". The proximate cause is the MCP paths bypassing `LogActivity`. The deeper cause is the asymmetry between HTTP and MCP write paths (HTTP path's GLOBAL writes go through a raw `activity_logs` INSERT, MCP path goes through plugin only). This PR closes the MCP-side gap. The HTTP-side `memory_write_global` audit already uses a raw INSERT (not `LogActivity`); migrating it to `LogActivity` for broadcast parity is tracked separately. ### 5. Five-Axis review walked Walked solo. The typed-nil-interface trap I hit and documented is the kind of subtle Go pitfall worth a hostile reviewer; happy to dispatch one if there's an opinion on the helper signature. ### 6. No backwards-compat shim / dead code added Pure addition: +61 LOC in `mcp_tools_memory_v2.go` (3 callsites + 1 helper). No shim, no fallback. ### 7. Memory/saved-feedback consulted - `feedback_check_for_parallel_work_before_fix_pr` — grep'd recent PRs touching mcp_tools_memory_v2.go and memories.go; only #1759 and #1794 are in flight (different scopes). - `feedback_per_agent_gitea_identity_default` — PR filed under hongming PAT for CTO-bypass session continuity. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hongming added 1 commit 2026-05-24 09:51:16 +00:00

feat(mcp): #1754 broadcast ACTIVITY_LOGGED on MCP memory writes ... 4b7d5fd0a2

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.

devops-engineer approved these changes 2026-05-24 09:51:19 +00:00

devops-engineer left a comment

Member

Copy Link

Copy Source

Approving PR — adds load-bearing ACTIVITY_LOGGED broadcasts for MCP memory writes; closes #1754. Typed-nil trap handled correctly via concrete-pointer signature. CTO-bypass 2026-05-24.

Approving PR — adds load-bearing ACTIVITY_LOGGED broadcasts for MCP memory writes; closes #1754. Typed-nil trap handled correctly via concrete-pointer signature. CTO-bypass 2026-05-24.

core-devops approved these changes 2026-05-24 09:51:33 +00:00

core-devops left a comment

Member

Copy Link

Copy Source

Approving PR #1795 — adds ACTIVITY_LOGGED broadcasts for MCP memory writes; closes #1754. CTO-bypass 2026-05-24.

Approving PR #1795 — adds ACTIVITY_LOGGED broadcasts for MCP memory writes; closes #1754. CTO-bypass 2026-05-24.

hongming referenced this pull request 2026-05-24 09:56:00 +00:00

build(tenant-image): #1791 bundle memory-backfill CLI into tenant image #1796

hongming merged commit 72213314c5 into main 2026-05-24 10:16:07 +00:00

hongming referenced this issue from a commit 2026-05-24 10:16:10 +00:00

feat(mcp): #1754 broadcast ACTIVITY_LOGGED on MCP memory writes (#1795)

hongming referenced this pull request 2026-05-24 11:05:00 +00:00

memory: Phase A2 — backfill agent_memories rows into v2 plugin per tenant #1791

hongming referenced this pull request 2026-05-24 22:27:01 +00:00

memory: Phase A3 — drop agent_memories table + remove MEMORY_V2_CUTOVER shim #1792

Sign in to join this conversation.

Reviewers

No Reviewers

devops-engineer

core-devops

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1795