diff --git a/canvas/src/components/MemoryInspectorPanel.tsx b/canvas/src/components/MemoryInspectorPanel.tsx index 6655ad37a..7d8775be1 100644 --- a/canvas/src/components/MemoryInspectorPanel.tsx +++ b/canvas/src/components/MemoryInspectorPanel.tsx @@ -24,9 +24,10 @@ * "no memories yet". */ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { api } from '@/lib/api'; import { ConfirmDialog } from '@/components/ConfirmDialog'; +import { useSocketEvent } from '@/hooks/useSocketEvent'; // ── Types ───────────────────────────────────────────────────────────────────── @@ -246,6 +247,60 @@ export function MemoryInspectorPanel({ workspaceId }: Props) { loadEntries(); }, [loadEntries]); + // Live-refresh on ACTIVITY_LOGGED events that look like memory writes + // for this workspace (#1734). Without this, the user sees a stale + // empty state after an agent commits — agent says "wrote memory", + // panel keeps showing nothing until they hit Refresh. + // + // What actually broadcasts ACTIVITY_LOGGED on the server today + // (workspace-server/internal/handlers/activity.go LogActivity / + // LogActivityTx — those are the only emitters): + // + // - `memory_write_global` — `POST /workspaces/:id/memories` for GLOBAL scope + // - `memory_edit_global` — `PATCH /workspaces/:id/memories/:id` for GLOBAL scope + // - `memory_delete_global` — `DELETE /workspaces/:id/memories/:id` for GLOBAL scope + // - `agent_log` — generic catch-all an agent emits via + // `POST /workspaces/:id/activity` + // + // The MCP-tool path (`commit_memory`, `commit_memory_v2`, + // `commit_summary`) does NOT broadcast on the wire today; it inserts + // into agent_memories (pre-A1) or calls the v2 plugin (post-A1) and + // never round-trips through LogActivity. Server-side follow-up is + // tracked in **#1754** — once the MCP handlers emit `memory_write` + // via LogActivity, the `agent_log` arm of the filter below can be + // dropped. `memory_write` is included pre-emptively so this code + // lights up the moment #1754 lands. Until then, `agent_log` catches + // MCP commits over-inclusively; the 300ms debounce bounds the + // refetch rate. Issue #1734 review finding. + // + // The 300ms debounce coalesces bursts so a chatty agent (e.g. an + // agent in a long task emitting agent_log every few hundred ms) + // doesn't hammer /v2/memories on every keystroke-equivalent. + const refetchTimerRef = useRef | null>(null); + useEffect(() => () => { + if (refetchTimerRef.current) clearTimeout(refetchTimerRef.current); + }, []); + useSocketEvent((msg) => { + if (msg.event !== 'ACTIVITY_LOGGED') return; + if (msg.workspace_id !== workspaceId) return; + const p = (msg.payload || {}) as Record; + const activityType = (p.activity_type as string) || ''; + switch (activityType) { + case 'memory_write': + case 'memory_write_global': + case 'memory_edit_global': + case 'memory_delete_global': + case 'agent_log': + break; + default: + return; + } + if (refetchTimerRef.current) clearTimeout(refetchTimerRef.current); + refetchTimerRef.current = setTimeout(() => { + loadEntries(); + }, 300); + }); + // ── Delete handlers ───────────────────────────────────────────────────────── const confirmDelete = useCallback(async () => { diff --git a/canvas/src/components/__tests__/EmptyState.test.tsx b/canvas/src/components/__tests__/EmptyState.test.tsx index fa042f390..10d0ebdb0 100644 --- a/canvas/src/components/__tests__/EmptyState.test.tsx +++ b/canvas/src/components/__tests__/EmptyState.test.tsx @@ -16,7 +16,7 @@ * - handleDeployed fires after 500ms delay * * Uses vi.hoisted + vi.mock to fully isolate the api module, matching - * the pattern established in ApprovalBanner, MemoryTab, and ScheduleTab tests. + * the pattern established in ApprovalBanner and ScheduleTab tests. */ import React from "react"; import { render, screen, fireEvent, cleanup, act } from "@testing-library/react"; diff --git a/canvas/src/components/__tests__/MemoryInspectorPanel.test.ts b/canvas/src/components/__tests__/MemoryInspectorPanel.test.ts deleted file mode 100644 index 797f27ac0..000000000 --- a/canvas/src/components/__tests__/MemoryInspectorPanel.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -// @vitest-environment jsdom -/** - * Unit tests for pure helpers from MemoryInspectorPanel: - * isPluginUnavailableError, formatRelativeTime, formatTTL - * - * These are the three exported non-component functions. The component - * itself (MemoryInspectorPanel) requires full API + store mocking and - * is exercised by the existing MemoryTab.test.tsx. - */ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { isPluginUnavailableError, formatTTL } from "../MemoryInspectorPanel"; - -// formatRelativeTime is not exported — tested via the component in MemoryTab.test.tsx - -describe("isPluginUnavailableError", () => { - it("returns true when Error message contains MEMORY_PLUGIN_URL", () => { - const err = new Error("memory: could not resolve MEMORY_PLUGIN_URL — plugin not configured"); - expect(isPluginUnavailableError(err)).toBe(true); - }); - - it("returns true for Error containing MEMORY_PLUGIN_URL", () => { - expect(isPluginUnavailableError(new Error("MEMORY_PLUGIN_URL is not set"))).toBe(true); - }); - - it("returns false for unrelated error messages", () => { - expect(isPluginUnavailableError(new Error("workspace not found"))).toBe(false); - }); - - it("returns false for null", () => { - expect(isPluginUnavailableError(null)).toBe(false); - }); - - it("returns false for undefined", () => { - expect(isPluginUnavailableError(undefined)).toBe(false); - }); - - it("returns false for plain objects without message", () => { - expect(isPluginUnavailableError({ code: 503 })).toBe(false); - }); - - it("is case-sensitive (MEMORY_PLUGIN_URL must match exactly)", () => { - const lowerErr = new Error("memory_plugin_url missing"); - const upperErr = new Error("MEMORY_PLUGIN_URL missing"); - expect(isPluginUnavailableError(lowerErr)).toBe(false); - expect(isPluginUnavailableError(upperErr)).toBe(true); - }); -}); - -describe("formatTTL", () => { - beforeEach(() => { vi.useFakeTimers(); }); - afterEach(() => { vi.useRealTimers(); }); - - it("returns '' for null", () => { - expect(formatTTL(null)).toBe(""); - }); - - it("returns '' for undefined", () => { - expect(formatTTL(undefined)).toBe(""); - }); - - it('returns "expired" when expiresAt is in the past', () => { - const past = new Date(Date.now() - 60_000).toISOString(); - expect(formatTTL(past)).toBe("expired"); - }); - - it('returns "Xs" for less than a minute', () => { - const soon = new Date(Date.now() + 30_000).toISOString(); - expect(formatTTL(soon)).toBe("30s"); - }); - - it('returns "Xm" for less than an hour', () => { - const soon = new Date(Date.now() + 5 * 60_000).toISOString(); - expect(formatTTL(soon)).toBe("5m"); - }); - - it('returns "Xh" for less than a day', () => { - const soon = new Date(Date.now() + 3 * 3_600_000).toISOString(); - expect(formatTTL(soon)).toBe("3h"); - }); - - it('returns "Xd" for more than a day', () => { - const soon = new Date(Date.now() + 2 * 86_400_000).toISOString(); - expect(formatTTL(soon)).toBe("2d"); - }); - - it("returns '' for invalid date string", () => { - expect(formatTTL("not-a-date")).toBe(""); - }); - - it("returns '' for empty string", () => { - expect(formatTTL("")).toBe(""); - }); -}); diff --git a/canvas/src/components/__tests__/MemoryInspectorPanel.test.tsx b/canvas/src/components/__tests__/MemoryInspectorPanel.test.tsx index 0111451f9..81b69afdd 100644 --- a/canvas/src/components/__tests__/MemoryInspectorPanel.test.tsx +++ b/canvas/src/components/__tests__/MemoryInspectorPanel.test.tsx @@ -31,6 +31,17 @@ vi.mock('@/lib/api', () => ({ }, })); +// Capture the socket-event handler the panel registers so individual +// tests can replay an ACTIVITY_LOGGED message without spinning up a +// real WebSocket. One handler at a time is fine — the panel mounts +// exactly one useSocketEvent subscriber. +let __socketHandler: ((msg: unknown) => void) | null = null; +vi.mock('@/hooks/useSocketEvent', () => ({ + useSocketEvent: (handler: (msg: unknown) => void) => { + __socketHandler = handler; + }, +})); + vi.mock('@/components/ConfirmDialog', () => ({ ConfirmDialog: ({ open, @@ -516,3 +527,156 @@ describe('MemoryInspectorPanel — refresh', () => { }); }); }); + +// Live-refresh subscription wired in #1734 so the panel reacts to +// ACTIVITY_LOGGED events for memory writes on this workspace without +// the user clicking Refresh. The hook is mocked at the top of the +// file to capture the registered handler in __socketHandler. +describe('MemoryInspectorPanel — live refresh on activity', () => { + it('refetches memories when ACTIVITY_LOGGED arrives with activity_type=memory_write for the same workspace', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + stubFetch([MEM_BASIC]); + render(); + await waitFor(() => screen.getByLabelText('Refresh memories')); + expect(__socketHandler).toBeTruthy(); + + const before = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + + __socketHandler!({ + event: 'ACTIVITY_LOGGED', + workspace_id: 'ws-1', + payload: { activity_type: 'memory_write' }, + }); + + // 300ms debounce inside the panel — advance the fake timer so the + // queued refetch fires. + await vi.advanceTimersByTimeAsync(350); + + await waitFor(() => { + const after = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + expect(after).toBe(before + 1); + }); + vi.useRealTimers(); + }); + + it('ignores ACTIVITY_LOGGED events from other workspaces', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + stubFetch([MEM_BASIC]); + render(); + await waitFor(() => screen.getByLabelText('Refresh memories')); + + const before = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + + __socketHandler!({ + event: 'ACTIVITY_LOGGED', + workspace_id: 'ws-OTHER', + payload: { activity_type: 'memory_write' }, + }); + + await vi.advanceTimersByTimeAsync(500); + + const after = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + expect(after).toBe(before); + vi.useRealTimers(); + }); + + it('ignores activity types that are not memory-related', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + stubFetch([MEM_BASIC]); + render(); + await waitFor(() => screen.getByLabelText('Refresh memories')); + + const before = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + + __socketHandler!({ + event: 'ACTIVITY_LOGGED', + workspace_id: 'ws-1', + payload: { activity_type: 'a2a_send' }, + }); + + await vi.advanceTimersByTimeAsync(500); + + const after = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + expect(after).toBe(before); + vi.useRealTimers(); + }); + + // Server-side emitters confirmed via grep of workspace-server/internal/handlers + // are `memory_write_global`, `memory_edit_global`, `memory_delete_global` + // (memories.go `LogActivity` calls for GLOBAL-scope writes). Pin each + // so a future filter narrow-down can't silently drop one and let the + // panel go stale on its actual production trigger. + it.each([ + 'memory_write', // pre-emptive: not yet emitted by server, see component comment + 'memory_write_global', // memories.go:218 (Commit) + 'memory_edit_global', // memories.go:617 (Update) + 'memory_delete_global', // memories.go (Delete) — paired with the above two + 'agent_log', // generic catch-all + ])('refetches on activity_type=%s', async (activityType) => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + stubFetch([MEM_BASIC]); + render(); + await waitFor(() => screen.getByLabelText('Refresh memories')); + + const before = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + + __socketHandler!({ + event: 'ACTIVITY_LOGGED', + workspace_id: 'ws-1', + payload: { activity_type: activityType }, + }); + + await vi.advanceTimersByTimeAsync(350); + + await waitFor(() => { + const after = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + expect(after).toBe(before + 1); + }); + vi.useRealTimers(); + }); + + it('coalesces a burst of memory_write events into one refetch', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + stubFetch([MEM_BASIC]); + render(); + await waitFor(() => screen.getByLabelText('Refresh memories')); + + const before = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + + for (let i = 0; i < 5; i++) { + __socketHandler!({ + event: 'ACTIVITY_LOGGED', + workspace_id: 'ws-1', + payload: { activity_type: 'memory_write' }, + }); + } + + await vi.advanceTimersByTimeAsync(350); + + await waitFor(() => { + const after = mockGet.mock.calls.filter((c) => + (c[0] as string).includes('/v2/memories'), + ).length; + expect(after).toBe(before + 1); + }); + vi.useRealTimers(); + }); +}); diff --git a/canvas/src/components/tabs/ChannelsTab.tsx b/canvas/src/components/tabs/ChannelsTab.tsx index 9ab79b4cc..3d6e8acdc 100644 --- a/canvas/src/components/tabs/ChannelsTab.tsx +++ b/canvas/src/components/tabs/ChannelsTab.tsx @@ -369,7 +369,7 @@ export function ChannelsTab({ workspaceId }: Props) { onClick={handleCreate} // Was bg-accent-strong hover:bg-accent — accent is the // LIGHTER variant; same AA contrast trap fixed in - // ScheduleTab/MemoryTab/OnboardingWizard. + // ScheduleTab/OnboardingWizard. className="w-full text-xs py-1.5 rounded bg-accent hover:bg-accent-strong text-white transition focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-2 focus-visible:ring-offset-surface" > Connect Channel diff --git a/canvas/src/components/tabs/MemoryTab.tsx b/canvas/src/components/tabs/MemoryTab.tsx deleted file mode 100644 index 8e5608019..000000000 --- a/canvas/src/components/tabs/MemoryTab.tsx +++ /dev/null @@ -1,471 +0,0 @@ -"use client"; - -import { useCallback, useEffect, useMemo, useState } from "react"; -import { api } from "@/lib/api"; - -interface Props { - workspaceId: string; -} - -interface MemoryEntry { - key: string; - value: unknown; - version?: number; - expires_at: string | null; - updated_at: string; -} - -const AWARENESS_BASE_URL = - process.env.NEXT_PUBLIC_AWARENESS_URL || "http://localhost:37800"; - -export function MemoryTab({ workspaceId }: Props) { - const [entries, setEntries] = useState([]); - const [loading, setLoading] = useState(true); - const [showAwareness, setShowAwareness] = useState(true); - const [showAdvanced, setShowAdvanced] = useState(false); - const [expanded, setExpanded] = useState(null); - const [showAdd, setShowAdd] = useState(false); - const [newKey, setNewKey] = useState(""); - const [newValue, setNewValue] = useState(""); - const [newTTL, setNewTTL] = useState(""); - const [error, setError] = useState(null); - const [editingKey, setEditingKey] = useState(null); - const [editValue, setEditValue] = useState(""); - const [editTTL, setEditTTL] = useState(""); - const [editError, setEditError] = useState(null); - - const awarenessUrl = useMemo(() => { - try { - const url = new URL(AWARENESS_BASE_URL); - url.searchParams.set("workspaceId", workspaceId); - return url.toString(); - } catch { - return AWARENESS_BASE_URL; - } - }, [workspaceId]); - - const awarenessStatus = useMemo(() => { - try { - const url = new URL(AWARENESS_BASE_URL); - return url.origin.includes("localhost") ? "local" : url.hostname; - } catch { - return "unavailable"; - } - }, []); - - const loadMemory = useCallback(async () => { - setLoading(true); - setError(null); - try { - const data = await api.get(`/workspaces/${workspaceId}/memory`); - setEntries(data); - } catch (e) { - setEntries([]); - setError(e instanceof Error ? e.message : "Failed to load memory"); - } finally { - setLoading(false); - } - }, [workspaceId]); - - useEffect(() => { - loadMemory(); - }, [loadMemory]); - - const handleAdd = async () => { - setError(null); - if (!newKey.trim()) { - setError("Key is required"); - return; - } - - let parsedValue: unknown; - try { - parsedValue = JSON.parse(newValue); - } catch { - parsedValue = newValue; - } - - const body: Record = { key: newKey, value: parsedValue }; - if (newTTL) { - const ttl = parseInt(newTTL); - if (!Number.isNaN(ttl) && ttl > 0) body.ttl_seconds = ttl; - } - - try { - await api.post(`/workspaces/${workspaceId}/memory`, body); - setNewKey(""); - setNewValue(""); - setNewTTL(""); - setShowAdd(false); - loadMemory(); - } catch (e) { - setError(e instanceof Error ? e.message : "Failed to add"); - } - }; - - const handleDelete = async (key: string) => { - setError(null); - try { - await api.del(`/workspaces/${workspaceId}/memory/${encodeURIComponent(key)}`); - setEntries((prev) => prev.filter((e) => e.key !== key)); - if (expanded === key) setExpanded(null); - } catch (e) { - setError(e instanceof Error ? e.message : "Failed to delete entry"); - } - }; - - const beginEdit = (entry: MemoryEntry) => { - setEditError(null); - setEditingKey(entry.key); - // Stringify objects/arrays as pretty JSON; render plain strings raw so the - // editor doesn't surprise users with surrounding quotes. - setEditValue( - typeof entry.value === "string" - ? entry.value - : JSON.stringify(entry.value, null, 2), - ); - if (entry.expires_at) { - const remainingMs = new Date(entry.expires_at).getTime() - Date.now(); - const ttl = Math.max(0, Math.floor(remainingMs / 1000)); - setEditTTL(ttl > 0 ? String(ttl) : ""); - } else { - setEditTTL(""); - } - }; - - const cancelEdit = () => { - setEditingKey(null); - setEditValue(""); - setEditTTL(""); - setEditError(null); - }; - - const handleEditSave = async (entry: MemoryEntry) => { - setEditError(null); - - let parsedValue: unknown; - try { - parsedValue = JSON.parse(editValue); - } catch { - parsedValue = editValue; - } - - // if_match_version closes the silent-overwrite hole when two writers - // race. The handler returns 409 with the current version on mismatch - // — surface that as a retry hint and reload to pick up the new state. - const body: Record = { key: entry.key, value: parsedValue }; - if (typeof entry.version === "number") { - body.if_match_version = entry.version; - } - if (editTTL) { - const ttl = parseInt(editTTL); - if (!Number.isNaN(ttl) && ttl > 0) body.ttl_seconds = ttl; - } - - try { - await api.post(`/workspaces/${workspaceId}/memory`, body); - cancelEdit(); - loadMemory(); - } catch (e) { - const message = e instanceof Error ? e.message : "Failed to save"; - if (message.includes("409") || /if_match_version mismatch/i.test(message)) { - setEditError("This entry changed since you opened it. Reloading."); - loadMemory(); - } else { - setEditError(message); - } - } - }; - - const openAwareness = () => { - window.open(awarenessUrl, "_blank", "noopener,noreferrer"); - }; - - if (loading) { - return
Loading memory...
; - } - - return ( -
- {error && !showAdd && ( -
- {error} -
- )} - -
-
-
-
Awareness dashboard
-

- Embedded view for the local Awareness memory UI. The current workspace id is appended to the URL for workspace-scoped routing or future filtering. -

-
-
- - -
-
- - {showAwareness ? ( - AWARENESS_BASE_URL ? ( -
-