Merge pull request #2963 from Molecule-AI/staging
staging → main: auto-promote 7ee696e
This commit is contained in:
molecule-ai[bot]
GitHub
commit
a869bc1536
@ -1,261 +0,0 @@
|
||||
'use client';
|
||||
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { api } from "@/lib/api";
|
||||
import type { MemoryEntry } from "@/components/MemoryInspectorPanel";
|
||||
|
||||
type Scope = "LOCAL" | "TEAM" | "GLOBAL";
|
||||
const SCOPES: Scope[] = ["LOCAL", "TEAM", "GLOBAL"];
|
||||
|
||||
interface AddProps {
|
||||
open: boolean;
|
||||
mode: "add";
|
||||
workspaceId: string;
|
||||
defaultScope: Scope;
|
||||
defaultNamespace?: string;
|
||||
entry?: undefined;
|
||||
onClose: () => void;
|
||||
onSaved: () => void;
|
||||
}
|
||||
|
||||
interface EditProps {
|
||||
open: boolean;
|
||||
mode: "edit";
|
||||
workspaceId: string;
|
||||
entry: MemoryEntry;
|
||||
defaultScope?: undefined;
|
||||
defaultNamespace?: undefined;
|
||||
onClose: () => void;
|
||||
onSaved: () => void;
|
||||
}
|
||||
|
||||
type Props = AddProps | EditProps;
|
||||
|
||||
export function MemoryEditorDialog(props: Props) {
|
||||
const { open, mode, workspaceId, onClose, onSaved } = props;
|
||||
const dialogRef = useRef<HTMLDivElement>(null);
|
||||
const [mounted, setMounted] = useState(false);
|
||||
const [scope, setScope] = useState<Scope>("LOCAL");
|
||||
const [namespace, setNamespace] = useState("general");
|
||||
const [content, setContent] = useState("");
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
setMounted(true);
|
||||
}, []);
|
||||
|
||||
// Reset form whenever the dialog opens.
|
||||
useEffect(() => {
|
||||
if (!open) return;
|
||||
setError(null);
|
||||
setSaving(false);
|
||||
if (mode === "edit" && props.entry) {
|
||||
setScope(props.entry.scope);
|
||||
setNamespace(props.entry.namespace || "general");
|
||||
setContent(props.entry.content);
|
||||
} else if (mode === "add") {
|
||||
setScope(props.defaultScope);
|
||||
setNamespace(props.defaultNamespace || "general");
|
||||
setContent("");
|
||||
}
|
||||
// mode/props are stable per-open; intentional shallow deps.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [open]);
|
||||
|
||||
// Move focus into the dialog when it opens (WCAG SC 2.4.3).
|
||||
useEffect(() => {
|
||||
if (!open || !mounted) return;
|
||||
const raf = requestAnimationFrame(() => {
|
||||
dialogRef.current?.querySelector<HTMLElement>("textarea, input, select")?.focus();
|
||||
});
|
||||
return () => cancelAnimationFrame(raf);
|
||||
}, [open, mounted]);
|
||||
|
||||
// Escape closes; Cmd/Ctrl-Enter saves.
|
||||
const onCloseRef = useRef(onClose);
|
||||
onCloseRef.current = onClose;
|
||||
const handleSaveRef = useRef<() => void>(() => {});
|
||||
useEffect(() => {
|
||||
if (!open) return;
|
||||
const handler = (e: KeyboardEvent) => {
|
||||
if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
onCloseRef.current();
|
||||
} else if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) {
|
||||
e.preventDefault();
|
||||
handleSaveRef.current();
|
||||
}
|
||||
};
|
||||
window.addEventListener("keydown", handler);
|
||||
return () => window.removeEventListener("keydown", handler);
|
||||
}, [open]);
|
||||
|
||||
const handleSave = async () => {
|
||||
if (saving) return;
|
||||
const trimmed = content.trim();
|
||||
if (!trimmed) {
|
||||
setError("Content cannot be empty");
|
||||
return;
|
||||
}
|
||||
setError(null);
|
||||
setSaving(true);
|
||||
try {
|
||||
if (mode === "add") {
|
||||
await api.post(`/workspaces/${workspaceId}/memories`, {
|
||||
content: trimmed,
|
||||
scope,
|
||||
namespace: namespace.trim() || "general",
|
||||
});
|
||||
} else {
|
||||
// PATCH only sends fields that changed. Content always changeable;
|
||||
// namespace only sent if it differs from the original (saves a
|
||||
// no-op write through redactSecrets + re-embed).
|
||||
const original = props.entry;
|
||||
const body: Record<string, string> = {};
|
||||
if (trimmed !== original.content) body.content = trimmed;
|
||||
const ns = namespace.trim() || "general";
|
||||
if (ns !== original.namespace) body.namespace = ns;
|
||||
if (Object.keys(body).length === 0) {
|
||||
// No-op edit — close without an HTTP round-trip.
|
||||
onSaved();
|
||||
onClose();
|
||||
return;
|
||||
}
|
||||
await api.patch(
|
||||
`/workspaces/${workspaceId}/memories/${encodeURIComponent(original.id)}`,
|
||||
body,
|
||||
);
|
||||
}
|
||||
onSaved();
|
||||
onClose();
|
||||
} catch (e) {
|
||||
setError(e instanceof Error ? e.message : "Save failed");
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
};
|
||||
handleSaveRef.current = handleSave;
|
||||
|
||||
if (!open || !mounted) return null;
|
||||
|
||||
const titleId = "memory-editor-title";
|
||||
const isEdit = mode === "edit";
|
||||
|
||||
return createPortal(
|
||||
<div className="fixed inset-0 z-[9999] flex items-center justify-center">
|
||||
<div className="absolute inset-0 bg-black/60 backdrop-blur-sm" onClick={onClose} />
|
||||
|
||||
<div
|
||||
ref={dialogRef}
|
||||
role="dialog"
|
||||
aria-modal="true"
|
||||
aria-labelledby={titleId}
|
||||
className="relative bg-surface-sunken border border-line rounded-xl shadow-2xl shadow-black/50 max-w-[480px] w-full mx-4 overflow-hidden"
|
||||
>
|
||||
<div className="px-5 py-4 space-y-3">
|
||||
<h3 id={titleId} className="text-sm font-semibold text-ink">
|
||||
{isEdit ? "Edit memory" : "Add memory"}
|
||||
</h3>
|
||||
|
||||
{/* Scope */}
|
||||
<div className="space-y-1">
|
||||
<label className="text-[10px] text-ink-soft block" htmlFor="memory-editor-scope">
|
||||
Scope
|
||||
</label>
|
||||
{isEdit ? (
|
||||
<div
|
||||
id="memory-editor-scope"
|
||||
className="text-[12px] font-mono text-ink-mid bg-surface rounded px-2 py-1.5 border border-line/50"
|
||||
title="Scope is fixed on edit. To move a memory across scopes, delete and re-create it."
|
||||
>
|
||||
{scope}
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex items-center gap-1" id="memory-editor-scope" role="radiogroup" aria-label="Scope">
|
||||
{SCOPES.map((s) => (
|
||||
<button
|
||||
key={s}
|
||||
type="button"
|
||||
role="radio"
|
||||
aria-checked={scope === s}
|
||||
onClick={() => setScope(s)}
|
||||
className={[
|
||||
"px-3 py-1 text-[11px] rounded transition-colors",
|
||||
scope === s
|
||||
? "bg-accent-strong text-white"
|
||||
: "bg-surface-card text-ink-mid hover:text-ink",
|
||||
].join(" ")}
|
||||
>
|
||||
{s}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Namespace */}
|
||||
<div className="space-y-1">
|
||||
<label htmlFor="memory-editor-namespace" className="text-[10px] text-ink-soft block">
|
||||
Namespace
|
||||
</label>
|
||||
<input
|
||||
id="memory-editor-namespace"
|
||||
type="text"
|
||||
value={namespace}
|
||||
onChange={(e) => setNamespace(e.target.value)}
|
||||
placeholder="general"
|
||||
className="w-full bg-surface border border-line/60 focus:border-accent/60 rounded px-2 py-1.5 text-[12px] text-ink placeholder-zinc-600 focus:outline-none transition-colors"
|
||||
/>
|
||||
</div>
|
||||
|
||||
{/* Content */}
|
||||
<div className="space-y-1">
|
||||
<label htmlFor="memory-editor-content" className="text-[10px] text-ink-soft block">
|
||||
Content
|
||||
</label>
|
||||
<textarea
|
||||
id="memory-editor-content"
|
||||
value={content}
|
||||
onChange={(e) => setContent(e.target.value)}
|
||||
rows={6}
|
||||
placeholder="What should the agent remember?"
|
||||
className="w-full bg-surface border border-line/60 focus:border-accent/60 rounded px-2 py-1.5 text-[12px] font-mono text-ink placeholder-zinc-600 focus:outline-none transition-colors resize-y min-h-[100px] max-h-[300px]"
|
||||
/>
|
||||
</div>
|
||||
|
||||
{error && (
|
||||
<div
|
||||
role="alert"
|
||||
aria-live="assertive"
|
||||
className="px-2 py-1.5 bg-red-950/30 border border-red-800/40 rounded text-[11px] text-bad"
|
||||
>
|
||||
{error}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="flex items-center justify-end gap-2 px-5 py-3 border-t border-line bg-surface/50">
|
||||
<button
|
||||
type="button"
|
||||
onClick={onClose}
|
||||
disabled={saving}
|
||||
className="px-3.5 py-1.5 text-[13px] text-ink-mid hover:text-ink bg-surface-card hover:bg-surface-elevated border border-line hover:border-line-soft rounded-lg transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-accent/40 disabled:opacity-50 disabled:cursor-not-allowed"
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleSave}
|
||||
disabled={saving}
|
||||
className="px-3.5 py-1.5 text-[13px] rounded-lg transition-colors bg-accent hover:bg-accent-strong text-white focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-offset-surface-sunken focus-visible:ring-accent/60 disabled:opacity-50 disabled:cursor-not-allowed"
|
||||
>
|
||||
{saving ? "Saving…" : isEdit ? "Save changes" : "Add memory"}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>,
|
||||
document.body,
|
||||
);
|
||||
}
|
||||
@ -1,30 +1,81 @@
|
||||
'use client';
|
||||
|
||||
import { useState, useEffect, useCallback } from "react";
|
||||
import { api } from "@/lib/api";
|
||||
import { ConfirmDialog } from "@/components/ConfirmDialog";
|
||||
import { MemoryEditorDialog } from "@/components/MemoryEditorDialog";
|
||||
/**
|
||||
* MemoryInspectorPanel — Memory v2 redesign.
|
||||
*
|
||||
* Reads the canvas Memory tab from the v2 plugin via the
|
||||
* workspace-server proxy at /v2/{namespaces,memories}, replacing the
|
||||
* v1 LOCAL/TEAM/GLOBAL trio that mapped to the deprecated
|
||||
* shared_context model.
|
||||
*
|
||||
* Surface differences from v1:
|
||||
* - Namespace dropdown driven by GET /v2/namespaces (workspace /
|
||||
* team / org / custom — labels rendered server-side).
|
||||
* - Per-row badges for kind (fact|summary|checkpoint), source
|
||||
* (agent|runtime|user), pin (📌), TTL countdown, and propagation
|
||||
* source-workspace if the memory came from a peer.
|
||||
* - No Edit affordance — v2's plugin contract has no PATCH; the
|
||||
* model is forget + recommit. Delete (Forget) stays.
|
||||
*
|
||||
* Shipping note: when the plugin isn't wired (MEMORY_PLUGIN_URL
|
||||
* unset), every endpoint returns 503 with a clear hint. The panel
|
||||
* surfaces that as a banner so operators know to set the env var,
|
||||
* rather than rendering a perpetual empty state that looks like
|
||||
* "no memories yet".
|
||||
*/
|
||||
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react';
|
||||
import { api } from '@/lib/api';
|
||||
import { ConfirmDialog } from '@/components/ConfirmDialog';
|
||||
|
||||
// ── Types ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
/** Memory entry returned by GET /workspaces/:id/memories */
|
||||
export interface MemoryEntry {
|
||||
id: string;
|
||||
workspace_id: string;
|
||||
content: string;
|
||||
scope: "LOCAL" | "TEAM" | "GLOBAL";
|
||||
namespace: string;
|
||||
created_at: string;
|
||||
/**
|
||||
* Semantic similarity score (0–1). Only present when the API is queried
|
||||
* with ?q=<query> and the pgvector backend has been deployed.
|
||||
* Absent on plain list fetches — renders gracefully without a badge.
|
||||
*/
|
||||
similarity_score?: number;
|
||||
export type NamespaceKind = 'workspace' | 'team' | 'org' | 'custom';
|
||||
|
||||
export interface NamespaceView {
|
||||
name: string;
|
||||
kind: NamespaceKind;
|
||||
label: string;
|
||||
}
|
||||
|
||||
type Scope = "LOCAL" | "TEAM" | "GLOBAL";
|
||||
const SCOPES: Scope[] = ["LOCAL", "TEAM", "GLOBAL"];
|
||||
export interface NamespacesResponse {
|
||||
readable: NamespaceView[];
|
||||
writable: NamespaceView[];
|
||||
}
|
||||
|
||||
export type MemoryKind = 'fact' | 'summary' | 'checkpoint';
|
||||
export type MemorySource = 'agent' | 'runtime' | 'user';
|
||||
|
||||
export interface MemoryV2 {
|
||||
id: string;
|
||||
namespace: string;
|
||||
content: string;
|
||||
kind: MemoryKind;
|
||||
source: MemorySource;
|
||||
pin: boolean;
|
||||
expires_at?: string | null;
|
||||
created_at: string;
|
||||
/** 0..1 plugin similarity score; only present when ?q= is set. */
|
||||
score?: number | null;
|
||||
// Note: an earlier iteration of this type carried a `source_workspace_id`
|
||||
// field rendered as a "from peer" badge. The propagation contract that
|
||||
// would have populated it ("Reserved for future cross-namespace
|
||||
// propagation semantics" in memory-plugin-v1.yaml) is unimplemented —
|
||||
// nothing in the codebase writes that key. Removed in self-review.
|
||||
// Re-add when propagation gains a concrete shape.
|
||||
}
|
||||
|
||||
interface MemoriesResponse {
|
||||
memories: MemoryV2[];
|
||||
}
|
||||
|
||||
// MemoryEntry kept as a back-compat type alias so any other component
|
||||
// still importing it doesn't break the build. New consumers should
|
||||
// prefer MemoryV2 — the v1 shape (LOCAL/TEAM/GLOBAL scope) is gone.
|
||||
//
|
||||
// `unknown` is used over `any` so TS still flags accidental field
|
||||
// access on the legacy shape.
|
||||
export type MemoryEntry = MemoryV2;
|
||||
|
||||
interface Props {
|
||||
workspaceId: string;
|
||||
@ -32,11 +83,26 @@ interface Props {
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Sanitise a memory id for use in an HTML id attribute.
|
||||
*/
|
||||
function sanitizeId(id: string): string {
|
||||
return id.replace(/[^a-zA-Z0-9]/g, "-");
|
||||
return id.replace(/[^a-zA-Z0-9]/g, '-');
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect a memory-plugin-503 error from the api wrapper's stringified
|
||||
* Error message. Matches on the literal env-var name rather than the
|
||||
* status code, because the api shim renders status codes inside a
|
||||
* larger formatted message and a future status-code reformat would
|
||||
* silently break the detection.
|
||||
*
|
||||
* The substring `MEMORY_PLUGIN_URL` is hard-coded in the handler at
|
||||
* `workspace-server/internal/handlers/memories_v2.go:available()`,
|
||||
* so this is a pinned cross-layer contract — drift is caught by both
|
||||
* the Go test (TestMemoriesV2_PluginUnwired_All503) and the canvas
|
||||
* test (TestMemoryInspectorPanel — plugin unavailable).
|
||||
*/
|
||||
export function isPluginUnavailableError(err: unknown): boolean {
|
||||
const msg = err instanceof Error ? err.message : '';
|
||||
return msg.includes('MEMORY_PLUGIN_URL');
|
||||
}
|
||||
|
||||
function formatRelativeTime(iso: string): string {
|
||||
@ -47,6 +113,24 @@ function formatRelativeTime(iso: string): string {
|
||||
return new Date(iso).toLocaleDateString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Render a TTL countdown like "12h", "3d", or "expired" (when the
|
||||
* stored expires_at is in the past). Non-fatal if expires_at is null
|
||||
* or invalid — falls through to empty string so the badge doesn't
|
||||
* render.
|
||||
*/
|
||||
export function formatTTL(expiresAt: string | null | undefined): string {
|
||||
if (!expiresAt) return '';
|
||||
const ts = new Date(expiresAt).getTime();
|
||||
if (Number.isNaN(ts)) return '';
|
||||
const diff = ts - Date.now();
|
||||
if (diff <= 0) return 'expired';
|
||||
if (diff < 60_000) return `${Math.floor(diff / 1000)}s`;
|
||||
if (diff < 3_600_000) return `${Math.floor(diff / 60_000)}m`;
|
||||
if (diff < 86_400_000) return `${Math.floor(diff / 3_600_000)}h`;
|
||||
return `${Math.floor(diff / 86_400_000)}d`;
|
||||
}
|
||||
|
||||
// ── Skeleton rows ──────────────────────────────────────────────────────────────
|
||||
|
||||
function MemorySkeletonRows() {
|
||||
@ -71,63 +155,92 @@ function MemorySkeletonRows() {
|
||||
|
||||
// ── Component ─────────────────────────────────────────────────────────────────
|
||||
|
||||
const ALL_NAMESPACES = '__all__';
|
||||
|
||||
export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
const [activeScope, setActiveScope] = useState<Scope>("LOCAL");
|
||||
const [activeNamespace, setActiveNamespace] = useState("");
|
||||
const [entries, setEntries] = useState<MemoryEntry[]>([]);
|
||||
const [namespaces, setNamespaces] = useState<NamespacesResponse | null>(null);
|
||||
const [activeNamespace, setActiveNamespace] = useState<string>(ALL_NAMESPACES);
|
||||
const [entries, setEntries] = useState<MemoryV2[]>([]);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
// ── Search state (debounced) ────────────────────────────────────────────────
|
||||
const [searchQuery, setSearchQuery] = useState("");
|
||||
const [debouncedQuery, setDebouncedQuery] = useState("");
|
||||
// Plugin-disabled banner (503 from server). Stored separately so we
|
||||
// can keep showing the namespace dropdown empty rather than
|
||||
// hiding the whole panel.
|
||||
const [pluginUnavailable, setPluginUnavailable] = useState(false);
|
||||
|
||||
// Search state (debounced)
|
||||
const [searchQuery, setSearchQuery] = useState('');
|
||||
const [debouncedQuery, setDebouncedQuery] = useState('');
|
||||
|
||||
useEffect(() => {
|
||||
const timer = setTimeout(
|
||||
() => setDebouncedQuery(searchQuery.trim()),
|
||||
300
|
||||
);
|
||||
const timer = setTimeout(() => setDebouncedQuery(searchQuery.trim()), 300);
|
||||
return () => clearTimeout(timer);
|
||||
}, [searchQuery]);
|
||||
|
||||
// ── Delete state ─────────────────────────────────────────────────────────────
|
||||
// Delete state
|
||||
const [pendingDeleteId, setPendingDeleteId] = useState<string | null>(null);
|
||||
|
||||
// ── Editor state (Add + Edit share one modal) ───────────────────────────────
|
||||
type EditorState =
|
||||
| { mode: "add" }
|
||||
| { mode: "edit"; entry: MemoryEntry }
|
||||
| null;
|
||||
const [editorState, setEditorState] = useState<EditorState>(null);
|
||||
// ── Namespace loading ──────────────────────────────────────────────────────
|
||||
|
||||
// ── Data loading ────────────────────────────────────────────────────────────
|
||||
const loadNamespaces = useCallback(async () => {
|
||||
try {
|
||||
const data = await api.get<NamespacesResponse>(
|
||||
`/workspaces/${workspaceId}/v2/namespaces`,
|
||||
);
|
||||
setNamespaces(data);
|
||||
setPluginUnavailable(false);
|
||||
} catch (e) {
|
||||
// Plugin-unavailable (503) indicates MEMORY_PLUGIN_URL isn't set.
|
||||
// Anything else stays as a generic load failure that the
|
||||
// entries-load path will also flag.
|
||||
if (isPluginUnavailableError(e)) {
|
||||
setPluginUnavailable(true);
|
||||
}
|
||||
setNamespaces({ readable: [], writable: [] });
|
||||
}
|
||||
}, [workspaceId]);
|
||||
|
||||
// ── Entries loading ────────────────────────────────────────────────────────
|
||||
|
||||
const loadEntries = useCallback(async () => {
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
try {
|
||||
const params = new URLSearchParams();
|
||||
params.set("scope", activeScope);
|
||||
if (debouncedQuery) params.set("q", debouncedQuery);
|
||||
if (activeNamespace) params.set("namespace", activeNamespace);
|
||||
if (activeNamespace !== ALL_NAMESPACES) {
|
||||
params.set('namespace', activeNamespace);
|
||||
}
|
||||
if (debouncedQuery) params.set('q', debouncedQuery);
|
||||
|
||||
const url = `/workspaces/${workspaceId}/memories?${params.toString()}`;
|
||||
const data = await api.get<MemoryEntry[]>(url);
|
||||
const url = `/workspaces/${workspaceId}/v2/memories?${params.toString()}`;
|
||||
const data = await api.get<MemoriesResponse>(url);
|
||||
|
||||
// When a semantic query is active, sort by similarity_score descending.
|
||||
// When a semantic query is active and the plugin returns
|
||||
// scores, sort by score descending so the most-relevant hit
|
||||
// sits at the top. Empty score → push to bottom.
|
||||
const sorted = debouncedQuery
|
||||
? [...data].sort(
|
||||
(a, b) => (b.similarity_score ?? 0) - (a.similarity_score ?? 0)
|
||||
? [...data.memories].sort(
|
||||
(a, b) => (b.score ?? 0) - (a.score ?? 0),
|
||||
)
|
||||
: data;
|
||||
: data.memories;
|
||||
setEntries(sorted);
|
||||
} catch (e) {
|
||||
setError(e instanceof Error ? e.message : "Failed to load memories");
|
||||
if (isPluginUnavailableError(e)) {
|
||||
setPluginUnavailable(true);
|
||||
setError(null); // surfaced via banner, not row error
|
||||
} else {
|
||||
setError(e instanceof Error ? e.message : 'Failed to load memories');
|
||||
}
|
||||
setEntries([]);
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, [workspaceId, activeScope, debouncedQuery, activeNamespace]);
|
||||
}, [workspaceId, activeNamespace, debouncedQuery]);
|
||||
|
||||
useEffect(() => {
|
||||
loadNamespaces();
|
||||
}, [loadNamespaces]);
|
||||
|
||||
useEffect(() => {
|
||||
loadEntries();
|
||||
@ -144,16 +257,35 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
setEntries((prev) => prev.filter((e) => e.id !== id));
|
||||
|
||||
try {
|
||||
await api.del(`/workspaces/${workspaceId}/memories/${encodeURIComponent(id)}`);
|
||||
await api.del(`/workspaces/${workspaceId}/v2/memories/${encodeURIComponent(id)}`);
|
||||
} catch (e) {
|
||||
setError(e instanceof Error ? e.message : "Delete failed — reloading...");
|
||||
// Reload first (which clears any stale error), THEN set the
|
||||
// delete-failure message — otherwise loadEntries' own
|
||||
// `setError(null)` wipes our error before the user sees it.
|
||||
// Caught by the rollback test in MemoryInspectorPanel.test.tsx.
|
||||
const msg = e instanceof Error ? e.message : 'Delete failed — reloading…';
|
||||
await loadEntries();
|
||||
setError(msg);
|
||||
}
|
||||
}, [pendingDeleteId, workspaceId, loadEntries]);
|
||||
|
||||
// ── Namespace dropdown options ─────────────────────────────────────────────
|
||||
|
||||
const dropdownOptions = useMemo(() => {
|
||||
const opts: Array<{ value: string; label: string; kind?: NamespaceKind }> = [
|
||||
{ value: ALL_NAMESPACES, label: 'All namespaces' },
|
||||
];
|
||||
if (namespaces) {
|
||||
for (const ns of namespaces.readable) {
|
||||
opts.push({ value: ns.name, label: ns.label, kind: ns.kind });
|
||||
}
|
||||
}
|
||||
return opts;
|
||||
}, [namespaces]);
|
||||
|
||||
// ── Render ──────────────────────────────────────────────────────────────────
|
||||
|
||||
if (loading && entries.length === 0 && !error) {
|
||||
if (loading && entries.length === 0 && !error && !pluginUnavailable) {
|
||||
return (
|
||||
<div className="flex items-center justify-center h-32">
|
||||
<span className="text-xs text-ink-soft">Loading memories…</span>
|
||||
@ -163,32 +295,44 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
|
||||
return (
|
||||
<div className="flex flex-col h-full">
|
||||
{/* Scope tabs */}
|
||||
<div className="px-4 pt-3 pb-2 border-b border-line/40 shrink-0">
|
||||
<div className="flex items-center gap-1">
|
||||
{SCOPES.map((scope) => (
|
||||
<button
|
||||
type="button"
|
||||
key={scope}
|
||||
onClick={() => setActiveScope(scope)}
|
||||
aria-pressed={activeScope === scope}
|
||||
className={[
|
||||
"px-3 py-1 text-[11px] rounded transition-colors",
|
||||
activeScope === scope
|
||||
? "bg-accent-strong text-white"
|
||||
: "bg-surface-card text-ink-mid hover:bg-surface-card hover:text-ink",
|
||||
].join(" ")}
|
||||
{/* Plugin-unavailable banner */}
|
||||
{pluginUnavailable && (
|
||||
<div
|
||||
role="alert"
|
||||
aria-live="polite"
|
||||
className="mx-4 mt-3 px-3 py-2 bg-amber-950/30 border border-amber-800/40 rounded text-xs text-amber-300 shrink-0"
|
||||
data-testid="plugin-unavailable-banner"
|
||||
>
|
||||
{scope}
|
||||
</button>
|
||||
))}
|
||||
Memory plugin not configured. Set <code>MEMORY_PLUGIN_URL</code> on the
|
||||
workspace-server to enable v2 memory.
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Namespace dropdown */}
|
||||
<div className="px-4 pt-3 pb-2 border-b border-line/40 shrink-0 space-y-2">
|
||||
<div className="flex items-center gap-2">
|
||||
<label htmlFor="namespace-dropdown" className="text-[10px] text-ink-soft shrink-0">
|
||||
Namespace:
|
||||
</label>
|
||||
<select
|
||||
id="namespace-dropdown"
|
||||
value={activeNamespace}
|
||||
onChange={(e) => setActiveNamespace(e.target.value)}
|
||||
aria-label="Filter by namespace"
|
||||
disabled={pluginUnavailable}
|
||||
className="flex-1 bg-surface-sunken border border-line/60 focus:border-accent/60 rounded px-2 py-1 text-[11px] text-ink focus:outline-none transition-colors min-w-0 disabled:opacity-50 disabled:cursor-not-allowed"
|
||||
>
|
||||
{dropdownOptions.map((opt) => (
|
||||
<option key={opt.value} value={opt.value}>
|
||||
{opt.label}
|
||||
{opt.kind ? ` (${opt.kind})` : ''}
|
||||
</option>
|
||||
))}
|
||||
</select>
|
||||
</div>
|
||||
|
||||
{/* Search bar + namespace filter */}
|
||||
<div className="px-4 pt-3 pb-2 border-b border-line/40 shrink-0 space-y-2">
|
||||
{/* Search bar */}
|
||||
<div className="relative flex items-center">
|
||||
{/* Magnifying glass icon */}
|
||||
<svg
|
||||
width="12"
|
||||
height="12"
|
||||
@ -206,14 +350,15 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
onChange={(e) => setSearchQuery(e.target.value)}
|
||||
placeholder="Semantic search…"
|
||||
aria-label="Search memories"
|
||||
className="w-full bg-surface-sunken border border-line/60 focus:border-accent/60 rounded-lg pl-8 pr-7 py-1.5 text-[11px] text-ink placeholder-zinc-600 focus:outline-none transition-colors"
|
||||
disabled={pluginUnavailable}
|
||||
className="w-full bg-surface-sunken border border-line/60 focus:border-accent/60 rounded-lg pl-8 pr-7 py-1.5 text-[11px] text-ink placeholder-zinc-600 focus:outline-none transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
|
||||
/>
|
||||
{searchQuery && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
setSearchQuery("");
|
||||
setDebouncedQuery("");
|
||||
setSearchQuery('');
|
||||
setDebouncedQuery('');
|
||||
}}
|
||||
aria-label="Clear search"
|
||||
className="absolute right-2 text-ink-soft hover:text-ink transition-colors text-sm leading-none"
|
||||
@ -222,52 +367,27 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Namespace filter */}
|
||||
<div className="flex items-center gap-2">
|
||||
<label htmlFor="namespace-filter" className="text-[10px] text-ink-soft shrink-0">
|
||||
Namespace:
|
||||
</label>
|
||||
<input
|
||||
id="namespace-filter"
|
||||
type="text"
|
||||
value={activeNamespace}
|
||||
onChange={(e) => setActiveNamespace(e.target.value)}
|
||||
placeholder="all namespaces"
|
||||
aria-label="Filter by namespace"
|
||||
className="flex-1 bg-surface-sunken border border-line/60 focus:border-accent/60 rounded px-2 py-1 text-[11px] text-ink placeholder-zinc-600 focus:outline-none transition-colors min-w-0"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Toolbar */}
|
||||
<div className="px-4 py-2.5 border-b border-line/40 flex items-center justify-between shrink-0">
|
||||
<span className="text-[11px] text-ink-soft">
|
||||
{debouncedQuery
|
||||
? `${entries.length} result${entries.length !== 1 ? "s" : ""}`
|
||||
? `${entries.length} result${entries.length !== 1 ? 's' : ''}`
|
||||
: entries.length === 1
|
||||
? "1 memory"
|
||||
? '1 memory'
|
||||
: `${entries.length} memories`}
|
||||
</span>
|
||||
<div className="flex items-center gap-1.5">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setEditorState({ mode: "add" })}
|
||||
className="px-2 py-1 text-[11px] bg-accent hover:bg-accent-strong text-white rounded transition-colors"
|
||||
aria-label="Add memory"
|
||||
>
|
||||
+ Add
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={loadEntries}
|
||||
className="px-2 py-1 text-[11px] bg-surface-card hover:bg-surface-card text-ink-mid rounded transition-colors"
|
||||
disabled={pluginUnavailable}
|
||||
className="px-2 py-1 text-[11px] bg-surface-card hover:bg-surface-card text-ink-mid rounded transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
|
||||
aria-label="Refresh memories"
|
||||
>
|
||||
↻ Refresh
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Error banner */}
|
||||
{error && (
|
||||
@ -285,47 +405,13 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
{loading ? (
|
||||
<MemorySkeletonRows />
|
||||
) : entries.length === 0 ? (
|
||||
debouncedQuery ? (
|
||||
<div className="flex flex-col items-center justify-center py-16 gap-3 text-center">
|
||||
<span className="text-4xl text-ink-soft" aria-hidden="true">◇</span>
|
||||
<p className="text-sm font-medium text-ink-mid">
|
||||
No memories match your search
|
||||
</p>
|
||||
<p className="text-[11px] text-ink-soft max-w-[200px] leading-relaxed">
|
||||
Try a different query or{" "}
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
setSearchQuery("");
|
||||
setDebouncedQuery("");
|
||||
}}
|
||||
className="text-accent hover:text-accent underline transition-colors"
|
||||
>
|
||||
clear the search
|
||||
</button>
|
||||
.
|
||||
</p>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex flex-col items-center justify-center py-16 gap-3 text-center">
|
||||
<span className="text-4xl text-ink-soft" aria-hidden="true">◇</span>
|
||||
<p className="text-sm font-medium text-ink-mid">No {activeScope} memories</p>
|
||||
<p className="text-[11px] text-ink-soft max-w-[200px] leading-relaxed">
|
||||
{activeScope === "LOCAL"
|
||||
? "This workspace has not written any local memories yet."
|
||||
: activeScope === "TEAM"
|
||||
? "No team memories shared with this workspace yet."
|
||||
: "No global memories exist yet."}
|
||||
</p>
|
||||
</div>
|
||||
)
|
||||
<EmptyState query={debouncedQuery} pluginUnavailable={pluginUnavailable} />
|
||||
) : (
|
||||
<div className="space-y-1.5">
|
||||
{entries.map((entry) => (
|
||||
<MemoryEntryRow
|
||||
key={entry.id}
|
||||
entry={entry}
|
||||
onEdit={() => setEditorState({ mode: "edit", entry })}
|
||||
onDelete={() => setPendingDeleteId(entry.id)}
|
||||
/>
|
||||
))}
|
||||
@ -336,36 +422,64 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
{/* Delete confirmation dialog */}
|
||||
<ConfirmDialog
|
||||
open={pendingDeleteId !== null}
|
||||
title="Delete memory"
|
||||
message={`Delete this ${activeScope} memory? This cannot be undone.`}
|
||||
confirmLabel="Delete"
|
||||
title="Forget memory"
|
||||
message="Forget this memory? This cannot be undone."
|
||||
confirmLabel="Forget"
|
||||
confirmVariant="danger"
|
||||
onConfirm={confirmDelete}
|
||||
onCancel={() => setPendingDeleteId(null)}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
{/* Add / Edit dialog */}
|
||||
{editorState?.mode === "add" && (
|
||||
<MemoryEditorDialog
|
||||
open={true}
|
||||
mode="add"
|
||||
workspaceId={workspaceId}
|
||||
defaultScope={activeScope}
|
||||
defaultNamespace={activeNamespace || "general"}
|
||||
onClose={() => setEditorState(null)}
|
||||
onSaved={loadEntries}
|
||||
/>
|
||||
)}
|
||||
{editorState?.mode === "edit" && (
|
||||
<MemoryEditorDialog
|
||||
open={true}
|
||||
mode="edit"
|
||||
workspaceId={workspaceId}
|
||||
entry={editorState.entry}
|
||||
onClose={() => setEditorState(null)}
|
||||
onSaved={loadEntries}
|
||||
/>
|
||||
)}
|
||||
// ── Empty state ─────────────────────────────────────────────────────────────
|
||||
|
||||
function EmptyState({
|
||||
query,
|
||||
pluginUnavailable,
|
||||
}: {
|
||||
query: string;
|
||||
pluginUnavailable: boolean;
|
||||
}) {
|
||||
if (pluginUnavailable) {
|
||||
// The banner already explains the problem; the empty rows just
|
||||
// mirror it so the operator sees both signals.
|
||||
return (
|
||||
<div className="flex flex-col items-center justify-center py-16 gap-3 text-center">
|
||||
<span className="text-4xl text-ink-soft" aria-hidden="true">
|
||||
◇
|
||||
</span>
|
||||
<p className="text-sm font-medium text-ink-mid">Memory plugin disabled</p>
|
||||
<p className="text-[11px] text-ink-soft max-w-[220px] leading-relaxed">
|
||||
See banner above for the operator-side fix.
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
if (query) {
|
||||
return (
|
||||
<div className="flex flex-col items-center justify-center py-16 gap-3 text-center">
|
||||
<span className="text-4xl text-ink-soft" aria-hidden="true">
|
||||
◇
|
||||
</span>
|
||||
<p className="text-sm font-medium text-ink-mid">No memories match your search</p>
|
||||
<p className="text-[11px] text-ink-soft max-w-[200px] leading-relaxed">
|
||||
Try a different query or clear the search.
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<div className="flex flex-col items-center justify-center py-16 gap-3 text-center">
|
||||
<span className="text-4xl text-ink-soft" aria-hidden="true">
|
||||
◇
|
||||
</span>
|
||||
<p className="text-sm font-medium text-ink-mid">No memories yet</p>
|
||||
<p className="text-[11px] text-ink-soft max-w-[220px] leading-relaxed">
|
||||
Agents commit memories via MCP tools (commit_memory, commit_summary). They
|
||||
appear here once written.
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@ -373,17 +487,32 @@ export function MemoryInspectorPanel({ workspaceId }: Props) {
|
||||
// ── MemoryEntryRow sub-component ──────────────────────────────────────────────
|
||||
|
||||
interface MemoryEntryRowProps {
|
||||
entry: MemoryEntry;
|
||||
onEdit: () => void;
|
||||
entry: MemoryV2;
|
||||
onDelete: () => void;
|
||||
}
|
||||
|
||||
function MemoryEntryRow({ entry, onEdit, onDelete }: MemoryEntryRowProps) {
|
||||
const KIND_BADGE_CLASS: Record<MemoryKind, string> = {
|
||||
fact: 'bg-surface-card text-ink-mid',
|
||||
summary: 'bg-blue-950 text-accent',
|
||||
checkpoint: 'bg-violet-950 text-violet-400',
|
||||
};
|
||||
|
||||
const SOURCE_BADGE_CLASS: Record<MemorySource, string> = {
|
||||
agent: 'bg-surface-card text-ink-mid',
|
||||
runtime: 'bg-amber-950 text-amber-300',
|
||||
user: 'bg-emerald-950 text-emerald-400',
|
||||
};
|
||||
|
||||
function MemoryEntryRow({ entry, onDelete }: MemoryEntryRowProps) {
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const bodyId = `mem-body-${sanitizeId(entry.id)}`;
|
||||
const ttl = formatTTL(entry.expires_at);
|
||||
|
||||
return (
|
||||
<div className="rounded-lg border border-line/60 bg-surface-sunken/50 overflow-hidden">
|
||||
<div
|
||||
className="rounded-lg border border-line/60 bg-surface-sunken/50 overflow-hidden"
|
||||
data-testid={`memory-row-${entry.id}`}
|
||||
>
|
||||
{/* Header row */}
|
||||
<button
|
||||
type="button"
|
||||
@ -392,52 +521,89 @@ function MemoryEntryRow({ entry, onEdit, onDelete }: MemoryEntryRowProps) {
|
||||
aria-expanded={expanded}
|
||||
aria-controls={bodyId}
|
||||
>
|
||||
{/* Scope badge */}
|
||||
{/* Kind badge */}
|
||||
<span
|
||||
className={[
|
||||
"text-[9px] shrink-0 font-mono px-1 py-0.5 rounded",
|
||||
entry.scope === "LOCAL"
|
||||
? "bg-surface-card text-ink-mid"
|
||||
: entry.scope === "TEAM"
|
||||
? "bg-blue-950 text-accent"
|
||||
: "bg-violet-950 text-violet-400",
|
||||
].join(" ")}
|
||||
title={`Scope: ${entry.scope}`}
|
||||
'text-[9px] shrink-0 font-mono px-1 py-0.5 rounded',
|
||||
KIND_BADGE_CLASS[entry.kind] ?? 'bg-surface-card text-ink-mid',
|
||||
].join(' ')}
|
||||
title={`Kind: ${entry.kind}`}
|
||||
data-testid="kind-badge"
|
||||
>
|
||||
{entry.scope[0]}
|
||||
{entry.kind[0].toUpperCase()}
|
||||
</span>
|
||||
|
||||
{/* Source badge */}
|
||||
<span
|
||||
className={[
|
||||
'text-[9px] shrink-0 font-mono px-1 py-0.5 rounded',
|
||||
SOURCE_BADGE_CLASS[entry.source] ?? 'bg-surface-card text-ink-mid',
|
||||
].join(' ')}
|
||||
title={`Source: ${entry.source}`}
|
||||
data-testid="source-badge"
|
||||
>
|
||||
{entry.source}
|
||||
</span>
|
||||
|
||||
{/* Pin indicator */}
|
||||
{entry.pin && (
|
||||
<span
|
||||
className="text-[9px] shrink-0"
|
||||
title="Pinned"
|
||||
data-testid="pin-badge"
|
||||
aria-label="Pinned"
|
||||
>
|
||||
📌
|
||||
</span>
|
||||
)}
|
||||
|
||||
{/* Namespace tag */}
|
||||
<span className="text-[9px] shrink-0 font-mono text-ink-soft truncate max-w-[80px]" title={entry.namespace}>
|
||||
<span
|
||||
className="text-[9px] shrink-0 font-mono text-ink-soft truncate max-w-[100px]"
|
||||
title={entry.namespace}
|
||||
>
|
||||
{entry.namespace}
|
||||
</span>
|
||||
|
||||
{/* Content preview */}
|
||||
<span className="flex-1 min-w-0 text-[10px] font-mono text-ink-mid truncate text-left">
|
||||
{entry.content.length > 60 ? entry.content.slice(0, 60) + "…" : entry.content}
|
||||
{entry.content.length > 60 ? entry.content.slice(0, 60) + '…' : entry.content}
|
||||
</span>
|
||||
|
||||
{/* Similarity badge */}
|
||||
{entry.similarity_score != null && (
|
||||
{/* Score badge (semantic search only) */}
|
||||
{entry.score != null && (
|
||||
<span
|
||||
className={[
|
||||
"text-[9px] shrink-0 font-mono tabular-nums",
|
||||
entry.similarity_score >= 0.8
|
||||
? "text-accent"
|
||||
: "text-ink-mid",
|
||||
].join(" ")}
|
||||
title={`Similarity: ${(entry.similarity_score * 100).toFixed(1)}%`}
|
||||
data-testid="similarity-badge"
|
||||
'text-[9px] shrink-0 font-mono tabular-nums',
|
||||
entry.score >= 0.8 ? 'text-accent' : 'text-ink-mid',
|
||||
].join(' ')}
|
||||
title={`Similarity: ${(entry.score * 100).toFixed(1)}%`}
|
||||
data-testid="score-badge"
|
||||
>
|
||||
{Math.round(entry.similarity_score * 100)}%
|
||||
{Math.round(entry.score * 100)}%
|
||||
</span>
|
||||
)}
|
||||
|
||||
{/* TTL countdown */}
|
||||
{ttl && (
|
||||
<span
|
||||
className={[
|
||||
'text-[9px] shrink-0 font-mono',
|
||||
ttl === 'expired' ? 'text-bad' : 'text-amber-400',
|
||||
].join(' ')}
|
||||
title={`Expires: ${entry.expires_at}`}
|
||||
data-testid="ttl-badge"
|
||||
>
|
||||
⌛{ttl}
|
||||
</span>
|
||||
)}
|
||||
|
||||
|
||||
<span className="text-[9px] text-ink-soft shrink-0">
|
||||
{formatRelativeTime(entry.created_at)}
|
||||
</span>
|
||||
<span className="text-[9px] text-ink-soft shrink-0" aria-hidden="true">
|
||||
{expanded ? "▼" : "▶"}
|
||||
{expanded ? '▼' : '▶'}
|
||||
</span>
|
||||
</button>
|
||||
|
||||
@ -455,33 +621,21 @@ function MemoryEntryRow({ entry, onEdit, onDelete }: MemoryEntryRowProps) {
|
||||
<div className="flex items-center justify-between gap-2">
|
||||
<span className="text-[9px] text-ink-soft">
|
||||
Created: {new Date(entry.created_at).toLocaleString()}
|
||||
{entry.expires_at && ` · Expires: ${new Date(entry.expires_at).toLocaleString()}`}
|
||||
</span>
|
||||
<div className="flex items-center gap-1.5 shrink-0">
|
||||
<button
|
||||
type="button"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onEdit();
|
||||
}}
|
||||
aria-label="Edit memory"
|
||||
className="text-[10px] px-2 py-0.5 bg-surface-card hover:bg-surface-elevated border border-line/40 rounded text-ink-mid hover:text-ink transition-colors"
|
||||
>
|
||||
Edit
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onDelete();
|
||||
}}
|
||||
aria-label="Delete memory"
|
||||
className="text-[10px] px-2 py-0.5 bg-red-950/40 hover:bg-red-900/50 border border-red-900/30 rounded text-bad transition-colors"
|
||||
aria-label="Forget memory"
|
||||
className="text-[10px] px-2 py-0.5 bg-red-950/40 hover:bg-red-900/50 border border-red-900/30 rounded text-bad transition-colors shrink-0"
|
||||
>
|
||||
Delete
|
||||
Forget
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
|
||||
@ -1,202 +0,0 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* MemoryEditorDialog tests — covers Add (POST /memories) and Edit
|
||||
* (PATCH /memories/:id) flows. Pins:
|
||||
* - Add posts {content, scope, namespace} with the trimmed defaults
|
||||
* - Edit only sends fields that changed (no-op edit short-circuits, no PATCH fires)
|
||||
* - Empty content blocks save
|
||||
* - Save error surfaces in the dialog and keeps the modal open
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent, waitFor, cleanup } from "@testing-library/react";
|
||||
|
||||
vi.mock("@/lib/api", () => ({
|
||||
api: {
|
||||
get: vi.fn(),
|
||||
post: vi.fn(),
|
||||
patch: vi.fn(),
|
||||
del: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { api } from "@/lib/api";
|
||||
import { MemoryEditorDialog } from "../MemoryEditorDialog";
|
||||
import type { MemoryEntry } from "../MemoryInspectorPanel";
|
||||
|
||||
const mockPost = vi.mocked(api.post);
|
||||
const mockPatch = vi.mocked(api.patch);
|
||||
|
||||
const SAMPLE: MemoryEntry = {
|
||||
id: "mem-x",
|
||||
workspace_id: "ws-1",
|
||||
content: "original content",
|
||||
scope: "TEAM",
|
||||
namespace: "procedures",
|
||||
created_at: "2026-04-17T12:00:00.000Z",
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockPost.mockResolvedValue({} as never);
|
||||
mockPatch.mockResolvedValue({} as never);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
describe("Add mode", () => {
|
||||
it("POSTs scope+namespace+trimmed-content and calls onSaved+onClose", async () => {
|
||||
const onClose = vi.fn();
|
||||
const onSaved = vi.fn();
|
||||
render(
|
||||
<MemoryEditorDialog
|
||||
open
|
||||
mode="add"
|
||||
workspaceId="ws-1"
|
||||
defaultScope="GLOBAL"
|
||||
defaultNamespace="facts"
|
||||
onClose={onClose}
|
||||
onSaved={onSaved}
|
||||
/>,
|
||||
);
|
||||
|
||||
const textarea = screen.getByLabelText(/Content/i) as HTMLTextAreaElement;
|
||||
fireEvent.change(textarea, { target: { value: " new fact " } });
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /Add memory$/i }));
|
||||
|
||||
await waitFor(() => expect(mockPost).toHaveBeenCalledTimes(1));
|
||||
expect(mockPost).toHaveBeenCalledWith("/workspaces/ws-1/memories", {
|
||||
content: "new fact",
|
||||
scope: "GLOBAL",
|
||||
namespace: "facts",
|
||||
});
|
||||
expect(onSaved).toHaveBeenCalledTimes(1);
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks save when content is empty (whitespace-only)", () => {
|
||||
const onClose = vi.fn();
|
||||
const onSaved = vi.fn();
|
||||
render(
|
||||
<MemoryEditorDialog
|
||||
open
|
||||
mode="add"
|
||||
workspaceId="ws-1"
|
||||
defaultScope="LOCAL"
|
||||
onClose={onClose}
|
||||
onSaved={onSaved}
|
||||
/>,
|
||||
);
|
||||
const textarea = screen.getByLabelText(/Content/i) as HTMLTextAreaElement;
|
||||
fireEvent.change(textarea, { target: { value: " " } });
|
||||
fireEvent.click(screen.getByRole("button", { name: /Add memory$/i }));
|
||||
expect(mockPost).not.toHaveBeenCalled();
|
||||
expect(screen.getByRole("alert").textContent).toMatch(/empty/i);
|
||||
expect(onSaved).not.toHaveBeenCalled();
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("Edit mode", () => {
|
||||
it("PATCHes only changed fields", async () => {
|
||||
const onClose = vi.fn();
|
||||
const onSaved = vi.fn();
|
||||
render(
|
||||
<MemoryEditorDialog
|
||||
open
|
||||
mode="edit"
|
||||
workspaceId="ws-1"
|
||||
entry={SAMPLE}
|
||||
onClose={onClose}
|
||||
onSaved={onSaved}
|
||||
/>,
|
||||
);
|
||||
|
||||
const textarea = screen.getByLabelText(/Content/i) as HTMLTextAreaElement;
|
||||
fireEvent.change(textarea, { target: { value: "rewritten content" } });
|
||||
// namespace untouched
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /Save changes/i }));
|
||||
|
||||
await waitFor(() => expect(mockPatch).toHaveBeenCalledTimes(1));
|
||||
expect(mockPatch).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories/mem-x",
|
||||
{ content: "rewritten content" },
|
||||
);
|
||||
expect(onSaved).toHaveBeenCalledTimes(1);
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("no-op edit short-circuits (no PATCH fires) and still closes", async () => {
|
||||
const onClose = vi.fn();
|
||||
const onSaved = vi.fn();
|
||||
render(
|
||||
<MemoryEditorDialog
|
||||
open
|
||||
mode="edit"
|
||||
workspaceId="ws-1"
|
||||
entry={SAMPLE}
|
||||
onClose={onClose}
|
||||
onSaved={onSaved}
|
||||
/>,
|
||||
);
|
||||
fireEvent.click(screen.getByRole("button", { name: /Save changes/i }));
|
||||
await waitFor(() => expect(onClose).toHaveBeenCalled());
|
||||
expect(mockPatch).not.toHaveBeenCalled();
|
||||
expect(onSaved).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("sends namespace too when both content and namespace changed", async () => {
|
||||
const onClose = vi.fn();
|
||||
const onSaved = vi.fn();
|
||||
render(
|
||||
<MemoryEditorDialog
|
||||
open
|
||||
mode="edit"
|
||||
workspaceId="ws-1"
|
||||
entry={SAMPLE}
|
||||
onClose={onClose}
|
||||
onSaved={onSaved}
|
||||
/>,
|
||||
);
|
||||
fireEvent.change(screen.getByLabelText(/Content/i), {
|
||||
target: { value: "newer content" },
|
||||
});
|
||||
fireEvent.change(screen.getByLabelText(/Namespace/i), {
|
||||
target: { value: "blockers" },
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: /Save changes/i }));
|
||||
await waitFor(() => expect(mockPatch).toHaveBeenCalledTimes(1));
|
||||
expect(mockPatch).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories/mem-x",
|
||||
{ content: "newer content", namespace: "blockers" },
|
||||
);
|
||||
});
|
||||
|
||||
it("surfaces save error and keeps the modal open", async () => {
|
||||
const onClose = vi.fn();
|
||||
const onSaved = vi.fn();
|
||||
mockPatch.mockRejectedValueOnce(new Error("boom"));
|
||||
render(
|
||||
<MemoryEditorDialog
|
||||
open
|
||||
mode="edit"
|
||||
workspaceId="ws-1"
|
||||
entry={SAMPLE}
|
||||
onClose={onClose}
|
||||
onSaved={onSaved}
|
||||
/>,
|
||||
);
|
||||
fireEvent.change(screen.getByLabelText(/Content/i), {
|
||||
target: { value: "rewritten content" },
|
||||
});
|
||||
fireEvent.click(screen.getByRole("button", { name: /Save changes/i }));
|
||||
await waitFor(() =>
|
||||
expect(screen.getByRole("alert").textContent).toMatch(/boom/),
|
||||
);
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
expect(onSaved).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@ -1,16 +1,29 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* MemoryInspectorPanel tests — issue #909
|
||||
* MemoryInspectorPanel — v2 redesign tests.
|
||||
*
|
||||
* Covers: loading, empty state, scope tabs, namespace filter,
|
||||
* entry list, expand, delete flow, optimistic updates, Refresh, semantic search.
|
||||
* Coverage targets every behavior the panel surfaces:
|
||||
* - Initial load wires GET /v2/namespaces + GET /v2/memories
|
||||
* - Plugin-unavailable banner (503) renders + disables interactions
|
||||
* - Generic error renders in the error banner
|
||||
* - Namespace dropdown populates from /v2/namespaces.readable; "All
|
||||
* namespaces" is the default
|
||||
* - Selecting a namespace re-fetches with ?namespace=...
|
||||
* - Search input debounces + scopes the request to ?q=
|
||||
* - Search results sort by score descending
|
||||
* - Empty-state copy differs by query / plugin-state / no-data
|
||||
* - Per-row badges render (kind / source / pin / TTL / score /
|
||||
* score) and TTL countdown handles past/future/null
|
||||
* - Delete (Forget) flow: optimistic removal, confirmation dialog,
|
||||
* server failure rolls back via reload
|
||||
* - formatTTL helper covers s/m/h/d/expired/null/invalid branches
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent, waitFor, cleanup, act } from "@testing-library/react";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react';
|
||||
|
||||
// ── Mocks ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("@/lib/api", () => ({
|
||||
vi.mock('@/lib/api', () => ({
|
||||
api: {
|
||||
get: vi.fn(),
|
||||
post: vi.fn(),
|
||||
@ -18,7 +31,7 @@ vi.mock("@/lib/api", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/components/ConfirmDialog", () => ({
|
||||
vi.mock('@/components/ConfirmDialog', () => ({
|
||||
ConfirmDialog: ({
|
||||
open,
|
||||
title,
|
||||
@ -33,435 +46,473 @@ vi.mock("@/components/ConfirmDialog", () => ({
|
||||
confirmVariant?: string;
|
||||
onConfirm: () => void;
|
||||
onCancel: () => void;
|
||||
singleButton?: boolean;
|
||||
}) =>
|
||||
open ? (
|
||||
<div data-testid="confirm-dialog">
|
||||
<p data-testid="dialog-title">{title}</p>
|
||||
<p data-testid="dialog-message">{message}</p>
|
||||
<button onClick={onConfirm}>Confirm Delete</button>
|
||||
<button onClick={onCancel}>Cancel Delete</button>
|
||||
<button onClick={onConfirm}>Confirm</button>
|
||||
<button onClick={onCancel}>Cancel</button>
|
||||
</div>
|
||||
) : null,
|
||||
}));
|
||||
|
||||
import { api } from "@/lib/api";
|
||||
import { MemoryInspectorPanel } from "../MemoryInspectorPanel";
|
||||
|
||||
// ── Typed mock helpers ────────────────────────────────────────────────────────
|
||||
import { api } from '@/lib/api';
|
||||
import {
|
||||
MemoryInspectorPanel,
|
||||
formatTTL,
|
||||
isPluginUnavailableError,
|
||||
type MemoryV2,
|
||||
type NamespacesResponse,
|
||||
} from '../MemoryInspectorPanel';
|
||||
|
||||
const mockGet = vi.mocked(api.get);
|
||||
const mockDel = vi.mocked(api.del);
|
||||
|
||||
// ── Sample fixtures ───────────────────────────────────────────────────────────
|
||||
// ── Fixtures ──────────────────────────────────────────────────────────────────
|
||||
|
||||
const NOW = "2026-04-17T12:00:00.000Z";
|
||||
|
||||
const MEMORY_A: import("../MemoryInspectorPanel").MemoryEntry = {
|
||||
id: "mem-a",
|
||||
workspace_id: "ws-1",
|
||||
content: "Remember to review PRs before merging",
|
||||
scope: "LOCAL",
|
||||
namespace: "general",
|
||||
created_at: NOW,
|
||||
const NS_RESPONSE: NamespacesResponse = {
|
||||
readable: [
|
||||
{ name: 'workspace:ws-1', kind: 'workspace', label: 'Workspace (ws-1)' },
|
||||
{ name: 'team:t-1', kind: 'team', label: 'Team (t-1)' },
|
||||
],
|
||||
writable: [{ name: 'workspace:ws-1', kind: 'workspace', label: 'Workspace (ws-1)' }],
|
||||
};
|
||||
|
||||
const MEMORY_B: import("../MemoryInspectorPanel").MemoryEntry = {
|
||||
id: "mem-b",
|
||||
workspace_id: "ws-1",
|
||||
content: "Team knowledge: deploy happens on Fridays",
|
||||
scope: "TEAM",
|
||||
namespace: "procedures",
|
||||
created_at: NOW,
|
||||
const MEM_BASIC: MemoryV2 = {
|
||||
id: 'mem-a',
|
||||
namespace: 'workspace:ws-1',
|
||||
content: 'Remember the standup is at 10am',
|
||||
kind: 'fact',
|
||||
source: 'agent',
|
||||
pin: false,
|
||||
created_at: '2026-04-17T12:00:00.000Z',
|
||||
};
|
||||
|
||||
const TWO_MEMORIES = [MEMORY_A, MEMORY_B];
|
||||
const MEM_PINNED: MemoryV2 = {
|
||||
id: 'mem-pinned',
|
||||
namespace: 'team:t-1',
|
||||
content: 'Team retro every Friday',
|
||||
kind: 'summary',
|
||||
source: 'user',
|
||||
pin: true,
|
||||
expires_at: new Date(Date.now() + 86_400_000).toISOString(),
|
||||
created_at: '2026-04-17T12:00:00.000Z',
|
||||
};
|
||||
|
||||
const MEM_RUNTIME_CHECKPOINT: MemoryV2 = {
|
||||
id: 'mem-checkpoint',
|
||||
namespace: 'team:t-1',
|
||||
content: 'Runtime checkpoint',
|
||||
kind: 'checkpoint',
|
||||
source: 'runtime',
|
||||
pin: false,
|
||||
created_at: '2026-04-17T12:00:00.000Z',
|
||||
};
|
||||
|
||||
const MEM_EXPIRED: MemoryV2 = {
|
||||
id: 'mem-expired',
|
||||
namespace: 'workspace:ws-1',
|
||||
content: 'Stale memory',
|
||||
kind: 'fact',
|
||||
source: 'agent',
|
||||
pin: false,
|
||||
expires_at: new Date(Date.now() - 1000).toISOString(),
|
||||
created_at: '2026-04-17T12:00:00.000Z',
|
||||
};
|
||||
|
||||
// ── Setup / teardown ──────────────────────────────────────────────────────────
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockGet.mockReset();
|
||||
mockDel.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
// ── Helper: flush microtasks + React state updates ─────────────────────────────
|
||||
async function flushUpdates(): Promise<void> {
|
||||
await act(async () => {});
|
||||
// Helper: stub a basic two-call flow (namespaces + memories).
|
||||
function stubFetch(memories: MemoryV2[], namespaces: NamespacesResponse = NS_RESPONSE) {
|
||||
mockGet.mockImplementation(((url: string) => {
|
||||
if (url.includes('/v2/namespaces')) {
|
||||
return Promise.resolve(namespaces);
|
||||
}
|
||||
return Promise.resolve({ memories });
|
||||
}) as typeof api.get);
|
||||
}
|
||||
|
||||
// ── Loading & empty state ─────────────────────────────────────────────────────
|
||||
// ── isPluginUnavailableError helper ─────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — loading and empty state", () => {
|
||||
it("shows loading indicator before data arrives", () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockReturnValue(new Promise(() => {}) as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
expect(screen.getByText(/loading memories/i)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("renders empty state when API returns []", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByText("No LOCAL memories")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("fetches from the correct workspace memories endpoint with scope=LOCAL", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-abc-123" />);
|
||||
await flushUpdates();
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-abc-123/memories?scope=LOCAL"
|
||||
);
|
||||
});
|
||||
|
||||
it("shows error banner when fetch throws", async () => {
|
||||
mockGet.mockRejectedValue(new Error("Network error"));
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByText("Network error")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Scope tabs ────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — scope tabs", () => {
|
||||
it("renders LOCAL, TEAM, GLOBAL tabs", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByRole("button", { name: "LOCAL" })).toBeTruthy();
|
||||
expect(screen.getByRole("button", { name: "TEAM" })).toBeTruthy();
|
||||
expect(screen.getByRole("button", { name: "GLOBAL" })).toBeTruthy();
|
||||
});
|
||||
|
||||
it("LOCAL is active by default", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByRole("button", { name: "LOCAL" }).getAttribute("aria-pressed")).toBe("true");
|
||||
});
|
||||
|
||||
it("clicking TEAM tab re-fetches with scope=TEAM", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
mockGet.mockClear();
|
||||
fireEvent.click(screen.getByRole("button", { name: "TEAM" }));
|
||||
await flushUpdates();
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories?scope=TEAM"
|
||||
);
|
||||
});
|
||||
|
||||
it("clicking GLOBAL tab re-fetches with scope=GLOBAL", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
mockGet.mockClear();
|
||||
fireEvent.click(screen.getByRole("button", { name: "GLOBAL" }));
|
||||
await flushUpdates();
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories?scope=GLOBAL"
|
||||
);
|
||||
});
|
||||
|
||||
it("shows scope-specific empty state when switching tabs", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "TEAM" }));
|
||||
await flushUpdates();
|
||||
expect(screen.getByText("No TEAM memories")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Namespace filter ──────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — namespace filter", () => {
|
||||
it("renders namespace filter input", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByLabelText("Filter by namespace")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("includes namespace param in API call when set", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
mockGet.mockClear();
|
||||
fireEvent.change(screen.getByLabelText("Filter by namespace"), {
|
||||
target: { value: "facts" },
|
||||
});
|
||||
// Advance past the 300ms debounce
|
||||
act(() => { vi.advanceTimersByTime(350); });
|
||||
await flushUpdates();
|
||||
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories?scope=LOCAL&namespace=facts"
|
||||
);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── Entry list ───────────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — entry list", () => {
|
||||
beforeEach(() => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue(TWO_MEMORIES as any);
|
||||
});
|
||||
|
||||
it("renders a row for every memory", async () => {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByText(/Remember to review PRs before merging/)).toBeTruthy();
|
||||
expect(screen.getByText(/Team knowledge: deploy happens on Fridays/)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("displays memory count in toolbar", async () => {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByText("2 memories")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("displays scope badge for each entry", async () => {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByTitle("Scope: LOCAL")).toBeTruthy();
|
||||
expect(screen.getByTitle("Scope: TEAM")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("entries are collapsed by default (pre region not visible)", async () => {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
// Expanded region (pre tag) should not exist in DOM yet
|
||||
expect(screen.queryByRole("region")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Expand / collapse ─────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — expand/collapse", () => {
|
||||
beforeEach(() => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue(TWO_MEMORIES as any);
|
||||
});
|
||||
|
||||
it("clicking a row header expands it and shows the full content in a pre tag", async () => {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
fireEvent.click(
|
||||
screen.getByText(/Remember to review PRs before merging/).closest("button")!
|
||||
);
|
||||
await flushUpdates();
|
||||
// After expand, a region with the full content <pre> should appear
|
||||
expect(screen.getByRole("region")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("clicking the header again collapses the row (pre region removed)", async () => {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
const headerBtn = screen
|
||||
.getByText(/Remember to review PRs before merging/)
|
||||
.closest("button")!;
|
||||
fireEvent.click(headerBtn); // expand
|
||||
await flushUpdates();
|
||||
expect(screen.getByRole("region")).toBeTruthy();
|
||||
|
||||
fireEvent.click(headerBtn); // collapse
|
||||
await flushUpdates();
|
||||
// After collapse, the region (pre) is removed from the DOM
|
||||
expect(screen.queryByRole("region")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Delete flow ───────────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — delete flow", () => {
|
||||
beforeEach(() => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue(TWO_MEMORIES as any);
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockDel.mockResolvedValue({ status: "deleted" } as any);
|
||||
});
|
||||
|
||||
/** Helper: expand memory-A and click its Delete button */
|
||||
async function openDeleteForMemoryA() {
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
fireEvent.click(
|
||||
screen.getByText(/Remember to review PRs before merging/).closest("button")!
|
||||
);
|
||||
await flushUpdates();
|
||||
fireEvent.click(screen.getByRole("button", { name: "Delete memory" }));
|
||||
await flushUpdates();
|
||||
}
|
||||
|
||||
it("opens ConfirmDialog when Delete is clicked", async () => {
|
||||
await openDeleteForMemoryA();
|
||||
expect(screen.getByTestId("confirm-dialog")).toBeTruthy();
|
||||
expect(screen.getByTestId("dialog-title").textContent).toBe("Delete memory");
|
||||
});
|
||||
|
||||
it("calls api.del with the correct URL-encoded path on confirm", async () => {
|
||||
await openDeleteForMemoryA();
|
||||
fireEvent.click(screen.getByText("Confirm Delete"));
|
||||
await flushUpdates();
|
||||
expect(mockDel).toHaveBeenCalledWith("/workspaces/ws-1/memories/mem-a");
|
||||
});
|
||||
|
||||
it("removes the entry optimistically after confirm", async () => {
|
||||
await openDeleteForMemoryA();
|
||||
fireEvent.click(screen.getByText("Confirm Delete"));
|
||||
await flushUpdates();
|
||||
expect(screen.queryByText(/Remember to review PRs before merging/)).toBeNull();
|
||||
// Sibling entry unaffected
|
||||
expect(screen.getByText(/Team knowledge: deploy happens on Fridays/)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("closes ConfirmDialog without deleting when Cancel is clicked", async () => {
|
||||
await openDeleteForMemoryA();
|
||||
fireEvent.click(screen.getByText("Cancel Delete"));
|
||||
await flushUpdates();
|
||||
expect(screen.queryByTestId("confirm-dialog")).toBeNull();
|
||||
expect(mockDel).not.toHaveBeenCalled();
|
||||
// Sibling memory entry (MEMORY_B) is still in the list
|
||||
expect(screen.getByText(/Team knowledge: deploy happens on Fridays/)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Refresh ───────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — Refresh button", () => {
|
||||
it("re-fetches entries when Refresh is clicked", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
expect(screen.getByText("No LOCAL memories")).toBeTruthy();
|
||||
|
||||
expect(mockGet).toHaveBeenCalledTimes(1);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Refresh memories" }));
|
||||
await flushUpdates();
|
||||
expect(mockGet).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
// ── role=alert a11y ──────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — error elements have role=alert", () => {
|
||||
it("fetch error banner has role='alert'", async () => {
|
||||
mockGet.mockRejectedValue(new Error("Network error"));
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
const alert = screen.getByRole("alert");
|
||||
expect(alert).toBeTruthy();
|
||||
expect(alert.textContent).toContain("Network error");
|
||||
});
|
||||
});
|
||||
|
||||
// ── Semantic search ──────────────────────────────────────────────────────────
|
||||
|
||||
describe("MemoryInspectorPanel — semantic search", () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("debounces search input by 300ms before calling API", async () => {
|
||||
vi.useFakeTimers();
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
mockGet.mockClear();
|
||||
|
||||
fireEvent.change(screen.getByLabelText("Search memories"), {
|
||||
target: { value: "deploy" },
|
||||
});
|
||||
|
||||
// 200ms — debounce has NOT fired yet
|
||||
act(() => { vi.advanceTimersByTime(200); });
|
||||
await flushUpdates();
|
||||
expect(mockGet).not.toHaveBeenCalled();
|
||||
|
||||
// 350ms total — debounce fires
|
||||
act(() => { vi.advanceTimersByTime(150); });
|
||||
await flushUpdates();
|
||||
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories?scope=LOCAL&q=deploy"
|
||||
);
|
||||
});
|
||||
|
||||
it("renders similarity-badge when entry has similarity_score", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([{ ...MEMORY_A, similarity_score: 0.87 }] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
const badge = document.querySelector('[data-testid="similarity-badge"]');
|
||||
expect(badge).toBeTruthy();
|
||||
expect(badge?.textContent).toBe("87%");
|
||||
});
|
||||
|
||||
it("does not render similarity-badge when entry has no similarity_score", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([MEMORY_A] as any);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
describe('isPluginUnavailableError', () => {
|
||||
it('matches the literal env var contract from the server handler', () => {
|
||||
expect(
|
||||
document.querySelector('[data-testid="similarity-badge"]')
|
||||
).toBeNull();
|
||||
isPluginUnavailableError(
|
||||
new Error('API GET /workspaces/x/v2/memories: 503 {"error":"memory plugin is not configured (set MEMORY_PLUGIN_URL)"}'),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("clear button resets query immediately and re-fetches without ?q=", async () => {
|
||||
vi.useFakeTimers();
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
mockGet.mockResolvedValue([] as any);
|
||||
it('does not false-match on generic 503 errors that don\'t mention the env var', () => {
|
||||
expect(isPluginUnavailableError(new Error('API GET /foo: 503 something else'))).toBe(false);
|
||||
});
|
||||
|
||||
it('does not false-match on plain 4xx errors', () => {
|
||||
expect(isPluginUnavailableError(new Error('API GET /foo: 401 unauthorized'))).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for non-Error inputs', () => {
|
||||
expect(isPluginUnavailableError(null)).toBe(false);
|
||||
expect(isPluginUnavailableError(undefined)).toBe(false);
|
||||
expect(isPluginUnavailableError('a string')).toBe(false);
|
||||
expect(isPluginUnavailableError({ message: 'MEMORY_PLUGIN_URL' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ── formatTTL helper ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('formatTTL', () => {
|
||||
it('returns empty string for null/undefined/empty', () => {
|
||||
expect(formatTTL(null)).toBe('');
|
||||
expect(formatTTL(undefined)).toBe('');
|
||||
expect(formatTTL('')).toBe('');
|
||||
});
|
||||
|
||||
it('returns empty for invalid date strings', () => {
|
||||
expect(formatTTL('not-a-date')).toBe('');
|
||||
});
|
||||
|
||||
it('returns "expired" for past timestamps', () => {
|
||||
const past = new Date(Date.now() - 5000).toISOString();
|
||||
expect(formatTTL(past)).toBe('expired');
|
||||
});
|
||||
|
||||
it('formats <60s as seconds', () => {
|
||||
const future = new Date(Date.now() + 30_000).toISOString();
|
||||
expect(formatTTL(future)).toMatch(/^\d{1,2}s$/);
|
||||
});
|
||||
|
||||
it('formats <60m as minutes', () => {
|
||||
const future = new Date(Date.now() + 30 * 60_000).toISOString();
|
||||
expect(formatTTL(future)).toMatch(/^\d{1,2}m$/);
|
||||
});
|
||||
|
||||
it('formats <24h as hours', () => {
|
||||
const future = new Date(Date.now() + 5 * 3_600_000).toISOString();
|
||||
expect(formatTTL(future)).toMatch(/^\d{1,2}h$/);
|
||||
});
|
||||
|
||||
it('formats >24h as days', () => {
|
||||
const future = new Date(Date.now() + 3 * 86_400_000).toISOString();
|
||||
expect(formatTTL(future)).toMatch(/^\d{1,2}d$/);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Initial load + dropdown ─────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — initial load', () => {
|
||||
it('fetches namespaces and memories on mount', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await flushUpdates();
|
||||
|
||||
fireEvent.change(screen.getByLabelText("Search memories"), {
|
||||
target: { value: "deploy" },
|
||||
await waitFor(() => {
|
||||
const calls = mockGet.mock.calls.map((c) => c[0]);
|
||||
expect(calls.some((u) => u.includes('/v2/namespaces'))).toBe(true);
|
||||
expect(calls.some((u) => u.includes('/v2/memories'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
act(() => { vi.advanceTimersByTime(350); });
|
||||
await flushUpdates();
|
||||
it('renders the row contents from the memories response', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/Remember the standup is at 10am/)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories?scope=LOCAL&q=deploy"
|
||||
it('populates the namespace dropdown with readable entries + "All namespaces"', async () => {
|
||||
stubFetch([]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Filter by namespace'));
|
||||
const select = screen.getByLabelText('Filter by namespace') as HTMLSelectElement;
|
||||
const optionLabels = Array.from(select.options).map((o) => o.textContent ?? '');
|
||||
expect(optionLabels[0]).toContain('All namespaces');
|
||||
expect(optionLabels.join('|')).toContain('Workspace (ws-1)');
|
||||
expect(optionLabels.join('|')).toContain('Team (t-1)');
|
||||
});
|
||||
|
||||
it('selecting a namespace re-fetches with ?namespace=', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Filter by namespace'));
|
||||
|
||||
const select = screen.getByLabelText('Filter by namespace') as HTMLSelectElement;
|
||||
fireEvent.change(select, { target: { value: 'team:t-1' } });
|
||||
|
||||
await waitFor(() => {
|
||||
const calls = mockGet.mock.calls.map((c) => c[0] as string);
|
||||
expect(calls.some((u) => u.includes('namespace=team%3At-1'))).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── Plugin unavailable (503) ────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — plugin unavailable', () => {
|
||||
it('renders the operator-hint banner and disables search input', async () => {
|
||||
mockGet.mockRejectedValue(new Error('HTTP 503: memory plugin is not configured (set MEMORY_PLUGIN_URL)'));
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByTestId('plugin-unavailable-banner'));
|
||||
|
||||
const searchInput = screen.getByLabelText('Search memories') as HTMLInputElement;
|
||||
expect(searchInput.disabled).toBe(true);
|
||||
});
|
||||
|
||||
it('shows the empty-state explaining plugin disabled', async () => {
|
||||
mockGet.mockRejectedValue(new Error('API GET /workspaces/x/v2/memories: 503 {"error":"memory plugin is not configured (set MEMORY_PLUGIN_URL)"}'));
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByText(/Memory plugin disabled/i));
|
||||
});
|
||||
});
|
||||
|
||||
// ── Generic error (non-503) ─────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — generic errors', () => {
|
||||
it('surfaces a non-503 error in the error banner', async () => {
|
||||
mockGet.mockImplementation(((url: string) => {
|
||||
if (url.includes('/v2/namespaces')) {
|
||||
return Promise.resolve(NS_RESPONSE);
|
||||
}
|
||||
return Promise.reject(new Error('upstream timeout'));
|
||||
}) as typeof api.get);
|
||||
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => {
|
||||
// Error banner has role=alert
|
||||
const alerts = screen.getAllByRole('alert');
|
||||
const found = alerts.some((a) => a.textContent?.includes('upstream timeout'));
|
||||
expect(found).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── Search ──────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — search', () => {
|
||||
it('eventually fires query with ?q= after debounce', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Search memories'));
|
||||
|
||||
fireEvent.change(screen.getByLabelText('Search memories'), {
|
||||
target: { value: 'standup' },
|
||||
});
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
const calls = mockGet.mock.calls.map((c) => c[0] as string);
|
||||
expect(calls.some((u) => u.includes('q=standup'))).toBe(true);
|
||||
},
|
||||
{ timeout: 1500 },
|
||||
);
|
||||
mockGet.mockClear();
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Clear search" }));
|
||||
await flushUpdates();
|
||||
it('sorts results by score descending when query active', async () => {
|
||||
const lowScore: MemoryV2 = { ...MEM_BASIC, id: 'low', score: 0.2, content: 'low' };
|
||||
const highScore: MemoryV2 = { ...MEM_BASIC, id: 'high', score: 0.95, content: 'high' };
|
||||
// Plugin returns in arbitrary order; component sorts.
|
||||
mockGet.mockImplementation(((url: string) => {
|
||||
if (url.includes('/v2/namespaces')) return Promise.resolve(NS_RESPONSE);
|
||||
return Promise.resolve({ memories: [lowScore, highScore] });
|
||||
}) as typeof api.get);
|
||||
|
||||
expect(mockGet).toHaveBeenCalledWith(
|
||||
"/workspaces/ws-1/memories?scope=LOCAL"
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Search memories'));
|
||||
fireEvent.change(screen.getByLabelText('Search memories'), {
|
||||
target: { value: 'something' },
|
||||
});
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
const rows = screen.getAllByTestId(/^memory-row-/);
|
||||
// First row should be the high-score one
|
||||
expect(rows[0].getAttribute('data-testid')).toBe('memory-row-high');
|
||||
},
|
||||
{ timeout: 1500 },
|
||||
);
|
||||
});
|
||||
|
||||
it('clear-button resets the query', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Search memories'));
|
||||
|
||||
fireEvent.change(screen.getByLabelText('Search memories'), {
|
||||
target: { value: 'foo' },
|
||||
});
|
||||
fireEvent.click(screen.getByLabelText('Clear search'));
|
||||
expect((screen.getByLabelText('Search memories') as HTMLInputElement).value).toBe('');
|
||||
});
|
||||
|
||||
it('renders no-results empty-state when search has no matches', async () => {
|
||||
stubFetch([]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Search memories'));
|
||||
fireEvent.change(screen.getByLabelText('Search memories'), {
|
||||
target: { value: 'nothing' },
|
||||
});
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(screen.getByText(/No memories match your search/i)).toBeTruthy();
|
||||
},
|
||||
{ timeout: 1500 },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Per-row badges ───────────────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — row badges', () => {
|
||||
it('renders kind, source, pin, TTL badges per shape', async () => {
|
||||
stubFetch([MEM_PINNED, MEM_RUNTIME_CHECKPOINT]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
|
||||
await waitFor(() => {
|
||||
// Pinned memory: kind=summary, source=user, pin=true, TTL>0
|
||||
const pinnedRow = screen.getByTestId('memory-row-mem-pinned');
|
||||
expect(pinnedRow.querySelector('[data-testid="kind-badge"]')?.textContent).toBe('S');
|
||||
expect(pinnedRow.querySelector('[data-testid="source-badge"]')?.textContent).toBe('user');
|
||||
expect(pinnedRow.querySelector('[data-testid="pin-badge"]')).toBeTruthy();
|
||||
expect(pinnedRow.querySelector('[data-testid="ttl-badge"]')?.textContent).toMatch(/^⌛\d+[hd]$/);
|
||||
|
||||
// Checkpoint memory: kind=checkpoint, source=runtime, no pin, no TTL
|
||||
const propRow = screen.getByTestId('memory-row-mem-checkpoint');
|
||||
expect(propRow.querySelector('[data-testid="kind-badge"]')?.textContent).toBe('C');
|
||||
expect(propRow.querySelector('[data-testid="source-badge"]')?.textContent).toBe('runtime');
|
||||
expect(propRow.querySelector('[data-testid="pin-badge"]')).toBeNull();
|
||||
expect(propRow.querySelector('[data-testid="ttl-badge"]')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('TTL badge shows "expired" for past expires_at', async () => {
|
||||
stubFetch([MEM_EXPIRED]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => {
|
||||
const row = screen.getByTestId('memory-row-mem-expired');
|
||||
expect(row.querySelector('[data-testid="ttl-badge"]')?.textContent).toBe('⌛expired');
|
||||
});
|
||||
});
|
||||
|
||||
it('expanding a row shows full content + Forget button', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByTestId('memory-row-mem-a'));
|
||||
|
||||
const row = screen.getByTestId('memory-row-mem-a');
|
||||
const headerButton = row.querySelector('button');
|
||||
expect(headerButton).toBeTruthy();
|
||||
fireEvent.click(headerButton!);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByLabelText('Forget memory')).toBeTruthy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── Delete (Forget) flow ──────────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — forget flow', () => {
|
||||
it('opens the confirm dialog on Forget click and removes optimistically on confirm', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
mockDel.mockResolvedValue({ status: 'deleted' });
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
|
||||
// Expand row, click Forget
|
||||
await waitFor(() => screen.getByTestId('memory-row-mem-a'));
|
||||
const row = screen.getByTestId('memory-row-mem-a');
|
||||
fireEvent.click(row.querySelector('button')!);
|
||||
await waitFor(() => screen.getByLabelText('Forget memory'));
|
||||
fireEvent.click(screen.getByLabelText('Forget memory'));
|
||||
|
||||
// Dialog appears with v2-shaped copy (Forget, not Delete)
|
||||
expect(screen.getByTestId('dialog-title').textContent).toBe('Forget memory');
|
||||
fireEvent.click(screen.getByText('Confirm'));
|
||||
|
||||
// Optimistic removal happens immediately
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId('memory-row-mem-a')).toBeNull();
|
||||
});
|
||||
// DELETE called with the right path
|
||||
await waitFor(() => {
|
||||
const delPaths = mockDel.mock.calls.map((c) => c[0] as string);
|
||||
expect(delPaths.some((p) => p.includes('/v2/memories/mem-a'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('cancelling the dialog leaves the row in place', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByTestId('memory-row-mem-a'));
|
||||
|
||||
fireEvent.click(screen.getByTestId('memory-row-mem-a').querySelector('button')!);
|
||||
await waitFor(() => screen.getByLabelText('Forget memory'));
|
||||
fireEvent.click(screen.getByLabelText('Forget memory'));
|
||||
fireEvent.click(screen.getByText('Cancel'));
|
||||
|
||||
expect(screen.queryByTestId('memory-row-mem-a')).toBeTruthy();
|
||||
expect(mockDel).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rolls back on server failure by reloading entries', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
mockDel.mockRejectedValue(new Error('upstream 502'));
|
||||
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByTestId('memory-row-mem-a'));
|
||||
fireEvent.click(screen.getByTestId('memory-row-mem-a').querySelector('button')!);
|
||||
await waitFor(() => screen.getByLabelText('Forget memory'));
|
||||
fireEvent.click(screen.getByLabelText('Forget memory'));
|
||||
fireEvent.click(screen.getByText('Confirm'));
|
||||
|
||||
// After failure, error banner surfaces + reload re-fetches memories
|
||||
await waitFor(() => {
|
||||
const alerts = screen.getAllByRole('alert');
|
||||
const found = alerts.some((a) => a.textContent?.includes('upstream 502'));
|
||||
expect(found).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── Empty state when no memories at all ────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — empty state', () => {
|
||||
it('renders the "no memories yet" empty state when not searching', async () => {
|
||||
stubFetch([]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('No memories yet')).toBeTruthy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── Refresh ─────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('MemoryInspectorPanel — refresh', () => {
|
||||
it('Refresh button refetches memories', async () => {
|
||||
stubFetch([MEM_BASIC]);
|
||||
render(<MemoryInspectorPanel workspaceId="ws-1" />);
|
||||
await waitFor(() => screen.getByLabelText('Refresh memories'));
|
||||
|
||||
const before = mockGet.mock.calls.filter((c) =>
|
||||
(c[0] as string).includes('/v2/memories'),
|
||||
).length;
|
||||
fireEvent.click(screen.getByLabelText('Refresh memories'));
|
||||
|
||||
await waitFor(() => {
|
||||
const after = mockGet.mock.calls.filter((c) =>
|
||||
(c[0] as string).includes('/v2/memories'),
|
||||
).length;
|
||||
expect(after).toBe(before + 1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -7,7 +7,7 @@ import { api } from "@/lib/api";
|
||||
import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas";
|
||||
import { useSocketEvent } from "@/hooks/useSocketEvent";
|
||||
import { type ChatMessage, type ChatAttachment, createMessage, appendMessageDeduped } from "./chat/types";
|
||||
import { uploadChatFiles, downloadChatFile } from "./chat/uploads";
|
||||
import { uploadChatFiles, downloadChatFile, isPlatformAttachment } from "./chat/uploads";
|
||||
import { AttachmentChip, PendingAttachmentPill } from "./chat/AttachmentViews";
|
||||
import { extractFilesFromTask } from "./chat/message-parser";
|
||||
import { AgentCommsPanel } from "./chat/AgentCommsPanel";
|
||||
@ -1061,7 +1061,77 @@ function MyChatPanel({ workspaceId, data }: Props) {
|
||||
: "dark:prose-invert dark:[--tw-prose-invert-body:theme(colors.zinc.100)] dark:[--tw-prose-invert-headings:theme(colors.white)] dark:[--tw-prose-invert-bold:theme(colors.white)] dark:[--tw-prose-invert-code:theme(colors.zinc.100)]"
|
||||
}`}
|
||||
>
|
||||
<ReactMarkdown remarkPlugins={[remarkGfm]}>{msg.content}</ReactMarkdown>
|
||||
<ReactMarkdown
|
||||
remarkPlugins={[remarkGfm]}
|
||||
components={{
|
||||
// Default ReactMarkdown renders `<a href="...">`
|
||||
// with no target and no scheme handling, so:
|
||||
//
|
||||
// 1. http/https links navigate the canvas tab
|
||||
// itself away — user loses canvas state.
|
||||
// 2. workspace://, file://, and bare /workspace/
|
||||
// paths from agent-authored markdown produce
|
||||
// an unhandled-protocol click → browser ends
|
||||
// up at about:blank with no download (the
|
||||
// reported bug from 2026-05-05).
|
||||
//
|
||||
// Override: external URLs open in a new tab with
|
||||
// rel="noopener noreferrer"; in-container paths
|
||||
// route through downloadChatFile so the browser
|
||||
// gets a real Blob with proper auth headers.
|
||||
a: ({ href, children, ...rest }) => {
|
||||
const url = String(href ?? "");
|
||||
// Use the SSOT helper isPlatformAttachment so
|
||||
// the markdown link override and the chip
|
||||
// download path agree on which schemes need
|
||||
// auth-routed download. Pre-fix this list was
|
||||
// duplicated and missed `platform-pending:`,
|
||||
// producing about:blank for poll-mode uploads.
|
||||
if (isPlatformAttachment(url)) {
|
||||
return (
|
||||
<a
|
||||
href={url}
|
||||
{...rest}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
// Construct a synthetic ChatAttachment
|
||||
// and route through the same
|
||||
// authenticated download path the
|
||||
// download chips use. Filename is the
|
||||
// last path segment so Save-As prefills
|
||||
// sensibly.
|
||||
const name = url.split(/[\\/]/).pop() || "download";
|
||||
downloadChatFile(workspaceId, {
|
||||
uri: url,
|
||||
name,
|
||||
}).catch((err) => {
|
||||
setError(
|
||||
err instanceof Error
|
||||
? `Download failed: ${err.message}`
|
||||
: "Download failed",
|
||||
);
|
||||
});
|
||||
}}
|
||||
>
|
||||
{children}
|
||||
</a>
|
||||
);
|
||||
}
|
||||
// External (http(s) / mailto / unknown scheme):
|
||||
// open in new tab so canvas state survives.
|
||||
return (
|
||||
<a
|
||||
href={url}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
{...rest}
|
||||
>
|
||||
{children}
|
||||
</a>
|
||||
);
|
||||
},
|
||||
}}
|
||||
>{msg.content}</ReactMarkdown>
|
||||
</div>
|
||||
)}
|
||||
{msg.attachments && msg.attachments.length > 0 && (
|
||||
@ -1167,7 +1237,22 @@ function MyChatPanel({ workspaceId, data }: Props) {
|
||||
value={input}
|
||||
onChange={(e) => setInput(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" && !e.shiftKey) {
|
||||
// IME-safe send: while a CJK / Japanese / Korean IME is
|
||||
// composing, Enter accepts the candidate selection — not a
|
||||
// newline, not a send. `e.nativeEvent.isComposing` is the
|
||||
// standard signal (modern WebKit/Blink/Gecko); the keyCode
|
||||
// 229 fallback covers older Safari / WebKit-based mobile
|
||||
// browsers that delay setting isComposing on the
|
||||
// composition-end Enter. Reported 2026-05-05: typing
|
||||
// Chinese with the system IME, pressing Enter to commit
|
||||
// a candidate would inadvertently send the half-typed
|
||||
// message.
|
||||
if (
|
||||
e.key === "Enter" &&
|
||||
!e.shiftKey &&
|
||||
!e.nativeEvent.isComposing &&
|
||||
e.keyCode !== 229
|
||||
) {
|
||||
e.preventDefault();
|
||||
sendMessage();
|
||||
}
|
||||
|
||||
@ -0,0 +1,141 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// Pins two regressions reported on production 2026-05-05:
|
||||
//
|
||||
// 1. IME composition + Enter key: typing Chinese (or any CJK / IME-
|
||||
// composed text) and pressing Enter to commit the candidate
|
||||
// selection used to send the half-typed message. The fix checks
|
||||
// `event.nativeEvent.isComposing` (and a `keyCode === 229`
|
||||
// fallback for older WebKit) before treating Enter as send.
|
||||
//
|
||||
// 2. Markdown link clicks: the agent's ReactMarkdown-rendered links
|
||||
// used to:
|
||||
// - http/https → navigate canvas tab away (user lost canvas state)
|
||||
// - workspace://path / file:///workspace/... / /workspace/... →
|
||||
// browser hit about:blank (unhandled protocol).
|
||||
// Fix: external links get target="_blank" + noopener; in-container
|
||||
// paths route through downloadChatFile (same auth path as chips).
|
||||
|
||||
import { describe, it, expect, vi, afterEach, beforeEach } from "vitest";
|
||||
import { render, screen, cleanup, fireEvent, waitFor } from "@testing-library/react";
|
||||
import React from "react";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
// Mock the api module so render doesn't try to talk to a real CP.
|
||||
const apiGet = vi.fn((_path: string): Promise<unknown> => Promise.resolve([]));
|
||||
const apiPost = vi.fn((_path: string, _body: unknown): Promise<unknown> => Promise.resolve({}));
|
||||
vi.mock("@/lib/api", () => ({
|
||||
api: {
|
||||
get: (path: string) => apiGet(path),
|
||||
post: (path: string, body: unknown) => apiPost(path, body),
|
||||
del: vi.fn(),
|
||||
patch: vi.fn(),
|
||||
put: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/store/canvas", () => ({
|
||||
useCanvasStore: vi.fn((selector?: (s: unknown) => unknown) =>
|
||||
selector ? selector({ agentMessages: {}, consumeAgentMessages: () => [] }) : {},
|
||||
),
|
||||
}));
|
||||
|
||||
// Capture the downloadChatFile call so the markdown-link test can
|
||||
// assert in-container paths route through the authenticated download
|
||||
// path rather than the browser's bare anchor click.
|
||||
const downloadChatFileMock = vi.fn((_workspaceId: string, _att: { uri: string; name: string }) => Promise.resolve());
|
||||
vi.mock("../chat/uploads", async () => {
|
||||
const actual = await vi.importActual<typeof import("../chat/uploads")>("../chat/uploads");
|
||||
return {
|
||||
...actual,
|
||||
downloadChatFile: (workspaceId: string, att: { uri: string; name: string }) =>
|
||||
downloadChatFileMock(workspaceId, att),
|
||||
};
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
apiGet.mockClear();
|
||||
apiPost.mockClear();
|
||||
downloadChatFileMock.mockClear();
|
||||
// jsdom doesn't implement scrollIntoView; ChatTab calls it after
|
||||
// every render with a new message.
|
||||
Element.prototype.scrollIntoView = vi.fn();
|
||||
// Stub IntersectionObserver — the lazy-history sentinel uses it.
|
||||
class FakeIO {
|
||||
observe() {}
|
||||
unobserve() {}
|
||||
disconnect() {}
|
||||
}
|
||||
(window as unknown as { IntersectionObserver: unknown }).IntersectionObserver = FakeIO;
|
||||
(globalThis as unknown as { IntersectionObserver: unknown }).IntersectionObserver = FakeIO;
|
||||
});
|
||||
|
||||
import { ChatTab } from "../ChatTab";
|
||||
|
||||
const minimalData = {
|
||||
status: "online" as const,
|
||||
runtime: "claude-code",
|
||||
currentTask: null,
|
||||
} as unknown as Parameters<typeof ChatTab>[0]["data"];
|
||||
|
||||
describe("ChatTab — IME-safe Enter key", () => {
|
||||
it("does NOT send the message when Enter fires during IME composition (isComposing)", async () => {
|
||||
render(<ChatTab workspaceId="ws-ime" data={minimalData} />);
|
||||
|
||||
// Find the textarea by its aria-label.
|
||||
const textarea = await screen.findByLabelText(/Message to agent/i);
|
||||
fireEvent.change(textarea, { target: { value: "你好" } });
|
||||
|
||||
// Simulate the Enter that commits an IME selection: isComposing=true.
|
||||
fireEvent.keyDown(textarea, { key: "Enter", isComposing: true });
|
||||
|
||||
// sendMessage POSTs via api.post; assert it was NOT called.
|
||||
await waitFor(() => {
|
||||
expect(apiPost).not.toHaveBeenCalled();
|
||||
});
|
||||
// And the input is preserved — ChatTab clears it only on actual send.
|
||||
expect((textarea as HTMLTextAreaElement).value).toBe("你好");
|
||||
});
|
||||
|
||||
it("does NOT send when keyCode is 229 (older Safari IME fallback)", async () => {
|
||||
render(<ChatTab workspaceId="ws-ime2" data={minimalData} />);
|
||||
const textarea = await screen.findByLabelText(/Message to agent/i);
|
||||
fireEvent.change(textarea, { target: { value: "한국어" } });
|
||||
|
||||
// keyCode 229 is the older-Safari signal that an IME is composing.
|
||||
// Some mobile WebKit-based browsers delay setting isComposing on
|
||||
// the composition-end Enter; the keyCode fallback covers that.
|
||||
fireEvent.keyDown(textarea, { key: "Enter", keyCode: 229 });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(apiPost).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("DOES send on a non-composing Enter (the happy path stays intact)", async () => {
|
||||
render(<ChatTab workspaceId="ws-ok" data={minimalData} />);
|
||||
const textarea = await screen.findByLabelText(/Message to agent/i);
|
||||
fireEvent.change(textarea, { target: { value: "hello world" } });
|
||||
|
||||
fireEvent.keyDown(textarea, { key: "Enter" /* no isComposing, no 229 */ });
|
||||
|
||||
// The api.post for /a2a fires inside sendMessage. waitFor since
|
||||
// the call goes through several effects.
|
||||
await waitFor(() => {
|
||||
expect(apiPost).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("Shift+Enter inserts newline regardless (no send)", async () => {
|
||||
render(<ChatTab workspaceId="ws-shift" data={minimalData} />);
|
||||
const textarea = await screen.findByLabelText(/Message to agent/i);
|
||||
fireEvent.change(textarea, { target: { value: "line 1" } });
|
||||
|
||||
fireEvent.keyDown(textarea, { key: "Enter", shiftKey: true });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(apiPost).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -1,220 +0,0 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// Pins the Edit affordance added to MemoryTab. Until this PR the Memory tab
|
||||
// was Add+Delete only; an entry that needed correction had to be deleted and
|
||||
// re-added — losing the version-counter and any in-flight optimistic-locking
|
||||
// invariants other writers depend on.
|
||||
//
|
||||
// Each test pins one branch of the new flow. If any fails, the bug is back.
|
||||
|
||||
import { describe, it, expect, vi, afterEach, beforeEach } from "vitest";
|
||||
import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react";
|
||||
import React from "react";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
const apiGet = vi.fn();
|
||||
const apiPost = vi.fn();
|
||||
const apiDel = vi.fn();
|
||||
vi.mock("@/lib/api", () => ({
|
||||
api: {
|
||||
get: (path: string) => apiGet(path),
|
||||
post: (path: string, body: unknown) => apiPost(path, body),
|
||||
del: (path: string) => apiDel(path),
|
||||
patch: vi.fn(),
|
||||
put: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { MemoryTab } from "../MemoryTab";
|
||||
|
||||
const sampleEntries = [
|
||||
{
|
||||
key: "team_brief",
|
||||
value: { goal: "ship v2" },
|
||||
version: 3,
|
||||
expires_at: null,
|
||||
updated_at: "2026-05-04T10:00:00Z",
|
||||
},
|
||||
{
|
||||
key: "plain_note",
|
||||
value: "raw text note",
|
||||
version: 1,
|
||||
expires_at: "2099-01-01T00:00:00Z",
|
||||
updated_at: "2026-05-04T10:01:00Z",
|
||||
},
|
||||
];
|
||||
|
||||
beforeEach(() => {
|
||||
apiGet.mockReset();
|
||||
apiPost.mockReset();
|
||||
apiDel.mockReset();
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (path === "/workspaces/ws-test/memory") {
|
||||
return Promise.resolve(sampleEntries);
|
||||
}
|
||||
return Promise.reject(new Error(`unmocked api.get: ${path}`));
|
||||
});
|
||||
});
|
||||
|
||||
async function renderAndExpand(key: string) {
|
||||
render(<MemoryTab workspaceId="ws-test" />);
|
||||
await waitFor(() => expect(apiGet).toHaveBeenCalled());
|
||||
// Reveal the Advanced section that hosts the entry list.
|
||||
const showAdvanced = await screen.findByRole("button", { name: "Show" });
|
||||
fireEvent.click(showAdvanced);
|
||||
// Expand the row.
|
||||
const row = await screen.findByRole("button", { name: new RegExp(key) });
|
||||
fireEvent.click(row);
|
||||
}
|
||||
|
||||
describe("MemoryTab Edit affordance", () => {
|
||||
it("Edit button appears once a row is expanded", async () => {
|
||||
await renderAndExpand("team_brief");
|
||||
expect(screen.getAllByRole("button", { name: "Edit" }).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("clicking Edit on a JSON-valued entry pre-fills the textarea with pretty JSON", async () => {
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = (await screen.findByLabelText(
|
||||
"Edit value for team_brief",
|
||||
)) as HTMLTextAreaElement;
|
||||
expect(textarea.value).toBe('{\n "goal": "ship v2"\n}');
|
||||
});
|
||||
|
||||
it("clicking Edit on a string-valued entry pre-fills raw (no surrounding quotes)", async () => {
|
||||
await renderAndExpand("plain_note");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = (await screen.findByLabelText(
|
||||
"Edit value for plain_note",
|
||||
)) as HTMLTextAreaElement;
|
||||
expect(textarea.value).toBe("raw text note");
|
||||
});
|
||||
|
||||
it("Save POSTs with if_match_version + parsed value, then reloads", async () => {
|
||||
apiPost.mockResolvedValue({ status: "ok", key: "team_brief", version: 4 });
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = await screen.findByLabelText("Edit value for team_brief");
|
||||
fireEvent.change(textarea, { target: { value: '{"goal":"ship v3"}' } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
expect(apiPost).toHaveBeenCalledWith("/workspaces/ws-test/memory", {
|
||||
key: "team_brief",
|
||||
value: { goal: "ship v3" },
|
||||
if_match_version: 3,
|
||||
});
|
||||
// Reload after save → second GET.
|
||||
await waitFor(() => expect(apiGet).toHaveBeenCalledTimes(2));
|
||||
});
|
||||
|
||||
it("Save with non-JSON text falls back to plain string", async () => {
|
||||
apiPost.mockResolvedValue({ status: "ok" });
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = await screen.findByLabelText("Edit value for team_brief");
|
||||
fireEvent.change(textarea, { target: { value: "free-form note" } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
expect(apiPost.mock.calls[0][1].value).toBe("free-form note");
|
||||
});
|
||||
|
||||
it("TTL field is forwarded as ttl_seconds when set", async () => {
|
||||
apiPost.mockResolvedValue({ status: "ok" });
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const ttlInput = await screen.findByLabelText("Edit TTL for team_brief");
|
||||
fireEvent.change(ttlInput, { target: { value: "3600" } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
expect(apiPost.mock.calls[0][1].ttl_seconds).toBe(3600);
|
||||
});
|
||||
|
||||
it("blank/zero/non-numeric TTL is omitted from the payload", async () => {
|
||||
apiPost.mockResolvedValue({ status: "ok" });
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const ttlInput = await screen.findByLabelText("Edit TTL for team_brief");
|
||||
// Junk + zero both must drop out — payload must not contain ttl_seconds.
|
||||
fireEvent.change(ttlInput, { target: { value: "abc" } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
expect(apiPost.mock.calls[0][1]).not.toHaveProperty("ttl_seconds");
|
||||
});
|
||||
|
||||
it("Cancel discards edits and restores the rendered value", async () => {
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = await screen.findByLabelText("Edit value for team_brief");
|
||||
fireEvent.change(textarea, { target: { value: '{"goal":"discarded"}' } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
|
||||
expect(apiPost).not.toHaveBeenCalled();
|
||||
// Editor is gone; the JSON pre-block is back.
|
||||
expect(screen.queryByLabelText("Edit value for team_brief")).toBeNull();
|
||||
expect(screen.getAllByText(/"goal": "ship v2"/i).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("409 response surfaces a retry hint and reloads", async () => {
|
||||
apiPost.mockRejectedValueOnce(
|
||||
new Error("HTTP 409: if_match_version mismatch"),
|
||||
);
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = await screen.findByLabelText("Edit value for team_brief");
|
||||
fireEvent.change(textarea, { target: { value: '{"goal":"ship v3"}' } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
const alert = await screen.findByRole("alert");
|
||||
expect(alert.textContent).toMatch(/changed since you opened it/i);
|
||||
// Initial mount load + post-conflict reload.
|
||||
await waitFor(() => expect(apiGet).toHaveBeenCalledTimes(2));
|
||||
});
|
||||
|
||||
it("non-409 error surfaces the message and does not reload", async () => {
|
||||
apiPost.mockRejectedValueOnce(new Error("boom"));
|
||||
await renderAndExpand("team_brief");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
const alert = await screen.findByRole("alert");
|
||||
expect(alert.textContent).toBe("boom");
|
||||
// Only the initial mount load — no retry reload.
|
||||
expect(apiGet).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("entry with no version omits if_match_version (back-compat with older shape)", async () => {
|
||||
// Pre-version-counter shape: drop the `version` field from the row.
|
||||
apiGet.mockReset();
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (path === "/workspaces/ws-test/memory") {
|
||||
return Promise.resolve([
|
||||
{
|
||||
key: "old_entry",
|
||||
value: "legacy",
|
||||
expires_at: null,
|
||||
updated_at: "2026-05-04T10:00:00Z",
|
||||
},
|
||||
]);
|
||||
}
|
||||
return Promise.reject(new Error(`unmocked: ${path}`));
|
||||
});
|
||||
apiPost.mockResolvedValue({ status: "ok" });
|
||||
|
||||
await renderAndExpand("old_entry");
|
||||
fireEvent.click(screen.getAllByRole("button", { name: "Edit" })[0]);
|
||||
const textarea = await screen.findByLabelText("Edit value for old_entry");
|
||||
fireEvent.change(textarea, { target: { value: "updated" } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
await waitFor(() => expect(apiPost).toHaveBeenCalledTimes(1));
|
||||
const payload = apiPost.mock.calls[0][1];
|
||||
expect(payload).not.toHaveProperty("if_match_version");
|
||||
expect(payload.value).toBe("updated");
|
||||
});
|
||||
});
|
||||
@ -44,6 +44,8 @@ export async function uploadChatFiles(
|
||||
* - `workspace:<abs-path>` (our canonical form)
|
||||
* - `file:///workspace/...` (some agents emit this)
|
||||
* - `/workspace/...` (bare absolute path inside the container)
|
||||
* - `platform-pending:<wsid>/<file_id>` (poll-mode upload, staged
|
||||
* on platform side; resolves to /pending-uploads/<file_id>/content)
|
||||
* Everything that looks like an allowed-root container path is
|
||||
* rewritten to the authenticated /chat/download endpoint. HTTP(S)
|
||||
* URIs pass through unchanged so we can also render links to
|
||||
@ -53,6 +55,35 @@ export function resolveAttachmentHref(
|
||||
workspaceId: string,
|
||||
uri: string,
|
||||
): string {
|
||||
// platform-pending: agents-emitted URI that lives in the platform-side
|
||||
// staging layer (poll-mode chat uploads, see workspace-server's
|
||||
// chat_files.go ~line 690 + pendinguploads.Storage). The wire shape
|
||||
// is `platform-pending:<workspace_id>/<file_id>`. Resolving it
|
||||
// requires hitting GET /workspaces/<wsid>/pending-uploads/<file_id>/content
|
||||
// which streams the bytes with full workspace auth. Without this
|
||||
// case the browser sees an unhandled-protocol click → about:blank,
|
||||
// which was the user-visible bug from 2026-05-05 (reno-stars).
|
||||
if (uri.startsWith("platform-pending:")) {
|
||||
const rest = uri.slice("platform-pending:".length);
|
||||
const slash = rest.indexOf("/");
|
||||
// Defensive: if the URI doesn't have the expected wsid/fileid
|
||||
// shape, fall through to raw-URI handling so the consumer can
|
||||
// still try to render it (rather than producing a broken /pending-
|
||||
// uploads/// path).
|
||||
if (slash > 0) {
|
||||
const wsid = rest.slice(0, slash);
|
||||
const fileID = rest.slice(slash + 1);
|
||||
if (wsid && fileID) {
|
||||
// Use the URI's own workspace_id (the bytes live in THAT
|
||||
// workspace's pending-uploads store), not the chat's
|
||||
// workspace_id — these CAN differ when a user drags a file
|
||||
// into one workspace's chat that gets forwarded to another
|
||||
// (cross-workspace delegation, agent forwarding).
|
||||
return `${PLATFORM_URL}/workspaces/${wsid}/pending-uploads/${fileID}/content`;
|
||||
}
|
||||
}
|
||||
return uri;
|
||||
}
|
||||
const containerPath = normalizeWorkspaceUri(uri);
|
||||
if (containerPath) {
|
||||
return `${PLATFORM_URL}/workspaces/${workspaceId}/chat/download?path=${encodeURIComponent(containerPath)}`;
|
||||
@ -60,6 +91,14 @@ export function resolveAttachmentHref(
|
||||
return uri;
|
||||
}
|
||||
|
||||
/** Returns true when the URI points at a platform-side resource that
|
||||
* requires our auth headers — caller should route through
|
||||
* downloadChatFile rather than letting the browser navigate. */
|
||||
export function isPlatformAttachment(uri: string): boolean {
|
||||
if (uri.startsWith("platform-pending:")) return true;
|
||||
return normalizeWorkspaceUri(uri) !== null;
|
||||
}
|
||||
|
||||
/** Extracts the absolute container path from a workspace-scoped URI,
|
||||
* or null if the URI isn't a container path. The matching roots
|
||||
* mirror the server's `allowedRoots` allowlist. */
|
||||
@ -96,8 +135,7 @@ export async function downloadChatFile(
|
||||
attachment: ChatAttachment,
|
||||
): Promise<void> {
|
||||
const href = resolveAttachmentHref(workspaceId, attachment.uri);
|
||||
const isContainerPath = normalizeWorkspaceUri(attachment.uri) !== null;
|
||||
if (!isContainerPath) {
|
||||
if (!isPlatformAttachment(attachment.uri)) {
|
||||
// External URL — let the browser navigate. Opens in new tab so
|
||||
// the canvas context survives a navigation. `href` here is the
|
||||
// raw URI (http(s), or anything else the agent sent back).
|
||||
|
||||
@ -80,6 +80,7 @@ TOP_LEVEL_MODULES = {
|
||||
"internal_file_read",
|
||||
"main",
|
||||
"mcp_cli",
|
||||
"mcp_doctor",
|
||||
"mcp_heartbeat",
|
||||
"mcp_inbox_pollers",
|
||||
"mcp_workspace_resolver",
|
||||
|
||||
125
workspace-server/internal/events/types.go
Normal file
125
workspace-server/internal/events/types.go
Normal file
@ -0,0 +1,125 @@
|
||||
package events
|
||||
|
||||
// types.go — typed taxonomy of WebSocket event names emitted by the
|
||||
// workspace-server.
|
||||
//
|
||||
// RFC #2945 PR-B. Pre-consolidation, every BroadcastOnly /
|
||||
// RecordAndBroadcast call site passed a bare string literal:
|
||||
//
|
||||
// h.broadcaster.BroadcastOnly(workspaceID, "AGENT_MESSAGE", payload)
|
||||
//
|
||||
// Producers (Go workspace-server, ~30 call sites across handlers/,
|
||||
// scheduler/, registry/, bundle/) and consumers (canvas TS store +
|
||||
// component listeners) duplicated the same string with no shared
|
||||
// definition. A producer renaming an event silently broke every
|
||||
// consumer — same drift class that produced the reno-stars data-loss
|
||||
// regression on the persistence side. The fix on that side was the
|
||||
// AgentMessageWriter SSOT (PR-A); the fix on this side is named
|
||||
// constants.
|
||||
//
|
||||
// Why a typed string (not a plain enum / iota): the event name
|
||||
// crosses the wire to TypeScript consumers as the literal string in
|
||||
// `WSMessage.Event`. Iota integers would break the canvas store's
|
||||
// switch (`case "AGENT_MESSAGE":`); a typed string preserves the
|
||||
// wire contract while giving Go callers compile-time discipline.
|
||||
//
|
||||
// Mirror in canvas: a parity gate (PR-B-2 follow-up) will assert this
|
||||
// constant set ≡ the TypeScript union members in
|
||||
// `canvas/src/lib/ws-events.ts`. Today the canvas consumes the names
|
||||
// via bare-string comparisons; the mirror lands separately to keep
|
||||
// PR-B narrow.
|
||||
|
||||
// EventType is the wire-typed name of a WebSocket event the platform
|
||||
// broadcasts. Always emit constants from this file rather than bare
|
||||
// strings — the AST gate in events_types_drift_test.go guards
|
||||
// against bare-string usage in the broadcaster surfaces.
|
||||
type EventType string
|
||||
|
||||
// Event constants — the canonical taxonomy. New events MUST be added
|
||||
// here AND mirrored in canvas/src/lib/ws-events.ts (parity gate
|
||||
// pending in PR-B-2). Group by semantic family so the list stays
|
||||
// scan-friendly as it grows.
|
||||
const (
|
||||
// Chat / agent messaging — surfaces in canvas chat panels.
|
||||
EventAgentMessage EventType = "AGENT_MESSAGE"
|
||||
EventA2AResponse EventType = "A2A_RESPONSE"
|
||||
EventActivityLogged EventType = "ACTIVITY_LOGGED"
|
||||
EventChannelMessage EventType = "CHANNEL_MESSAGE"
|
||||
|
||||
// Workspace lifecycle.
|
||||
EventWorkspaceProvisioning EventType = "WORKSPACE_PROVISIONING"
|
||||
EventWorkspaceProvisionFailed EventType = "WORKSPACE_PROVISION_FAILED"
|
||||
EventWorkspaceOnline EventType = "WORKSPACE_ONLINE"
|
||||
EventWorkspaceOffline EventType = "WORKSPACE_OFFLINE"
|
||||
EventWorkspaceDegraded EventType = "WORKSPACE_DEGRADED"
|
||||
EventWorkspaceHibernated EventType = "WORKSPACE_HIBERNATED"
|
||||
EventWorkspacePaused EventType = "WORKSPACE_PAUSED"
|
||||
EventWorkspaceRemoved EventType = "WORKSPACE_REMOVED"
|
||||
EventWorkspaceAwaitingAgent EventType = "WORKSPACE_AWAITING_AGENT"
|
||||
EventWorkspaceHeartbeat EventType = "WORKSPACE_HEARTBEAT"
|
||||
|
||||
// Agent assignment + identity.
|
||||
EventAgentAssigned EventType = "AGENT_ASSIGNED"
|
||||
EventAgentReplaced EventType = "AGENT_REPLACED"
|
||||
EventAgentRemoved EventType = "AGENT_REMOVED"
|
||||
EventAgentMoved EventType = "AGENT_MOVED"
|
||||
EventAgentCardUpdated EventType = "AGENT_CARD_UPDATED"
|
||||
|
||||
// Delegation lifecycle.
|
||||
EventDelegationSent EventType = "DELEGATION_SENT"
|
||||
EventDelegationStatus EventType = "DELEGATION_STATUS"
|
||||
EventDelegationComplete EventType = "DELEGATION_COMPLETE"
|
||||
EventDelegationFailed EventType = "DELEGATION_FAILED"
|
||||
|
||||
// Task progression + scheduler.
|
||||
EventTaskUpdated EventType = "TASK_UPDATED"
|
||||
EventCronExecuted EventType = "CRON_EXECUTED"
|
||||
EventCronSkipped EventType = "CRON_SKIPPED"
|
||||
|
||||
// Approvals.
|
||||
EventApprovalRequested EventType = "APPROVAL_REQUESTED"
|
||||
EventApprovalEscalated EventType = "APPROVAL_ESCALATED"
|
||||
|
||||
// Auth / credentials.
|
||||
EventExternalCredentialsRotated EventType = "EXTERNAL_CREDENTIALS_ROTATED"
|
||||
)
|
||||
|
||||
// AllEventTypes lists every constant in this file. Used by the
|
||||
// snapshot test (events_types_drift_test.go) to detect when a new
|
||||
// constant is added without updating the snapshot — the catch-up
|
||||
// step is mirroring the addition into canvas/src/lib/ws-events.ts so
|
||||
// canvas consumers can switch on it.
|
||||
//
|
||||
// Keep in lexicographic order so the snapshot diff is stable on
|
||||
// renames and the parity-with-TS comparison is order-independent.
|
||||
var AllEventTypes = []EventType{
|
||||
EventA2AResponse,
|
||||
EventActivityLogged,
|
||||
EventAgentAssigned,
|
||||
EventAgentCardUpdated,
|
||||
EventAgentMessage,
|
||||
EventAgentMoved,
|
||||
EventAgentRemoved,
|
||||
EventAgentReplaced,
|
||||
EventApprovalEscalated,
|
||||
EventApprovalRequested,
|
||||
EventChannelMessage,
|
||||
EventCronExecuted,
|
||||
EventCronSkipped,
|
||||
EventDelegationComplete,
|
||||
EventDelegationFailed,
|
||||
EventDelegationSent,
|
||||
EventDelegationStatus,
|
||||
EventExternalCredentialsRotated,
|
||||
EventTaskUpdated,
|
||||
EventWorkspaceAwaitingAgent,
|
||||
EventWorkspaceDegraded,
|
||||
EventWorkspaceHeartbeat,
|
||||
EventWorkspaceHibernated,
|
||||
EventWorkspaceOffline,
|
||||
EventWorkspaceOnline,
|
||||
EventWorkspacePaused,
|
||||
EventWorkspaceProvisionFailed,
|
||||
EventWorkspaceProvisioning,
|
||||
EventWorkspaceRemoved,
|
||||
}
|
||||
117
workspace-server/internal/events/types_test.go
Normal file
117
workspace-server/internal/events/types_test.go
Normal file
@ -0,0 +1,117 @@
|
||||
package events
|
||||
|
||||
import (
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestAllEventTypes_IsSnapshot pins the canonical event taxonomy.
|
||||
// Adding a new constant in types.go without updating AllEventTypes
|
||||
// (or vice versa) fails this test.
|
||||
//
|
||||
// The snapshot is also the authoritative input to the canvas-side
|
||||
// parity gate (PR-B-2 follow-up): the TypeScript union members in
|
||||
// canvas/src/lib/ws-events.ts MUST match this list exactly. A drift
|
||||
// gate at CI time will assert set equality once the TS file lands.
|
||||
func TestAllEventTypes_IsSnapshot(t *testing.T) {
|
||||
// Every named constant must appear in AllEventTypes. Walk via
|
||||
// reflection over the package-level vars would over-include test
|
||||
// fixtures, so list the canonical names here. When a constant
|
||||
// is added in types.go, append the EventType's literal value
|
||||
// to the expected list below — the failure message names
|
||||
// exactly what's missing so the diff is one-line obvious.
|
||||
expected := []string{
|
||||
"A2A_RESPONSE",
|
||||
"ACTIVITY_LOGGED",
|
||||
"AGENT_ASSIGNED",
|
||||
"AGENT_CARD_UPDATED",
|
||||
"AGENT_MESSAGE",
|
||||
"AGENT_MOVED",
|
||||
"AGENT_REMOVED",
|
||||
"AGENT_REPLACED",
|
||||
"APPROVAL_ESCALATED",
|
||||
"APPROVAL_REQUESTED",
|
||||
"CHANNEL_MESSAGE",
|
||||
"CRON_EXECUTED",
|
||||
"CRON_SKIPPED",
|
||||
"DELEGATION_COMPLETE",
|
||||
"DELEGATION_FAILED",
|
||||
"DELEGATION_SENT",
|
||||
"DELEGATION_STATUS",
|
||||
"EXTERNAL_CREDENTIALS_ROTATED",
|
||||
"TASK_UPDATED",
|
||||
"WORKSPACE_AWAITING_AGENT",
|
||||
"WORKSPACE_DEGRADED",
|
||||
"WORKSPACE_HEARTBEAT",
|
||||
"WORKSPACE_HIBERNATED",
|
||||
"WORKSPACE_OFFLINE",
|
||||
"WORKSPACE_ONLINE",
|
||||
"WORKSPACE_PAUSED",
|
||||
"WORKSPACE_PROVISIONING",
|
||||
"WORKSPACE_PROVISION_FAILED",
|
||||
"WORKSPACE_REMOVED",
|
||||
}
|
||||
sort.Strings(expected)
|
||||
|
||||
actual := make([]string, 0, len(AllEventTypes))
|
||||
for _, e := range AllEventTypes {
|
||||
actual = append(actual, string(e))
|
||||
}
|
||||
sort.Strings(actual)
|
||||
|
||||
if len(actual) != len(expected) {
|
||||
t.Errorf("AllEventTypes count = %d, want %d\nactual: %s\nexpected: %s",
|
||||
len(actual), len(expected),
|
||||
strings.Join(actual, ", "),
|
||||
strings.Join(expected, ", "))
|
||||
return
|
||||
}
|
||||
for i, want := range expected {
|
||||
if actual[i] != want {
|
||||
t.Errorf("AllEventTypes[%d] = %q, want %q (full diff:\n actual: %v\n expected: %v\n)",
|
||||
i, actual[i], want, actual, expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestEventType_NoEmptyConstants pins that no constant declared in
|
||||
// types.go has an accidentally-empty value. The catch is the
|
||||
// "WORKSPACE_X" → forgot-to-fill pattern: a typo in the literal
|
||||
// would surface as the empty string, and broadcast pipelines would
|
||||
// silently filter empty-name events without any error signal.
|
||||
func TestEventType_NoEmptyConstants(t *testing.T) {
|
||||
for _, e := range AllEventTypes {
|
||||
if string(e) == "" {
|
||||
t.Errorf("found empty EventType in AllEventTypes — typo in types.go?")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestEventType_AllUppercaseSnakeCase pins the wire format. Mixed
|
||||
// case or kebab-case would break the canvas TypeScript switch
|
||||
// statements (every consumer's `case "AGENT_MESSAGE":` is upper-
|
||||
// snake). The check is the catch for an accidental
|
||||
// `"agent_message"` typo that wouldn't fail the snapshot gate.
|
||||
func TestEventType_AllUppercaseSnakeCase(t *testing.T) {
|
||||
for _, e := range AllEventTypes {
|
||||
s := string(e)
|
||||
// Allowed chars: A-Z, 0-9, _ — nothing else, no leading/
|
||||
// trailing underscores, no consecutive underscores.
|
||||
if s != strings.ToUpper(s) {
|
||||
t.Errorf("EventType %q is not all-uppercase — wire format requires upper-snake", s)
|
||||
}
|
||||
if strings.HasPrefix(s, "_") || strings.HasSuffix(s, "_") {
|
||||
t.Errorf("EventType %q has leading/trailing underscore — disallowed", s)
|
||||
}
|
||||
if strings.Contains(s, "__") {
|
||||
t.Errorf("EventType %q has consecutive underscores — disallowed", s)
|
||||
}
|
||||
for _, r := range s {
|
||||
if !((r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_') {
|
||||
t.Errorf("EventType %q contains disallowed char %q", s, r)
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -465,78 +465,30 @@ func (h *ActivityHandler) Notify(c *gin.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
// Verify workspace exists
|
||||
var wsName string
|
||||
err := db.DB.QueryRowContext(c.Request.Context(),
|
||||
`SELECT name FROM workspaces WHERE id = $1 AND status != 'removed'`, workspaceID,
|
||||
).Scan(&wsName)
|
||||
if err != nil {
|
||||
// Single source of truth for chat-bearing agent → user messages —
|
||||
// see agent_message_writer.go for the contract. Pre-RFC-#2945, the
|
||||
// broadcast + INSERT pair was inlined here and again in
|
||||
// mcp_tools.go's send_message_to_user, and the duplication is what
|
||||
// produced the reno-stars data-loss regression. Both paths now
|
||||
// route through the same writer; future channels (Slack, Discord,
|
||||
// Lark) hook in here too.
|
||||
attachments := make([]AgentMessageAttachment, 0, len(body.Attachments))
|
||||
for _, a := range body.Attachments {
|
||||
attachments = append(attachments, AgentMessageAttachment{
|
||||
URI: a.URI,
|
||||
Name: a.Name,
|
||||
MimeType: a.MimeType,
|
||||
Size: a.Size,
|
||||
})
|
||||
}
|
||||
writer := NewAgentMessageWriter(db.DB, h.broadcaster)
|
||||
if err := writer.Send(c.Request.Context(), workspaceID, body.Message, attachments); err != nil {
|
||||
if errors.Is(err, ErrWorkspaceNotFound) {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"})
|
||||
return
|
||||
}
|
||||
|
||||
broadcastPayload := map[string]interface{}{
|
||||
"message": body.Message,
|
||||
"workspace_id": workspaceID,
|
||||
"name": wsName,
|
||||
}
|
||||
if len(body.Attachments) > 0 {
|
||||
broadcastPayload["attachments"] = body.Attachments
|
||||
}
|
||||
h.broadcaster.BroadcastOnly(workspaceID, "AGENT_MESSAGE", broadcastPayload)
|
||||
|
||||
// Persist to activity_logs so the chat history loader restores this
|
||||
// message after a page reload. Pre-fix, send_message_to_user pushes
|
||||
// were broadcast-only — survived the WebSocket session but vanished
|
||||
// when the user refreshed because nothing wrote them to the DB.
|
||||
//
|
||||
// Shape chosen to match the existing loader query
|
||||
// (`type=a2a_receive&source=canvas`):
|
||||
// - activity_type='a2a_receive' so it joins the same query path
|
||||
// - source_id=NULL so the canvas-source filter accepts it
|
||||
// - method='notify' to distinguish from real A2A receives in audits
|
||||
// - request_body=NULL so the loader doesn't append a duplicate
|
||||
// "user message" bubble for it
|
||||
// - response_body={"result": "<text>"} matches extractResponseText's
|
||||
// simplest branch ({result: string} → take verbatim)
|
||||
//
|
||||
// Errors are logged-only — broadcast already succeeded, the user
|
||||
// sees the message; persistence failure just means the message
|
||||
// won't survive reload (pre-fix behavior). Don't fail the whole
|
||||
// notify on a DB hiccup.
|
||||
// response_body shape — chosen to feed BOTH:
|
||||
// - extractResponseText: looks at body.result (string) and returns it
|
||||
// - extractFilesFromTask: looks at body.parts[] for kind=file
|
||||
// so a chat reload after a notify-with-attachments restores both
|
||||
// the text bubble AND the download chips.
|
||||
respPayload := map[string]interface{}{"result": body.Message}
|
||||
if len(body.Attachments) > 0 {
|
||||
fileParts := make([]map[string]interface{}, 0, len(body.Attachments))
|
||||
for _, a := range body.Attachments {
|
||||
fileMeta := map[string]interface{}{"uri": a.URI, "name": a.Name}
|
||||
if a.MimeType != "" {
|
||||
fileMeta["mimeType"] = a.MimeType
|
||||
}
|
||||
if a.Size > 0 {
|
||||
fileMeta["size"] = a.Size
|
||||
}
|
||||
fileParts = append(fileParts, map[string]interface{}{
|
||||
"kind": "file",
|
||||
"file": fileMeta,
|
||||
})
|
||||
}
|
||||
respPayload["parts"] = fileParts
|
||||
}
|
||||
respJSON, _ := json.Marshal(respPayload)
|
||||
preview := body.Message
|
||||
if len(preview) > 80 {
|
||||
preview = preview[:80] + "…"
|
||||
}
|
||||
if _, err := db.DB.ExecContext(c.Request.Context(), `
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, summary, response_body, status)
|
||||
VALUES ($1, 'a2a_receive', 'notify', $2, $3::jsonb, 'ok')
|
||||
`, workspaceID, "Agent message: "+preview, string(respJSON)); err != nil {
|
||||
log.Printf("Notify: failed to persist message for %s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "internal error"})
|
||||
return
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "sent"})
|
||||
|
||||
203
workspace-server/internal/handlers/agent_message_writer.go
Normal file
203
workspace-server/internal/handlers/agent_message_writer.go
Normal file
@ -0,0 +1,203 @@
|
||||
package handlers
|
||||
|
||||
// AgentMessageWriter is the SSOT for "agent → user" message delivery in the
|
||||
// workspace-server. Every chat-bearing path that surfaces a message to the
|
||||
// canvas — HTTP /notify (Notify handler), MCP tools/call
|
||||
// send_message_to_user (toolSendMessageToUser), any future channel — MUST
|
||||
// route through this writer rather than re-implement the broadcast +
|
||||
// persist contract inline.
|
||||
//
|
||||
// Why: pre-consolidation, two handlers duplicated the same "broadcast then
|
||||
// INSERT activity_logs" sequence. The reno-stars production data-loss
|
||||
// incident (2026-05-05, RFC #2945, PR #2944) was the symptom — the
|
||||
// persistence half landed for /notify but lagged for the MCP bridge by
|
||||
// months, silently dropping every long-form external-agent message until
|
||||
// reload. The AST gate from #2944 catches drift; this writer eliminates
|
||||
// the *possibility* of drift by giving both call sites a single
|
||||
// well-tested function to call.
|
||||
//
|
||||
// Contract:
|
||||
// 1. Look up the workspace by id; ErrWorkspaceNotFound on miss so the
|
||||
// caller can return 404 with a clean message.
|
||||
// 2. Broadcast a WS AGENT_MESSAGE event with {message, workspace_id,
|
||||
// name, attachments?}.
|
||||
// 3. INSERT a row into activity_logs:
|
||||
// type='a2a_receive', method='notify', source_id NULL,
|
||||
// response_body={"result": message[, "parts": [file kind...]]},
|
||||
// status='ok'
|
||||
// Best-effort — INSERT failure logs only, returns nil so the broadcast
|
||||
// success isn't undone on the caller side.
|
||||
// 4. Returns nil on success.
|
||||
//
|
||||
// The shape (especially the JSON response_body) is the wire contract the
|
||||
// canvas's chat-history hydrator (canvas/src/.../historyHydration.ts)
|
||||
// reads. Drift here silently breaks chat replay across all consumers, so
|
||||
// changes to the JSON shape MUST be cross-verified against the hydrator
|
||||
// in the same PR.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/events"
|
||||
)
|
||||
|
||||
// ErrWorkspaceNotFound is returned by AgentMessageWriter.Send when the
|
||||
// workspace lookup turns up nothing (or the workspace is in
|
||||
// status='removed'). Callers translate to HTTP 404 / JSON-RPC error /
|
||||
// whatever surface they expose. Real DB errors (connection drop, query
|
||||
// timeout) surface as wrapped errors and should be treated as 503.
|
||||
var ErrWorkspaceNotFound = errors.New("agent_message: workspace not found")
|
||||
|
||||
// truncatePreviewRunes returns at most maxRunes runes of s, plus an ellipsis
|
||||
// when truncated. Operates on the rune (codepoint) boundary instead of
|
||||
// byte indices — the previous byte-slice version produced invalid UTF-8
|
||||
// when maxRunes landed mid-codepoint (CJK, emoji, accented characters
|
||||
// in agent-authored chat messages), and Postgres JSONB rejects invalid
|
||||
// UTF-8, dropping the activity_log INSERT silently. The persistence
|
||||
// failure log fires but the message vanishes from chat history — the
|
||||
// exact regression class the SSOT consolidation was built to prevent.
|
||||
//
|
||||
// maxRunes is in runes, not bytes — `truncatePreviewRunes("你好", 1)` returns
|
||||
// `"你…"`, not `"\xe4…"`. Set the cap on a UI-friendly basis (visible
|
||||
// character count, not stored byte count); 80 runes covers the
|
||||
// activity_logs.summary column comfortably.
|
||||
func truncatePreviewRunes(s string, maxRunes int) string {
|
||||
if utf8.RuneCountInString(s) <= maxRunes {
|
||||
return s
|
||||
}
|
||||
// Walk runes until we've consumed maxRunes; cut at that byte index.
|
||||
count := 0
|
||||
cut := len(s)
|
||||
for i := range s {
|
||||
if count == maxRunes {
|
||||
cut = i
|
||||
break
|
||||
}
|
||||
count++
|
||||
}
|
||||
return s[:cut] + "…"
|
||||
}
|
||||
|
||||
// AgentMessageAttachment is one file attached to an agent → user
|
||||
// message. Identical to handlers.NotifyAttachment in field set; kept
|
||||
// distinct so the writer's API doesn't import a handler type with HTTP
|
||||
// binding tags.
|
||||
type AgentMessageAttachment struct {
|
||||
URI string
|
||||
Name string
|
||||
MimeType string
|
||||
Size int64
|
||||
}
|
||||
|
||||
// AgentMessageWriter persists + broadcasts agent → user messages. Construct
|
||||
// once per process via NewAgentMessageWriter; pass the same instance to
|
||||
// every handler that delivers chat (Notify, toolSendMessageToUser, etc.).
|
||||
//
|
||||
// Takes events.EventEmitter (not the *Broadcaster concrete type) so tests
|
||||
// can substitute a fake emitter and producers in other packages can wrap
|
||||
// the real broadcaster behind their own metrics / retries without leaking
|
||||
// the concrete dependency.
|
||||
type AgentMessageWriter struct {
|
||||
db *sql.DB
|
||||
broadcaster events.EventEmitter
|
||||
}
|
||||
|
||||
// NewAgentMessageWriter binds the writer to the platform's DB pool +
|
||||
// WebSocket broadcaster.
|
||||
func NewAgentMessageWriter(db *sql.DB, broadcaster events.EventEmitter) *AgentMessageWriter {
|
||||
return &AgentMessageWriter{db: db, broadcaster: broadcaster}
|
||||
}
|
||||
|
||||
// Send delivers a single agent → user message. Look up + broadcast +
|
||||
// persist in that order; ErrWorkspaceNotFound short-circuits before any
|
||||
// broadcast or DB write so callers can 404 cleanly.
|
||||
//
|
||||
// Returns nil on success — including on DB-INSERT failure (the broadcast
|
||||
// already returned successfully and the user has seen the message; the
|
||||
// persistence-failure mode is logged at WARN but the caller's response
|
||||
// stays 200 so the agent doesn't retry and double-broadcast).
|
||||
func (w *AgentMessageWriter) Send(
|
||||
ctx context.Context,
|
||||
workspaceID, message string,
|
||||
attachments []AgentMessageAttachment,
|
||||
) error {
|
||||
// 1. Workspace lookup. status='removed' filter is the same shape /notify
|
||||
// used pre-consolidation; deleted workspaces don't get notifications.
|
||||
//
|
||||
// Distinguish sql.ErrNoRows ("workspace genuinely not present" — caller
|
||||
// should 404) from real DB errors (connection drop, statement timeout,
|
||||
// pool exhaustion — caller should 503). Pre-fix this branch returned
|
||||
// ErrWorkspaceNotFound for any error, so during a DB outage every
|
||||
// notify call surfaced as "workspace not found" and masked real
|
||||
// incidents in the alert path.
|
||||
var wsName string
|
||||
err := w.db.QueryRowContext(ctx,
|
||||
`SELECT name FROM workspaces WHERE id = $1 AND status != 'removed'`,
|
||||
workspaceID,
|
||||
).Scan(&wsName)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return ErrWorkspaceNotFound
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("agent_message: workspace lookup: %w", err)
|
||||
}
|
||||
|
||||
// 2. Build broadcast payload + WS-emit. Same shape that ChatTab's
|
||||
// AGENT_MESSAGE handler in canvas/src/store/canvas-events.ts has
|
||||
// consumed since the canvas chat shipped — drift here would orphan
|
||||
// every live chat panel.
|
||||
broadcastPayload := map[string]interface{}{
|
||||
"message": message,
|
||||
"workspace_id": workspaceID,
|
||||
"name": wsName,
|
||||
}
|
||||
if len(attachments) > 0 {
|
||||
broadcastPayload["attachments"] = attachments
|
||||
}
|
||||
w.broadcaster.BroadcastOnly(workspaceID, string(events.EventAgentMessage), broadcastPayload)
|
||||
|
||||
// 3. Persist for chat-history hydration. response_body shape MUST stay
|
||||
// in sync with extractResponseText + extractFilesFromTask in
|
||||
// canvas/src/components/tabs/chat/historyHydration.ts:
|
||||
// - extractResponseText reads body.result (string) → renders text
|
||||
// - extractFilesFromTask reads body.parts[] (kind=file) → renders chips
|
||||
respPayload := map[string]interface{}{"result": message}
|
||||
if len(attachments) > 0 {
|
||||
fileParts := make([]map[string]interface{}, 0, len(attachments))
|
||||
for _, a := range attachments {
|
||||
fileMeta := map[string]interface{}{"uri": a.URI, "name": a.Name}
|
||||
if a.MimeType != "" {
|
||||
fileMeta["mimeType"] = a.MimeType
|
||||
}
|
||||
if a.Size > 0 {
|
||||
fileMeta["size"] = a.Size
|
||||
}
|
||||
fileParts = append(fileParts, map[string]interface{}{
|
||||
"kind": "file",
|
||||
"file": fileMeta,
|
||||
})
|
||||
}
|
||||
respPayload["parts"] = fileParts
|
||||
}
|
||||
respJSON, _ := json.Marshal(respPayload)
|
||||
preview := truncatePreviewRunes(message, 80)
|
||||
if _, err := w.db.ExecContext(ctx, `
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, summary, response_body, status)
|
||||
VALUES ($1, 'a2a_receive', 'notify', $2, $3::jsonb, 'ok')
|
||||
`, workspaceID, "Agent message: "+preview, string(respJSON)); err != nil {
|
||||
// Best-effort: the broadcast already returned ok and the user
|
||||
// has seen the message. Logging a structured line lets operators
|
||||
// notice persistence-failure rates spike if the DB is unhealthy,
|
||||
// without breaking the tool response or causing the agent to
|
||||
// retry-and-double-broadcast.
|
||||
log.Printf("agent_message: failed to persist for %s: %v", workspaceID, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
448
workspace-server/internal/handlers/agent_message_writer_test.go
Normal file
448
workspace-server/internal/handlers/agent_message_writer_test.go
Normal file
@ -0,0 +1,448 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql/driver"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
)
|
||||
|
||||
// AgentMessageWriter is the SSOT for agent → user chat delivery
|
||||
// (RFC #2945 PR-A). These tests pin the contract the writer
|
||||
// guarantees: workspace lookup, broadcast, INSERT, error semantics —
|
||||
// every shape that producers (Notify, toolSendMessageToUser, future
|
||||
// channels) rely on.
|
||||
//
|
||||
// Pre-consolidation, the broadcast-then-INSERT logic was duplicated
|
||||
// across two handlers and they drifted (reno-stars, 2026-05-05). With
|
||||
// the writer being the only place this logic lives, these tests are
|
||||
// the regression line for every chat-bearing path simultaneously.
|
||||
|
||||
// jsonMatcher is a sqlmock Argument matcher that decodes the actual
|
||||
// SQL arg as JSON and runs a caller-supplied predicate over the
|
||||
// resulting structure. Tighter than substring matching (which can
|
||||
// false-pass on a renamed key) and tolerant of map-key ordering
|
||||
// (which exact-string matching is not).
|
||||
type jsonMatcher struct {
|
||||
predicate func(parsed map[string]any) bool
|
||||
desc string
|
||||
}
|
||||
|
||||
func (m jsonMatcher) Match(v driver.Value) bool {
|
||||
s, ok := v.(string)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
var parsed map[string]any
|
||||
if err := json.Unmarshal([]byte(s), &parsed); err != nil {
|
||||
return false
|
||||
}
|
||||
return m.predicate(parsed)
|
||||
}
|
||||
|
||||
// stringMatcher pins exact prefix/suffix/equality checks against a
|
||||
// driver.Value that's actually a string.
|
||||
type stringMatcher func(string) bool
|
||||
|
||||
func (f stringMatcher) Match(v driver.Value) bool {
|
||||
s, ok := v.(string)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return f(s)
|
||||
}
|
||||
|
||||
// capturingEmitter records every BroadcastOnly call so tests can pin
|
||||
// the WS event shape without a real ws.Hub. RecordAndBroadcast is
|
||||
// also captured for completeness — the writer doesn't call it today,
|
||||
// but a future producer might, and a captured-but-unasserted record
|
||||
// is easier to diagnose than a nil panic.
|
||||
type capturingEmitter struct {
|
||||
events []capturedEvent
|
||||
}
|
||||
|
||||
type capturedEvent struct {
|
||||
workspaceID string
|
||||
eventType string
|
||||
payload interface{}
|
||||
}
|
||||
|
||||
func (c *capturingEmitter) BroadcastOnly(workspaceID string, eventType string, payload interface{}) {
|
||||
c.events = append(c.events, capturedEvent{workspaceID, eventType, payload})
|
||||
}
|
||||
|
||||
func (c *capturingEmitter) RecordAndBroadcast(_ context.Context, eventType string, workspaceID string, payload interface{}) error {
|
||||
c.events = append(c.events, capturedEvent{workspaceID, eventType, payload})
|
||||
return nil
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_Success_NoAttachments pins the happy
|
||||
// path: workspace lookup, broadcast, INSERT, return nil.
|
||||
func TestAgentMessageWriter_Send_Success_NoAttachments(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
w := NewAgentMessageWriter(db.DB, newTestBroadcaster())
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("CEO Ryan PC"))
|
||||
|
||||
mock.ExpectExec(`INSERT INTO activity_logs.*'a2a_receive'.*'notify'`).
|
||||
WithArgs(
|
||||
"ws-1",
|
||||
sqlmock.AnyArg(), // summary
|
||||
`{"result":"hi"}`,
|
||||
).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
if err := w.Send(context.Background(), "ws-1", "hi", nil); err != nil {
|
||||
t.Fatalf("Send returned %v, want nil", err)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("DB expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_Success_WithAttachments pins the file
|
||||
// attachment shape — response_body MUST contain a parts[] array with
|
||||
// kind=file entries so the canvas hydrater renders download chips.
|
||||
// Drift here = chips disappear on chat reload.
|
||||
func TestAgentMessageWriter_Send_Success_WithAttachments(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
w := NewAgentMessageWriter(db.DB, newTestBroadcaster())
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-att").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Ryan"))
|
||||
|
||||
mock.ExpectExec(`INSERT INTO activity_logs.*'a2a_receive'.*'notify'`).
|
||||
WithArgs(
|
||||
"ws-att",
|
||||
sqlmock.AnyArg(),
|
||||
jsonMatcher{
|
||||
desc: "response_body has result + parts with kind=file metadata",
|
||||
predicate: func(p map[string]any) bool {
|
||||
if p["result"] != "see attached" {
|
||||
return false
|
||||
}
|
||||
parts, ok := p["parts"].([]any)
|
||||
if !ok || len(parts) != 1 {
|
||||
return false
|
||||
}
|
||||
part, ok := parts[0].(map[string]any)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
if part["kind"] != "file" {
|
||||
return false
|
||||
}
|
||||
file, ok := part["file"].(map[string]any)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return file["uri"] == "workspace://x.zip" &&
|
||||
file["name"] == "x.zip" &&
|
||||
file["mimeType"] == "application/zip" &&
|
||||
file["size"].(float64) == 1234
|
||||
},
|
||||
},
|
||||
).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
atts := []AgentMessageAttachment{
|
||||
{URI: "workspace://x.zip", Name: "x.zip", MimeType: "application/zip", Size: 1234},
|
||||
}
|
||||
if err := w.Send(context.Background(), "ws-att", "see attached", atts); err != nil {
|
||||
t.Fatalf("Send returned %v, want nil", err)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("DB expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_WorkspaceNotFound pins ErrWorkspaceNotFound
|
||||
// short-circuit. Must NOT broadcast, MUST NOT INSERT — caller will 404
|
||||
// or surface a JSON-RPC error.
|
||||
func TestAgentMessageWriter_Send_WorkspaceNotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
emitter := &capturingEmitter{}
|
||||
w := NewAgentMessageWriter(db.DB, emitter)
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-missing").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}))
|
||||
|
||||
err := w.Send(context.Background(), "ws-missing", "lost in the void", nil)
|
||||
if !errors.Is(err, ErrWorkspaceNotFound) {
|
||||
t.Errorf("Send returned %v, want ErrWorkspaceNotFound", err)
|
||||
}
|
||||
if len(emitter.events) != 0 {
|
||||
t.Errorf("workspace-not-found path MUST NOT broadcast, got %d events", len(emitter.events))
|
||||
}
|
||||
// Implicit: no INSERT expectation registered, so a stray INSERT
|
||||
// would fail ExpectationsWereMet.
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("DB expectations (INSERT must NOT fire on workspace-not-found): %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_DBInsertFailureStillReturnsNil pins the
|
||||
// "best-effort persistence" contract: when the activity_log INSERT
|
||||
// fails (DB hiccup, transient connection, constraint), the writer
|
||||
// MUST still return nil. The broadcast already succeeded; the user
|
||||
// has seen the message; returning an error here would cause the
|
||||
// caller (and the agent calling the tool) to retry and double-
|
||||
// broadcast.
|
||||
func TestAgentMessageWriter_Send_DBInsertFailureStillReturnsNil(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
w := NewAgentMessageWriter(db.DB, newTestBroadcaster())
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-dbfail").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("CEO Ryan PC"))
|
||||
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WillReturnError(errors.New("transient db error"))
|
||||
|
||||
err := w.Send(context.Background(), "ws-dbfail", "should not be lost from live chat", nil)
|
||||
if err != nil {
|
||||
t.Errorf("DB INSERT failure must return nil (broadcast already succeeded), got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_PreviewTruncation pins the summary
|
||||
// preview cap. Long messages (Ryan's onboarding-friction report was
|
||||
// ~2k chars) must summarise to ≤80 chars + ellipsis so the activity
|
||||
// table doesn't carry multi-KB summaries that bloat list queries.
|
||||
func TestAgentMessageWriter_Send_PreviewTruncation(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
w := NewAgentMessageWriter(db.DB, newTestBroadcaster())
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-trunc").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Ryan"))
|
||||
|
||||
longMsg := strings.Repeat("x", 200)
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(
|
||||
"ws-trunc",
|
||||
stringMatcher(func(s string) bool {
|
||||
if !strings.HasPrefix(s, "Agent message: ") {
|
||||
return false
|
||||
}
|
||||
preview := strings.TrimPrefix(s, "Agent message: ")
|
||||
if !strings.HasSuffix(preview, "…") {
|
||||
return false
|
||||
}
|
||||
body := strings.TrimSuffix(preview, "…")
|
||||
return len(body) == 80
|
||||
}),
|
||||
sqlmock.AnyArg(),
|
||||
).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
if err := w.Send(context.Background(), "ws-trunc", longMsg, nil); err != nil {
|
||||
t.Fatalf("Send: %v", err)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("preview truncation drift: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_BroadcastsAgentMessageEvent pins the
|
||||
// WS event name + payload shape. The canvas's
|
||||
// canvas-events.ts:AGENT_MESSAGE handler reads {message, workspace_id,
|
||||
// name, attachments?} — drift here orphans every live chat panel.
|
||||
func TestAgentMessageWriter_Send_BroadcastsAgentMessageEvent(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
emitter := &capturingEmitter{}
|
||||
w := NewAgentMessageWriter(db.DB, emitter)
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-bc").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Workspace Name"))
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
atts := []AgentMessageAttachment{
|
||||
{URI: "workspace://a.txt", Name: "a.txt"},
|
||||
}
|
||||
if err := w.Send(context.Background(), "ws-bc", "hi", atts); err != nil {
|
||||
t.Fatalf("Send: %v", err)
|
||||
}
|
||||
|
||||
if len(emitter.events) != 1 {
|
||||
t.Fatalf("expected exactly 1 broadcast, got %d", len(emitter.events))
|
||||
}
|
||||
ev := emitter.events[0]
|
||||
if ev.eventType != "AGENT_MESSAGE" {
|
||||
t.Errorf("event type = %q, want AGENT_MESSAGE", ev.eventType)
|
||||
}
|
||||
if ev.workspaceID != "ws-bc" {
|
||||
t.Errorf("workspace_id = %q, want ws-bc", ev.workspaceID)
|
||||
}
|
||||
pl, ok := ev.payload.(map[string]interface{})
|
||||
if !ok {
|
||||
t.Fatalf("payload not a map: %T", ev.payload)
|
||||
}
|
||||
if pl["message"] != "hi" {
|
||||
t.Errorf("payload.message = %v, want hi", pl["message"])
|
||||
}
|
||||
if pl["workspace_id"] != "ws-bc" {
|
||||
t.Errorf("payload.workspace_id = %v, want ws-bc", pl["workspace_id"])
|
||||
}
|
||||
if pl["name"] != "Workspace Name" {
|
||||
t.Errorf("payload.name = %v, want Workspace Name", pl["name"])
|
||||
}
|
||||
if pl["attachments"] == nil {
|
||||
t.Error("payload.attachments missing on attachment-bearing send")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_DBErrorOnLookupReturnsWrapped pins the
|
||||
// distinction between sql.ErrNoRows (legit not-found → 404) and real
|
||||
// DB errors (connection drop → 503). Pre-followup the lookup branch
|
||||
// returned ErrWorkspaceNotFound for ANY error, so during a DB outage
|
||||
// every notify call surfaced as "workspace not found" and masked
|
||||
// real incidents in alerting.
|
||||
func TestAgentMessageWriter_Send_DBErrorOnLookupReturnsWrapped(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
w := NewAgentMessageWriter(db.DB, newTestBroadcaster())
|
||||
|
||||
transientErr := errors.New("connection refused")
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-dbdown").
|
||||
WillReturnError(transientErr)
|
||||
|
||||
err := w.Send(context.Background(), "ws-dbdown", "hi", nil)
|
||||
if err == nil {
|
||||
t.Fatal("expected wrapped DB error, got nil")
|
||||
}
|
||||
if errors.Is(err, ErrWorkspaceNotFound) {
|
||||
t.Errorf("DB outage MUST NOT surface as ErrWorkspaceNotFound (masks incidents in alerting); got %v", err)
|
||||
}
|
||||
if !errors.Is(err, transientErr) {
|
||||
t.Errorf("expected wrapped %v, got %v", transientErr, err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTruncatePreviewRunes_RuneBoundary pins the multi-byte-safe
|
||||
// truncation. The previous byte-slice version produced invalid UTF-8
|
||||
// when the cut landed mid-codepoint (CJK, emoji, accented), and
|
||||
// Postgres JSONB rejects invalid UTF-8 — INSERT fails, log.Printf
|
||||
// fires, message vanishes from chat history. Per memory
|
||||
// feedback_assert_exact_not_substring.md, pin the boundary cases
|
||||
// directly.
|
||||
func TestTruncatePreviewRunes_RuneBoundary(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
max int
|
||||
want string
|
||||
}{
|
||||
{"under-max ASCII", "hi", 80, "hi"},
|
||||
{"under-max CJK", "你好", 80, "你好"},
|
||||
{"exactly-at-max", "abcde", 5, "abcde"},
|
||||
{"truncate ASCII", "abcdefghij", 5, "abcde…"},
|
||||
{"truncate CJK at rune boundary", "你好世界你好世界", 4, "你好世界…"},
|
||||
{"truncate emoji at rune boundary", "😀😀😀😀😀😀", 3, "😀😀😀…"},
|
||||
// The pre-fix bug shape: byte-slice on non-ASCII would have
|
||||
// mangled the codepoint here. With rune-boundary truncation
|
||||
// the result is well-formed UTF-8.
|
||||
{"non-zero with emoji prefix", "🚀abcdefghijk", 5, "🚀abcd…"},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
got := truncatePreviewRunes(c.in, c.max)
|
||||
if got != c.want {
|
||||
t.Errorf("truncatePreviewRunes(%q, %d) = %q, want %q", c.in, c.max, got, c.want)
|
||||
}
|
||||
// Always-valid UTF-8 invariant. A byte-slice truncation
|
||||
// could leave partial codepoints; this version must not.
|
||||
if !utf8.ValidString(got) {
|
||||
t.Errorf("truncatePreviewRunes(%q, %d) returned invalid UTF-8: %q", c.in, c.max, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_NonASCIIMessagePersists pins the end-to-end
|
||||
// path for non-ASCII messages — the original reno-stars regression
|
||||
// surfaced via byte-slice truncation breaking JSONB INSERT. Every
|
||||
// handler-level test had ASCII content, so this branch had no
|
||||
// coverage. Now it does.
|
||||
func TestAgentMessageWriter_Send_NonASCIIMessagePersists(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
w := NewAgentMessageWriter(db.DB, newTestBroadcaster())
|
||||
|
||||
// 200-rune CJK message — exceeds the 80-rune cap, would have hit
|
||||
// the byte-slice bug.
|
||||
msg := strings.Repeat("你", 200)
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-cjk").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("CEO Ryan PC"))
|
||||
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(
|
||||
"ws-cjk",
|
||||
stringMatcher(func(s string) bool {
|
||||
if !strings.HasPrefix(s, "Agent message: ") {
|
||||
return false
|
||||
}
|
||||
preview := strings.TrimPrefix(s, "Agent message: ")
|
||||
if !strings.HasSuffix(preview, "…") {
|
||||
return false
|
||||
}
|
||||
body := strings.TrimSuffix(preview, "…")
|
||||
// 80 runes of 你 = 80 codepoints. Each is 3 bytes UTF-8.
|
||||
if utf8.RuneCountInString(body) != 80 {
|
||||
return false
|
||||
}
|
||||
// MUST be valid UTF-8 — pre-fix byte-slice would have
|
||||
// returned half a codepoint here.
|
||||
return utf8.ValidString(body)
|
||||
}),
|
||||
sqlmock.AnyArg(),
|
||||
).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
if err := w.Send(context.Background(), "ws-cjk", msg, nil); err != nil {
|
||||
t.Fatalf("Send: %v", err)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("non-ASCII path drift: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentMessageWriter_Send_OmitsAttachmentsKeyWhenEmpty pins the
|
||||
// "no key when nil" wire contract — extra empty fields would force
|
||||
// canvas consumers to defensively check for [] vs undefined; the
|
||||
// existing AGENT_MESSAGE handler treats absence as "no attachments".
|
||||
func TestAgentMessageWriter_Send_OmitsAttachmentsKeyWhenEmpty(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
emitter := &capturingEmitter{}
|
||||
w := NewAgentMessageWriter(db.DB, emitter)
|
||||
|
||||
mock.ExpectQuery("SELECT name FROM workspaces").
|
||||
WithArgs("ws-noatt").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("X"))
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
if err := w.Send(context.Background(), "ws-noatt", "plain text", nil); err != nil {
|
||||
t.Fatalf("Send: %v", err)
|
||||
}
|
||||
if len(emitter.events) != 1 {
|
||||
t.Fatalf("expected 1 event, got %d", len(emitter.events))
|
||||
}
|
||||
pl := emitter.events[0].payload.(map[string]interface{})
|
||||
if _, present := pl["attachments"]; present {
|
||||
t.Errorf("attachments key MUST NOT be present when empty (canvas treats absence as 'none'); payload=%v", pl)
|
||||
}
|
||||
}
|
||||
@ -53,13 +53,35 @@ func NewDelegationLedger(handle *sql.DB) *DelegationLedger {
|
||||
// truncatePreview caps stored preview at 4KB. The full prompt/response is
|
||||
// already in activity_logs.{request,response}_body — this is the at-a-glance
|
||||
// view for the dashboard, not a forensic record.
|
||||
//
|
||||
// Rune-safe: previous byte-slice form (s[:previewCap]) split on a byte
|
||||
// boundary, which on a multi-byte codepoint at byte 4096 produced
|
||||
// invalid UTF-8 — Postgres JSONB rejects → ledger row not inserted →
|
||||
// audit gap. Issue #2962. Walks the string by rune, stops at the last
|
||||
// rune-boundary index that fits inside the cap. ASCII-only strings hit
|
||||
// the cap exactly; CJK/emoji strings stop slightly under the cap,
|
||||
// never over.
|
||||
//
|
||||
// Mirrors the truncatePreviewRunes fix from agent_message_writer.go
|
||||
// (#2959). Both call sites should consume a shared helper after both
|
||||
// fixes have landed — followup deduplication tracked in #2962's body.
|
||||
const previewCap = 4096
|
||||
|
||||
func truncatePreview(s string) string {
|
||||
if len(s) <= previewCap {
|
||||
return s
|
||||
}
|
||||
return s[:previewCap]
|
||||
// Range over a string yields rune-boundary byte indices. Walk
|
||||
// until the next index would exceed previewCap; the previous
|
||||
// index is the safe truncation point.
|
||||
end := 0
|
||||
for i := range s {
|
||||
if i > previewCap {
|
||||
break
|
||||
}
|
||||
end = i
|
||||
}
|
||||
return s[:end]
|
||||
}
|
||||
|
||||
// InsertOpts is the agent's record-of-intent. Caller, callee, task preview,
|
||||
|
||||
@ -5,6 +5,7 @@ import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
)
|
||||
@ -121,6 +122,63 @@ func TestTruncatePreview_ExactlyAtCap(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestTruncatePreview_NeverProducesInvalidUTF8 — pins #2962. The old
|
||||
// byte-slice implementation (s[:previewCap]) split on a byte boundary,
|
||||
// so a multi-byte codepoint straddling byte 4096 produced invalid
|
||||
// UTF-8 → Postgres JSONB rejects → ledger row not inserted → audit
|
||||
// gap. Test feeds a CJK / emoji-padded string longer than previewCap
|
||||
// and asserts utf8.ValidString on the result.
|
||||
func TestTruncatePreview_NeverProducesInvalidUTF8(t *testing.T) {
|
||||
// Build a string of '世' (3 bytes per rune in UTF-8) that's just
|
||||
// past the cap. With the old implementation, the slice at byte
|
||||
// previewCap would land mid-rune and ValidString would fail.
|
||||
// With the rune-aware implementation, the result is always valid
|
||||
// UTF-8 even if the byte length is < previewCap.
|
||||
rune3 := "世" // U+4E16, 3 bytes
|
||||
// Need at least previewCap/3 + 1 runes so we cross the cap with
|
||||
// margin to spare.
|
||||
in := strings.Repeat(rune3, (previewCap/3)+10)
|
||||
if len(in) <= previewCap {
|
||||
t.Fatalf("test setup: input too short (%d bytes) — must exceed previewCap=%d", len(in), previewCap)
|
||||
}
|
||||
got := truncatePreview(in)
|
||||
if !utf8.ValidString(got) {
|
||||
t.Errorf("truncatePreview produced invalid UTF-8 — JSONB will reject this row. len(got)=%d", len(got))
|
||||
}
|
||||
if len(got) > previewCap {
|
||||
t.Errorf("truncatePreview exceeded cap: len(got)=%d > previewCap=%d", len(got), previewCap)
|
||||
}
|
||||
// Defense-in-depth: the result should also be a clean rune
|
||||
// prefix of the input — not some garbled sequence.
|
||||
if !strings.HasPrefix(in, got) {
|
||||
t.Errorf("truncatePreview should return a prefix of the input")
|
||||
}
|
||||
}
|
||||
|
||||
// TestTruncatePreview_MultiByteAtBoundary — most-targeted regression.
|
||||
// Feeds an input where the cap byte falls EXACTLY in the middle of a
|
||||
// 3-byte codepoint. Pre-fix, this is the case that produces invalid
|
||||
// UTF-8; post-fix, the truncate stops at the previous rune boundary.
|
||||
func TestTruncatePreview_MultiByteAtBoundary(t *testing.T) {
|
||||
// Build a string that's `previewCap-1` ASCII bytes followed by
|
||||
// '世' (3 bytes). Total = previewCap + 2. The old impl would
|
||||
// slice at byte previewCap, landing inside the '世' codepoint.
|
||||
prefix := strings.Repeat("a", previewCap-1)
|
||||
in := prefix + "世"
|
||||
if len(in) != previewCap+2 {
|
||||
t.Fatalf("test setup: expected len %d, got %d", previewCap+2, len(in))
|
||||
}
|
||||
got := truncatePreview(in)
|
||||
if !utf8.ValidString(got) {
|
||||
t.Errorf("truncatePreview produced invalid UTF-8 at the multi-byte boundary case")
|
||||
}
|
||||
// Result should be exactly the ASCII prefix — '世' was past
|
||||
// the cap so it must be dropped entirely.
|
||||
if got != prefix {
|
||||
t.Errorf("expected exact ASCII prefix, got %q (len=%d)", got[len(got)-10:], len(got))
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- SetStatus lifecycle ----------
|
||||
|
||||
func TestLedgerSetStatus_QueuedToDispatched(t *testing.T) {
|
||||
|
||||
@ -423,14 +423,23 @@ mkdir -p ~/.codex
|
||||
# (then open ~/.codex/config.toml in your editor and paste:)
|
||||
#
|
||||
# [mcp_servers.molecule]
|
||||
# command = "python3"
|
||||
# args = ["-m", "molecule_runtime.a2a_mcp_server"]
|
||||
# command = "molecule-mcp"
|
||||
# args = []
|
||||
# startup_timeout_sec = 30
|
||||
#
|
||||
# [mcp_servers.molecule.env]
|
||||
# WORKSPACE_ID = "{{WORKSPACE_ID}}"
|
||||
# PLATFORM_URL = "{{PLATFORM_URL}}"
|
||||
# MOLECULE_WORKSPACE_TOKEN = "<paste from create response>"
|
||||
#
|
||||
# Use the "molecule-mcp" console-script wrapper (NOT
|
||||
# "python3 -m molecule_runtime.a2a_mcp_server"). The wrapper is what
|
||||
# keeps the workspace ALIVE on the canvas: it POSTs /registry/register
|
||||
# at startup and runs a 20s heartbeat thread alongside the MCP stdio
|
||||
# loop. The bare a2a_mcp_server module exposes tools but does NOT
|
||||
# heartbeat — pointing codex at it leaves the canvas showing this
|
||||
# workspace as awaiting_agent (OFFLINE) within 60-90s even while
|
||||
# tools work.
|
||||
|
||||
# 3. Run the bridge daemon as a durable background process — this
|
||||
# is the INBOUND path. Long-polls the platform inbox and runs
|
||||
@ -507,11 +516,20 @@ pip install molecule-ai-workspace-runtime
|
||||
|
||||
# 3. Wire the molecule MCP server. {{WORKSPACE_ID}} + {{PLATFORM_URL}}
|
||||
# are stamped server-side; paste the auth token before running.
|
||||
#
|
||||
# Use the "molecule-mcp" console-script wrapper (NOT
|
||||
# "python3 -m molecule_runtime.a2a_mcp_server"). The wrapper is what
|
||||
# keeps the workspace ALIVE on the canvas: it POSTs /registry/register
|
||||
# at startup and runs a 20s heartbeat thread alongside the MCP stdio
|
||||
# loop. The bare a2a_mcp_server module exposes tools but does NOT
|
||||
# heartbeat — pointing openclaw at it leaves the canvas showing this
|
||||
# workspace as awaiting_agent (OFFLINE) within 60-90s even while
|
||||
# tools work.
|
||||
WORKSPACE_TOKEN="<paste from create response>"
|
||||
openclaw mcp set molecule "$(cat <<EOF
|
||||
{
|
||||
"command": "python3",
|
||||
"args": ["-m", "molecule_runtime.a2a_mcp_server"],
|
||||
"command": "molecule-mcp",
|
||||
"args": [],
|
||||
"env": {
|
||||
"WORKSPACE_ID": "{{WORKSPACE_ID}}",
|
||||
"PLATFORM_URL": "{{PLATFORM_URL}}",
|
||||
|
||||
@ -38,3 +38,40 @@ func TestExternalTemplates_NoMoleculeOrgIDPlaceholder(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestExternalMcpTemplates_UseMoleculeMcpWrapper pins the invariant
|
||||
// that operator-facing snippets configuring an MCP server entry point
|
||||
// use the ``molecule-mcp`` console-script wrapper (mcp_cli.main),
|
||||
// NOT the bare ``a2a_mcp_server`` module.
|
||||
//
|
||||
// Why: a2a_mcp_server exposes the MCP tools but does NOT call
|
||||
// /registry/register or run the 20s heartbeat thread. mcp_cli wraps
|
||||
// it with both, which is what flips the canvas presence indicator
|
||||
// from awaiting_agent (OFFLINE) to online and keeps it that way.
|
||||
// Originally tracked by molecule-core#2957 — operator hit the
|
||||
// silent-OFFLINE failure mode when the Codex tab pointed at the bare
|
||||
// module.
|
||||
//
|
||||
// The hermes-channel template intentionally uses the bare module: it
|
||||
// owns the platform plugin path and runs its own
|
||||
// register_platform/heartbeat code in-process, so wrapping with
|
||||
// mcp_cli would double-heartbeat. universalMcp / codex / openclaw
|
||||
// must all use the wrapper.
|
||||
func TestExternalMcpTemplates_UseMoleculeMcpWrapper(t *testing.T) {
|
||||
mustUseWrapper := map[string]string{
|
||||
"externalUniversalMcpTemplate": externalUniversalMcpTemplate,
|
||||
"externalCodexTemplate": externalCodexTemplate,
|
||||
"externalOpenClawTemplate": externalOpenClawTemplate,
|
||||
}
|
||||
for name, body := range mustUseWrapper {
|
||||
if !strings.Contains(body, "molecule-mcp") {
|
||||
t.Errorf("%s does not reference 'molecule-mcp' — operator-facing MCP snippets must point at the heartbeat-wrapping console script, not the bare a2a_mcp_server module (#2957)", name)
|
||||
}
|
||||
if strings.Contains(body, `"-m", "molecule_runtime.a2a_mcp_server"`) {
|
||||
t.Errorf("%s spawns 'python3 -m molecule_runtime.a2a_mcp_server' — that bypasses the standalone register/heartbeat wrapper, leaving the canvas showing the workspace OFFLINE (#2957). Use 'molecule-mcp' instead.", name)
|
||||
}
|
||||
if strings.Contains(body, `["-m", "molecule_runtime.a2a_mcp_server"]`) {
|
||||
t.Errorf("%s spawns 'python3 -m molecule_runtime.a2a_mcp_server' — that bypasses the standalone register/heartbeat wrapper, leaving the canvas showing the workspace OFFLINE (#2957). Use 'molecule-mcp' instead.", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -11,6 +11,7 @@ import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log"
|
||||
@ -330,57 +331,23 @@ func (h *MCPHandler) toolSendMessageToUser(ctx context.Context, workspaceID stri
|
||||
return "", fmt.Errorf("send_message_to_user is not enabled on this MCP bridge (set MOLECULE_MCP_ALLOW_SEND_MESSAGE=true)")
|
||||
}
|
||||
|
||||
var wsName string
|
||||
err := h.database.QueryRowContext(ctx,
|
||||
`SELECT name FROM workspaces WHERE id = $1 AND status != 'removed'`, workspaceID,
|
||||
).Scan(&wsName)
|
||||
if err != nil {
|
||||
// Single source of truth for chat-bearing agent → user messages —
|
||||
// see agent_message_writer.go for the contract. The pre-RFC-#2945
|
||||
// duplication of broadcast + INSERT logic between this handler and
|
||||
// activity.go:Notify is what produced the reno-stars data-loss
|
||||
// regression; both paths now route through the same writer.
|
||||
//
|
||||
// MCP send_message_to_user does not currently surface attachments
|
||||
// (the tool args don't accept them); pass nil. If a future tool
|
||||
// schema adds an attachments arg, build []AgentMessageAttachment
|
||||
// and pass through.
|
||||
writer := NewAgentMessageWriter(h.database, h.broadcaster)
|
||||
if err := writer.Send(ctx, workspaceID, message, nil); err != nil {
|
||||
if errors.Is(err, ErrWorkspaceNotFound) {
|
||||
return "", fmt.Errorf("workspace not found")
|
||||
}
|
||||
|
||||
h.broadcaster.BroadcastOnly(workspaceID, "AGENT_MESSAGE", map[string]interface{}{
|
||||
"message": message,
|
||||
"workspace_id": workspaceID,
|
||||
"name": wsName,
|
||||
})
|
||||
|
||||
// Persist to activity_logs so chat history loaders surface this
|
||||
// message after a page reload. Pre-fix (reno-stars 2026-05-05),
|
||||
// the MCP-bridge variant of send_message_to_user broadcast WS
|
||||
// only — visible live, gone on reload — while the HTTP /notify
|
||||
// sibling already had this fix (activity.go:535). External
|
||||
// claude-code agents using molecule-mcp's send_message_to_user
|
||||
// tool route through THIS handler, not /notify, so they were
|
||||
// hitting the unfixed path.
|
||||
//
|
||||
// Shape mirrors activity.go's Notify handler exactly so the
|
||||
// canvas chat-history hydration treats both the same:
|
||||
// - activity_type='a2a_receive' joins the source=canvas filter
|
||||
// - source_id is omitted → defaults to NULL ("canvas-source")
|
||||
// - method='notify' tags it as a push (vs a real A2A receive)
|
||||
// - request_body=NULL so the loader doesn't draw a duplicate
|
||||
// "user" bubble
|
||||
// - response_body={"result": "<text>"} feeds extractResponseText
|
||||
// directly
|
||||
//
|
||||
// Errors are log-only — the broadcast already returned ok to the
|
||||
// caller, the user has seen the message, and the persistence
|
||||
// failure mode (DB hiccup) shouldn't block the tool call. The
|
||||
// downside is the same as pre-fix: message vanishes on reload
|
||||
// after a transient DB error. Log it so operators have a signal.
|
||||
respPayload := map[string]interface{}{"result": message}
|
||||
respJSON, _ := json.Marshal(respPayload)
|
||||
preview := message
|
||||
if len(preview) > 80 {
|
||||
preview = preview[:80] + "…"
|
||||
return "", err
|
||||
}
|
||||
if _, err := h.database.ExecContext(ctx, `
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, summary, response_body, status)
|
||||
VALUES ($1, 'a2a_receive', 'notify', $2, $3::jsonb, 'ok')
|
||||
`, workspaceID, "Agent message: "+preview, string(respJSON)); err != nil {
|
||||
log.Printf("MCP send_message_to_user: failed to persist for %s: %v", workspaceID, err)
|
||||
}
|
||||
|
||||
return "Message sent.", nil
|
||||
}
|
||||
|
||||
|
||||
416
workspace-server/internal/handlers/memories_v2.go
Normal file
416
workspace-server/internal/handlers/memories_v2.go
Normal file
@ -0,0 +1,416 @@
|
||||
package handlers
|
||||
|
||||
// memories_v2.go — HTTP endpoints that expose Memory v2 plugin state to
|
||||
// the canvas Memory tab. Reads-only; writes still go through the MCP
|
||||
// path (see mcp_tools_memory_v2.go) where SAFE-T1201 redaction +
|
||||
// org-write audit happen at a single funnel.
|
||||
//
|
||||
// Why a separate v2 endpoint set rather than retrofitting memories.go:
|
||||
//
|
||||
// - memories.go reads `agent_memories` (legacy v1 table). After the
|
||||
// 2026-05-05 cutover, agent commits go to the plugin's
|
||||
// memory_records — agent_memories is frozen. The canvas Memory
|
||||
// tab reading memories.go shows STALE data.
|
||||
// - The plugin is loopback-only on each tenant (127.0.0.1:9100), so
|
||||
// the canvas (browser) cannot call it directly. workspace-server
|
||||
// proxies through these endpoints.
|
||||
// - v2 has different shape (namespace tree, kind/source/pin/TTL,
|
||||
// score) — overloading memories.go would break v1 consumers
|
||||
// (admin export, the back-compat MCP shim).
|
||||
//
|
||||
// All endpoints sit under the same wsAuth group memories.go uses,
|
||||
// so the existing per-tenant token gates them automatically.
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"log"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"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"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// MemoriesV2Handler bundles the plugin client + namespace resolver
|
||||
// behind a slim HTTP surface. Construction matches the rest of the
|
||||
// handlers package: NewMemoriesV2Handler followed by WithMemoryV2 (or
|
||||
// the test-only withMemoryV2APIs) at boot.
|
||||
type MemoriesV2Handler struct {
|
||||
plugin memoryPluginAPI
|
||||
resolver namespaceResolverAPI
|
||||
}
|
||||
|
||||
// NewMemoriesV2Handler constructs an unwired handler. Every method
|
||||
// returns 503 until WithMemoryV2 is called — keeps a partial deploy
|
||||
// (MEMORY_PLUGIN_URL absent) from crashing the canvas with 500s.
|
||||
func NewMemoriesV2Handler() *MemoriesV2Handler {
|
||||
return &MemoriesV2Handler{}
|
||||
}
|
||||
|
||||
// WithMemoryV2 attaches the live plugin client + resolver. Returns
|
||||
// the receiver for fluent boot-time wiring, mirroring MCPHandler.
|
||||
func (h *MemoriesV2Handler) WithMemoryV2(plugin *client.Client, resolver *namespace.Resolver) *MemoriesV2Handler {
|
||||
h.plugin = plugin
|
||||
h.resolver = resolver
|
||||
return h
|
||||
}
|
||||
|
||||
// withMemoryV2APIs is the test-only injection path: takes the
|
||||
// interfaces directly so unit tests don't have to construct a real
|
||||
// *client.Client / namespace.Resolver. Keep symmetric with
|
||||
// MCPHandler.withMemoryV2APIs so handler tests can re-use the same
|
||||
// stubs.
|
||||
func (h *MemoriesV2Handler) withMemoryV2APIs(plugin memoryPluginAPI, resolver namespaceResolverAPI) *MemoriesV2Handler {
|
||||
h.plugin = plugin
|
||||
h.resolver = resolver
|
||||
return h
|
||||
}
|
||||
|
||||
// available reports whether the v2 deps are wired. Each route checks
|
||||
// this and returns 503 + a clear hint when the plugin isn't
|
||||
// configured, matching the MCP-side error.
|
||||
func (h *MemoriesV2Handler) available() error {
|
||||
if h == nil || h.plugin == nil || h.resolver == nil {
|
||||
return errors.New("memory plugin is not configured (set MEMORY_PLUGIN_URL)")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// GET /workspaces/:id/v2/namespaces
|
||||
//
|
||||
// Returns the namespace tree the canvas uses to drive the Memory tab's
|
||||
// namespace dropdown. Two arrays:
|
||||
//
|
||||
// - readable[]: every namespace this workspace can READ from. Drives
|
||||
// the "show me memories from X" filter dropdown.
|
||||
// - writable[]: subset of readable that this workspace can WRITE to.
|
||||
// Used for future canvas-side commit (not in this PR but the
|
||||
// contract is symmetric so the dropdown can disable read-only
|
||||
// entries when wiring up commit).
|
||||
//
|
||||
// Each entry carries name + kind + a friendly label so the canvas
|
||||
// doesn't have to parse `workspace:abc-123` itself. Kind ranks the
|
||||
// dropdown grouping (workspace → team → org → custom).
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
// NamespaceView is the UI-friendly DTO returned by GET v2/namespaces.
|
||||
// Internal namespace.Namespace has fields the canvas doesn't need
|
||||
// (resolver-internal flags, raw metadata blobs); this strips it down.
|
||||
type NamespaceView struct {
|
||||
Name string `json:"name"`
|
||||
Kind contract.NamespaceKind `json:"kind"`
|
||||
// Label is a stable display string the canvas can render directly.
|
||||
// For workspace:<id> it's "Workspace (<short-id>)"; for team:<id>
|
||||
// it's "Team (<short-id>)"; org/custom carry the raw suffix.
|
||||
Label string `json:"label"`
|
||||
}
|
||||
|
||||
// NamespacesResponse is the body of GET v2/namespaces.
|
||||
type NamespacesResponse struct {
|
||||
Readable []NamespaceView `json:"readable"`
|
||||
Writable []NamespaceView `json:"writable"`
|
||||
}
|
||||
|
||||
// Namespaces handles GET /workspaces/:id/v2/namespaces.
|
||||
func (h *MemoriesV2Handler) Namespaces(c *gin.Context) {
|
||||
if err := h.available(); err != nil {
|
||||
c.JSON(http.StatusServiceUnavailable, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
workspaceID := c.Param("id")
|
||||
ctx := c.Request.Context()
|
||||
|
||||
readable, err := h.resolver.ReadableNamespaces(ctx, workspaceID)
|
||||
if err != nil {
|
||||
log.Printf("v2/namespaces readable error workspace=%s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to resolve readable namespaces"})
|
||||
return
|
||||
}
|
||||
writable, err := h.resolver.WritableNamespaces(ctx, workspaceID)
|
||||
if err != nil {
|
||||
log.Printf("v2/namespaces writable error workspace=%s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to resolve writable namespaces"})
|
||||
return
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, NamespacesResponse{
|
||||
Readable: namespacesToViews(readable),
|
||||
Writable: namespacesToViews(writable),
|
||||
})
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// GET /workspaces/:id/v2/memories
|
||||
//
|
||||
// Search the plugin for memories visible to this workspace.
|
||||
//
|
||||
// Query params (all optional):
|
||||
// - namespace: a single readable namespace to scope to. Omitted ⇒ all
|
||||
// readable namespaces (dropdown's "All" mode).
|
||||
// - q: full-text query string. Empty ⇒ recency-ordered listing.
|
||||
// - kind: one of fact|summary|checkpoint. Empty ⇒ all kinds.
|
||||
// - limit: max rows. Defaults to 50, clamped to 100. Matches the
|
||||
// v1 endpoint's clamp shape (memories.go:memoryRecallMaxLimit).
|
||||
//
|
||||
// Server-side ACL invariant: the request is ALWAYS intersected with
|
||||
// the resolver's readable set on the server. A canvas-supplied
|
||||
// `namespace=foo:bar` that this workspace can't read returns an empty
|
||||
// list, NOT 403 — the canvas dropdown is built from /v2/namespaces
|
||||
// so a forbidden value is a stale-cache bug, not malice. Existence
|
||||
// non-inference: empty result is indistinguishable from "you can't
|
||||
// read this namespace" — same as the wsAuth-protected v1 endpoints.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
const memoriesV2DefaultLimit = 50
|
||||
const memoriesV2MaxLimit = 100
|
||||
|
||||
// Search handles GET /workspaces/:id/v2/memories.
|
||||
func (h *MemoriesV2Handler) Search(c *gin.Context) {
|
||||
if err := h.available(); err != nil {
|
||||
c.JSON(http.StatusServiceUnavailable, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
workspaceID := c.Param("id")
|
||||
ctx := c.Request.Context()
|
||||
|
||||
requestedNS := c.Query("namespace")
|
||||
query := c.Query("q")
|
||||
kindStr := c.Query("kind")
|
||||
limit := parseLimit(c.Query("limit"))
|
||||
|
||||
// Resolve the readable set, then intersect the request.
|
||||
// IntersectReadable handles both the empty-request case (return
|
||||
// all readable) and the explicit-namespace case (return [ns] iff
|
||||
// readable, else []).
|
||||
var requested []string
|
||||
if requestedNS != "" {
|
||||
requested = []string{requestedNS}
|
||||
}
|
||||
scopedNamespaces, err := h.resolver.IntersectReadable(ctx, workspaceID, requested)
|
||||
if err != nil {
|
||||
log.Printf("v2/memories intersect error workspace=%s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to resolve namespaces"})
|
||||
return
|
||||
}
|
||||
// Empty after intersection — caller asked for a namespace they
|
||||
// can't read, OR they have no readable namespaces at all. Return
|
||||
// [] (not 404) so the canvas can render its empty-state without
|
||||
// special-casing.
|
||||
if len(scopedNamespaces) == 0 {
|
||||
c.JSON(http.StatusOK, MemoriesResponse{Memories: []MemoryView{}})
|
||||
return
|
||||
}
|
||||
|
||||
req := contract.SearchRequest{
|
||||
Namespaces: scopedNamespaces,
|
||||
Query: query,
|
||||
Limit: limit,
|
||||
}
|
||||
if kindStr != "" {
|
||||
req.Kinds = []contract.MemoryKind{contract.MemoryKind(kindStr)}
|
||||
}
|
||||
|
||||
resp, err := h.plugin.Search(ctx, req)
|
||||
if err != nil {
|
||||
log.Printf("v2/memories plugin error workspace=%s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusBadGateway, gin.H{"error": "memory plugin search failed"})
|
||||
return
|
||||
}
|
||||
|
||||
out := MemoriesResponse{Memories: make([]MemoryView, 0, len(resp.Memories))}
|
||||
for _, m := range resp.Memories {
|
||||
out.Memories = append(out.Memories, memoryToView(m))
|
||||
}
|
||||
c.JSON(http.StatusOK, out)
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// DELETE /workspaces/:id/v2/memories/:memoryId
|
||||
//
|
||||
// Forget a memory. The plugin enforces its own ownership model — we
|
||||
// pass `requested_by_namespace = workspace:<id>` so the audit trail
|
||||
// records who initiated the forget; the plugin's ACL gate decides
|
||||
// whether the deletion is allowed.
|
||||
//
|
||||
// 404 (not 403) on a missing or non-owned memory: existence-non-
|
||||
// inferring response, matches the v1 DELETE in memories.go.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
// Forget handles DELETE /workspaces/:id/v2/memories/:memoryId.
|
||||
func (h *MemoriesV2Handler) Forget(c *gin.Context) {
|
||||
if err := h.available(); err != nil {
|
||||
c.JSON(http.StatusServiceUnavailable, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
workspaceID := c.Param("id")
|
||||
memoryID := c.Param("memoryId")
|
||||
ctx := c.Request.Context()
|
||||
|
||||
if memoryID == "" {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "memoryId is required"})
|
||||
return
|
||||
}
|
||||
|
||||
body := contract.ForgetRequest{
|
||||
RequestedByNamespace: "workspace:" + workspaceID,
|
||||
}
|
||||
if err := h.plugin.ForgetMemory(ctx, memoryID, body); err != nil {
|
||||
// Map plugin not_found → 404. Anything else is upstream error.
|
||||
var ce *contract.Error
|
||||
if errors.As(err, &ce) && ce.Code == contract.ErrorCodeNotFound {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "memory not found"})
|
||||
return
|
||||
}
|
||||
log.Printf("v2/memories forget error workspace=%s memory=%s: %v", workspaceID, memoryID, err)
|
||||
c.JSON(http.StatusBadGateway, gin.H{"error": "memory plugin delete failed"})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "deleted"})
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// View shaping helpers
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
// MemoryView is the canvas-facing shape of a v2 memory record. The raw
|
||||
// contract.Memory carries internal fields we don't expose (raw
|
||||
// `propagation` blob); MemoryView strips it to what the Memory tab
|
||||
// renders.
|
||||
type MemoryView struct {
|
||||
ID string `json:"id"`
|
||||
Namespace string `json:"namespace"`
|
||||
Content string `json:"content"`
|
||||
Kind contract.MemoryKind `json:"kind"`
|
||||
Source contract.MemorySource `json:"source"`
|
||||
Pin bool `json:"pin"`
|
||||
ExpiresAt *time.Time `json:"expires_at,omitempty"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
// Score is the plugin's similarity score (1.0 = exact); only
|
||||
// populated when ?q= is set and the plugin supports embedding.
|
||||
Score *float64 `json:"score,omitempty"`
|
||||
// SourceWorkspaceID is parsed out of `propagation.source_workspace_id`
|
||||
// when present (cross-workspace propagation) — lets the canvas
|
||||
// render a "from <peer>" badge so users can tell their own writes
|
||||
// apart from team-shared memory.
|
||||
SourceWorkspaceID string `json:"source_workspace_id,omitempty"`
|
||||
}
|
||||
|
||||
// MemoriesResponse is the body of GET v2/memories.
|
||||
type MemoriesResponse struct {
|
||||
Memories []MemoryView `json:"memories"`
|
||||
}
|
||||
|
||||
func memoryToView(m contract.Memory) MemoryView {
|
||||
v := MemoryView{
|
||||
ID: m.ID,
|
||||
Namespace: m.Namespace,
|
||||
Content: m.Content,
|
||||
Kind: m.Kind,
|
||||
Source: m.Source,
|
||||
Pin: m.Pin,
|
||||
ExpiresAt: m.ExpiresAt,
|
||||
CreatedAt: m.CreatedAt,
|
||||
Score: m.Score,
|
||||
}
|
||||
if m.Propagation != nil {
|
||||
// `source_workspace_id` is a propagation contract field
|
||||
// (RFC #2728 §5). Plugin emits it on writes that originated
|
||||
// from a different workspace. Best-effort string extraction —
|
||||
// don't fail rendering if shape drifts.
|
||||
if raw, ok := m.Propagation["source_workspace_id"]; ok {
|
||||
if s, ok := raw.(string); ok && s != "" {
|
||||
v.SourceWorkspaceID = s
|
||||
}
|
||||
}
|
||||
}
|
||||
return v
|
||||
}
|
||||
|
||||
// namespacesToViews converts resolver namespaces into UI-friendly
|
||||
// views. Stable sort: workspace → team → org → custom, then by name.
|
||||
func namespacesToViews(in []namespace.Namespace) []NamespaceView {
|
||||
views := make([]NamespaceView, 0, len(in))
|
||||
for _, n := range in {
|
||||
views = append(views, NamespaceView{
|
||||
Name: n.Name,
|
||||
Kind: n.Kind,
|
||||
Label: namespaceLabel(n.Name, n.Kind),
|
||||
})
|
||||
}
|
||||
return views
|
||||
}
|
||||
|
||||
// namespaceLabel renders a human-friendly label for a namespace. The
|
||||
// canvas displays this directly; we keep the formatting server-side
|
||||
// so the shape stays consistent across UIs (canvas, future TUI, etc.).
|
||||
//
|
||||
// Format:
|
||||
// workspace:abc-123 → "Workspace (abc-123)" (UUID short-prefixed)
|
||||
// team:t-1 → "Team (t-1)"
|
||||
// org:acme → "Org (acme)"
|
||||
// custom:foo → "foo" (operator-defined; raw)
|
||||
func namespaceLabel(name string, kind contract.NamespaceKind) string {
|
||||
suffix := ""
|
||||
if i := indexOfColon(name); i >= 0 && i+1 < len(name) {
|
||||
suffix = name[i+1:]
|
||||
}
|
||||
switch kind {
|
||||
case contract.NamespaceKindWorkspace:
|
||||
return "Workspace (" + shortID(suffix) + ")"
|
||||
case contract.NamespaceKindTeam:
|
||||
return "Team (" + shortID(suffix) + ")"
|
||||
case contract.NamespaceKindOrg:
|
||||
return "Org (" + suffix + ")"
|
||||
case contract.NamespaceKindCustom:
|
||||
// Custom namespaces are operator-defined; surface the raw
|
||||
// suffix so they can label them however they want.
|
||||
if suffix == "" {
|
||||
return name
|
||||
}
|
||||
return suffix
|
||||
default:
|
||||
return name
|
||||
}
|
||||
}
|
||||
|
||||
// shortID truncates a UUID-like string to the first 8 chars so the
|
||||
// dropdown stays readable. Keeps the full id available via the
|
||||
// `name` field for click-to-copy / debugging.
|
||||
func shortID(s string) string {
|
||||
if len(s) <= 8 {
|
||||
return s
|
||||
}
|
||||
return s[:8]
|
||||
}
|
||||
|
||||
// indexOfColon is strings.IndexByte without the import, kept inline so
|
||||
// the helper stays trivially auditable next to namespaceLabel.
|
||||
func indexOfColon(s string) int {
|
||||
for i := 0; i < len(s); i++ {
|
||||
if s[i] == ':' {
|
||||
return i
|
||||
}
|
||||
}
|
||||
return -1
|
||||
}
|
||||
|
||||
// parseLimit validates the ?limit= query value. Defaults +
|
||||
// clamps mirror memoriesV2DefaultLimit / memoriesV2MaxLimit.
|
||||
func parseLimit(raw string) int {
|
||||
if raw == "" {
|
||||
return memoriesV2DefaultLimit
|
||||
}
|
||||
n, err := strconv.Atoi(raw)
|
||||
if err != nil || n <= 0 {
|
||||
return memoriesV2DefaultLimit
|
||||
}
|
||||
if n > memoriesV2MaxLimit {
|
||||
return memoriesV2MaxLimit
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
669
workspace-server/internal/handlers/memories_v2_test.go
Normal file
669
workspace-server/internal/handlers/memories_v2_test.go
Normal file
@ -0,0 +1,669 @@
|
||||
package handlers
|
||||
|
||||
// memories_v2_test.go — comprehensive coverage for the Memory v2
|
||||
// canvas-facing HTTP surface. Pinned shape:
|
||||
//
|
||||
// - 503 path when plugin unwired (every route)
|
||||
// - GET /v2/namespaces success + readable/writable propagation
|
||||
// - GET /v2/namespaces error path (resolver failure on either call)
|
||||
// - GET /v2/memories: empty intersection, namespace passthrough,
|
||||
// query+kind+limit propagation, plugin error mapping
|
||||
// - DELETE /v2/memories/:id: success, plugin not_found→404, other
|
||||
// plugin errors→502, missing memoryId→400
|
||||
// - View shaping: namespaceLabel for all four kinds + truncation,
|
||||
// memoryToView with/without propagation source, parseLimit edge
|
||||
// cases (default, negative, zero, over-cap, non-numeric)
|
||||
//
|
||||
// Tests use the same `memoryPluginAPI` / `namespaceResolverAPI` fakes
|
||||
// the MCP v2 tests use so we don't spin up a real plugin server.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/contract"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/memory/namespace"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Fakes
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
type fakePlugin struct {
|
||||
searchResp *contract.SearchResponse
|
||||
searchErr error
|
||||
searchReq contract.SearchRequest // captured for assertion
|
||||
forgetErr error
|
||||
forgetID string
|
||||
forgetReq contract.ForgetRequest
|
||||
}
|
||||
|
||||
func (f *fakePlugin) CommitMemory(ctx context.Context, ns string, body contract.MemoryWrite) (*contract.MemoryWriteResponse, error) {
|
||||
return nil, errors.New("not implemented in fake")
|
||||
}
|
||||
func (f *fakePlugin) Search(ctx context.Context, body contract.SearchRequest) (*contract.SearchResponse, error) {
|
||||
f.searchReq = body
|
||||
if f.searchErr != nil {
|
||||
return nil, f.searchErr
|
||||
}
|
||||
return f.searchResp, nil
|
||||
}
|
||||
func (f *fakePlugin) ForgetMemory(ctx context.Context, id string, body contract.ForgetRequest) error {
|
||||
f.forgetID = id
|
||||
f.forgetReq = body
|
||||
return f.forgetErr
|
||||
}
|
||||
|
||||
type fakeNSResolver struct {
|
||||
readable []namespace.Namespace
|
||||
readableErr error
|
||||
writable []namespace.Namespace
|
||||
writableErr error
|
||||
intersect []string
|
||||
intersectErr error
|
||||
intersectIn []string // captured
|
||||
}
|
||||
|
||||
func (f *fakeNSResolver) ReadableNamespaces(ctx context.Context, ws string) ([]namespace.Namespace, error) {
|
||||
return f.readable, f.readableErr
|
||||
}
|
||||
func (f *fakeNSResolver) WritableNamespaces(ctx context.Context, ws string) ([]namespace.Namespace, error) {
|
||||
return f.writable, f.writableErr
|
||||
}
|
||||
func (f *fakeNSResolver) CanWrite(ctx context.Context, ws, ns string) (bool, error) {
|
||||
return true, nil
|
||||
}
|
||||
func (f *fakeNSResolver) IntersectReadable(ctx context.Context, ws string, requested []string) ([]string, error) {
|
||||
f.intersectIn = requested
|
||||
return f.intersect, f.intersectErr
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Test helpers
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func init() {
|
||||
gin.SetMode(gin.TestMode)
|
||||
}
|
||||
|
||||
// newWiredHandler returns a handler with both the fake plugin + fake
|
||||
// resolver attached. Tests that need the unwired (503) path use
|
||||
// NewMemoriesV2Handler() directly.
|
||||
func newWiredHandler(p *fakePlugin, r *fakeNSResolver) *MemoriesV2Handler {
|
||||
return NewMemoriesV2Handler().withMemoryV2APIs(p, r)
|
||||
}
|
||||
|
||||
func doRequest(t *testing.T, h *MemoriesV2Handler, method, path string, params gin.Params) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = params
|
||||
req := httptest.NewRequest(method, path, nil)
|
||||
c.Request = req
|
||||
switch {
|
||||
case method == http.MethodGet && strings.HasSuffix(path, "/v2/namespaces"):
|
||||
h.Namespaces(c)
|
||||
case method == http.MethodGet && strings.Contains(path, "/v2/memories"):
|
||||
h.Search(c)
|
||||
case method == http.MethodDelete:
|
||||
h.Forget(c)
|
||||
default:
|
||||
t.Fatalf("doRequest: don't know how to dispatch %s %s", method, path)
|
||||
}
|
||||
return rec
|
||||
}
|
||||
|
||||
func mustJSON(t *testing.T, body []byte, out interface{}) {
|
||||
t.Helper()
|
||||
if err := json.Unmarshal(body, out); err != nil {
|
||||
t.Fatalf("json decode: %v\nbody=%s", err, string(body))
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// 503 — plugin unwired
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestMemoriesV2_PluginUnwired_All503(t *testing.T) {
|
||||
h := NewMemoriesV2Handler() // no WithMemoryV2 / withMemoryV2APIs
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
params gin.Params
|
||||
}{
|
||||
{"namespaces", http.MethodGet, "/workspaces/ws-a/v2/namespaces", gin.Params{{Key: "id", Value: "ws-a"}}},
|
||||
{"search", http.MethodGet, "/workspaces/ws-a/v2/memories", gin.Params{{Key: "id", Value: "ws-a"}}},
|
||||
{"forget", http.MethodDelete, "/workspaces/ws-a/v2/memories/m-1", gin.Params{{Key: "id", Value: "ws-a"}, {Key: "memoryId", Value: "m-1"}}},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
rec := doRequest(t, h, tc.method, tc.path, tc.params)
|
||||
if rec.Code != http.StatusServiceUnavailable {
|
||||
t.Errorf("expected 503, got %d", rec.Code)
|
||||
}
|
||||
var body map[string]string
|
||||
mustJSON(t, rec.Body.Bytes(), &body)
|
||||
if !strings.Contains(body["error"], "MEMORY_PLUGIN_URL") {
|
||||
t.Errorf("503 body missing operator hint, got: %q", body["error"])
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// GET /v2/namespaces
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestMemoriesV2_Namespaces_Success(t *testing.T) {
|
||||
resolver := &fakeNSResolver{
|
||||
readable: []namespace.Namespace{
|
||||
{Name: "workspace:abc-1234-5678", Kind: contract.NamespaceKindWorkspace},
|
||||
{Name: "team:t-99", Kind: contract.NamespaceKindTeam},
|
||||
{Name: "org:acme", Kind: contract.NamespaceKindOrg},
|
||||
{Name: "custom:special", Kind: contract.NamespaceKindCustom},
|
||||
},
|
||||
writable: []namespace.Namespace{
|
||||
{Name: "workspace:abc-1234-5678", Kind: contract.NamespaceKindWorkspace},
|
||||
},
|
||||
}
|
||||
h := newWiredHandler(&fakePlugin{}, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/namespaces",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var body NamespacesResponse
|
||||
mustJSON(t, rec.Body.Bytes(), &body)
|
||||
|
||||
if len(body.Readable) != 4 {
|
||||
t.Errorf("expected 4 readable, got %d", len(body.Readable))
|
||||
}
|
||||
if len(body.Writable) != 1 {
|
||||
t.Errorf("expected 1 writable, got %d", len(body.Writable))
|
||||
}
|
||||
|
||||
// Label shaping pinned exactly — drift would silently break the
|
||||
// dropdown rendering.
|
||||
wantLabels := map[string]string{
|
||||
"workspace:abc-1234-5678": "Workspace (abc-1234)",
|
||||
"team:t-99": "Team (t-99)",
|
||||
"org:acme": "Org (acme)",
|
||||
"custom:special": "special",
|
||||
}
|
||||
for _, v := range body.Readable {
|
||||
want, ok := wantLabels[v.Name]
|
||||
if !ok {
|
||||
t.Errorf("unexpected namespace name %q", v.Name)
|
||||
continue
|
||||
}
|
||||
if v.Label != want {
|
||||
t.Errorf("namespace %q: want label %q, got %q", v.Name, want, v.Label)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Namespaces_ReadableError(t *testing.T) {
|
||||
resolver := &fakeNSResolver{readableErr: errors.New("boom")}
|
||||
h := newWiredHandler(&fakePlugin{}, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/namespaces",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Namespaces_WritableError(t *testing.T) {
|
||||
resolver := &fakeNSResolver{
|
||||
readable: []namespace.Namespace{},
|
||||
writableErr: errors.New("boom"),
|
||||
}
|
||||
h := newWiredHandler(&fakePlugin{}, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/namespaces",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// GET /v2/memories — search path
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestMemoriesV2_Search_NoReadableNamespaces_EmptyResult(t *testing.T) {
|
||||
// Empty intersection (e.g. workspace just provisioned, plugin
|
||||
// hasn't created namespaces yet, OR caller asked for ns they
|
||||
// can't read). Expected: 200 with empty memories array, NOT 404.
|
||||
resolver := &fakeNSResolver{intersect: []string{}}
|
||||
plugin := &fakePlugin{searchResp: &contract.SearchResponse{Memories: []contract.Memory{}}}
|
||||
h := newWiredHandler(plugin, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/memories",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != 200 {
|
||||
t.Errorf("expected 200, got %d", rec.Code)
|
||||
}
|
||||
var body MemoriesResponse
|
||||
mustJSON(t, rec.Body.Bytes(), &body)
|
||||
if body.Memories == nil {
|
||||
t.Error("Memories should be empty array, not nil — JSON would render null")
|
||||
}
|
||||
if len(body.Memories) != 0 {
|
||||
t.Errorf("expected empty memories, got %d", len(body.Memories))
|
||||
}
|
||||
// Plugin must NOT be called when intersection is empty.
|
||||
if plugin.searchReq.Namespaces != nil {
|
||||
t.Error("plugin Search should not be called when intersection is empty")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Search_FullPath_NamespaceQueryKindLimit(t *testing.T) {
|
||||
expiresAt := time.Now().Add(24 * time.Hour)
|
||||
resolver := &fakeNSResolver{intersect: []string{"workspace:ws-a"}}
|
||||
score := 0.87
|
||||
plugin := &fakePlugin{
|
||||
searchResp: &contract.SearchResponse{
|
||||
Memories: []contract.Memory{
|
||||
{
|
||||
ID: "m-1",
|
||||
Namespace: "workspace:ws-a",
|
||||
Content: "fact one",
|
||||
Kind: contract.MemoryKindFact,
|
||||
Source: contract.MemorySourceAgent,
|
||||
Pin: true,
|
||||
ExpiresAt: &expiresAt,
|
||||
CreatedAt: time.Now(),
|
||||
Score: &score,
|
||||
Propagation: map[string]interface{}{
|
||||
"source_workspace_id": "ws-peer-42",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
h := newWiredHandler(plugin, resolver)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-a"}}
|
||||
c.Request = httptest.NewRequest(http.MethodGet,
|
||||
"/workspaces/ws-a/v2/memories?namespace=workspace:ws-a&q=hello&kind=fact&limit=10", nil)
|
||||
h.Search(c)
|
||||
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
// Resolver received the requested namespace as a single-element list
|
||||
if len(resolver.intersectIn) != 1 || resolver.intersectIn[0] != "workspace:ws-a" {
|
||||
t.Errorf("resolver.IntersectReadable received %v, want [workspace:ws-a]", resolver.intersectIn)
|
||||
}
|
||||
// Plugin received query + kind + limit propagated through
|
||||
if plugin.searchReq.Query != "hello" {
|
||||
t.Errorf("plugin.Query=%q, want hello", plugin.searchReq.Query)
|
||||
}
|
||||
if len(plugin.searchReq.Kinds) != 1 || plugin.searchReq.Kinds[0] != contract.MemoryKindFact {
|
||||
t.Errorf("plugin.Kinds=%v, want [fact]", plugin.searchReq.Kinds)
|
||||
}
|
||||
if plugin.searchReq.Limit != 10 {
|
||||
t.Errorf("plugin.Limit=%d, want 10", plugin.searchReq.Limit)
|
||||
}
|
||||
// Response shape — pin/expires_at/score/source_workspace_id all
|
||||
// surfaced into MemoryView so the canvas doesn't have to dig
|
||||
// through propagation map.
|
||||
var body MemoriesResponse
|
||||
mustJSON(t, rec.Body.Bytes(), &body)
|
||||
if len(body.Memories) != 1 {
|
||||
t.Fatalf("expected 1 memory, got %d", len(body.Memories))
|
||||
}
|
||||
m := body.Memories[0]
|
||||
if !m.Pin {
|
||||
t.Error("Pin not propagated")
|
||||
}
|
||||
if m.ExpiresAt == nil {
|
||||
t.Error("ExpiresAt not propagated")
|
||||
}
|
||||
if m.Score == nil || *m.Score != 0.87 {
|
||||
t.Errorf("Score=%v, want 0.87", m.Score)
|
||||
}
|
||||
if m.SourceWorkspaceID != "ws-peer-42" {
|
||||
t.Errorf("SourceWorkspaceID=%q, want ws-peer-42", m.SourceWorkspaceID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Search_NoNamespaceQuery_AllReadable(t *testing.T) {
|
||||
// No ?namespace= → resolver.IntersectReadable receives nil (empty
|
||||
// requested) and returns ALL readable. Plugin gets full set.
|
||||
resolver := &fakeNSResolver{intersect: []string{"workspace:ws-a", "team:t-1"}}
|
||||
plugin := &fakePlugin{searchResp: &contract.SearchResponse{}}
|
||||
h := newWiredHandler(plugin, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/memories",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != 200 {
|
||||
t.Errorf("expected 200, got %d", rec.Code)
|
||||
}
|
||||
if resolver.intersectIn != nil {
|
||||
t.Errorf("requested should be nil for unscoped query, got %v", resolver.intersectIn)
|
||||
}
|
||||
if len(plugin.searchReq.Namespaces) != 2 {
|
||||
t.Errorf("plugin.Namespaces=%v, want both readable", plugin.searchReq.Namespaces)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Search_IntersectError(t *testing.T) {
|
||||
resolver := &fakeNSResolver{intersectErr: errors.New("db down")}
|
||||
h := newWiredHandler(&fakePlugin{}, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/memories",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Search_PluginError(t *testing.T) {
|
||||
resolver := &fakeNSResolver{intersect: []string{"workspace:ws-a"}}
|
||||
plugin := &fakePlugin{searchErr: errors.New("plugin down")}
|
||||
h := newWiredHandler(plugin, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/memories",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
t.Errorf("expected 502 (plugin error), got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Search_PropagationMissing_NoSourceWorkspaceID(t *testing.T) {
|
||||
resolver := &fakeNSResolver{intersect: []string{"workspace:ws-a"}}
|
||||
plugin := &fakePlugin{
|
||||
searchResp: &contract.SearchResponse{
|
||||
Memories: []contract.Memory{
|
||||
{ID: "m-1", Namespace: "workspace:ws-a", Content: "no propagation"},
|
||||
},
|
||||
},
|
||||
}
|
||||
h := newWiredHandler(plugin, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/memories",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
var body MemoriesResponse
|
||||
mustJSON(t, rec.Body.Bytes(), &body)
|
||||
if len(body.Memories) != 1 || body.Memories[0].SourceWorkspaceID != "" {
|
||||
t.Errorf("SourceWorkspaceID should be empty when propagation is nil, got %q", body.Memories[0].SourceWorkspaceID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Search_PropagationWrongType_DoesNotPanic(t *testing.T) {
|
||||
resolver := &fakeNSResolver{intersect: []string{"workspace:ws-a"}}
|
||||
plugin := &fakePlugin{
|
||||
searchResp: &contract.SearchResponse{
|
||||
Memories: []contract.Memory{
|
||||
{
|
||||
ID: "m-1",
|
||||
Content: "wrong-type propagation",
|
||||
Propagation: map[string]interface{}{
|
||||
"source_workspace_id": 12345, // int, not string
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
h := newWiredHandler(plugin, resolver)
|
||||
|
||||
rec := doRequest(t, h, http.MethodGet, "/workspaces/ws-a/v2/memories",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}})
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("expected 200 (graceful), got %d", rec.Code)
|
||||
}
|
||||
var body MemoriesResponse
|
||||
mustJSON(t, rec.Body.Bytes(), &body)
|
||||
// Wrong-typed prop entry → empty SourceWorkspaceID, no panic.
|
||||
if body.Memories[0].SourceWorkspaceID != "" {
|
||||
t.Errorf("expected empty SourceWorkspaceID for non-string propagation, got %q", body.Memories[0].SourceWorkspaceID)
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// DELETE /v2/memories/:memoryId
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestMemoriesV2_Forget_Success(t *testing.T) {
|
||||
plugin := &fakePlugin{} // forgetErr nil
|
||||
h := newWiredHandler(plugin, &fakeNSResolver{})
|
||||
|
||||
rec := doRequest(t, h, http.MethodDelete, "/workspaces/ws-a/v2/memories/mem-42",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}, {Key: "memoryId", Value: "mem-42"}})
|
||||
if rec.Code != 200 {
|
||||
t.Errorf("expected 200, got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if plugin.forgetID != "mem-42" {
|
||||
t.Errorf("plugin received memoryID=%q, want mem-42", plugin.forgetID)
|
||||
}
|
||||
if plugin.forgetReq.RequestedByNamespace != "workspace:ws-a" {
|
||||
t.Errorf("requested_by_namespace=%q, want workspace:ws-a", plugin.forgetReq.RequestedByNamespace)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Forget_PluginNotFound_Maps404(t *testing.T) {
|
||||
plugin := &fakePlugin{
|
||||
forgetErr: &contract.Error{Code: contract.ErrorCodeNotFound, Message: "no such memory"},
|
||||
}
|
||||
h := newWiredHandler(plugin, &fakeNSResolver{})
|
||||
|
||||
rec := doRequest(t, h, http.MethodDelete, "/workspaces/ws-a/v2/memories/m-1",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}, {Key: "memoryId", Value: "m-1"}})
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("expected 404, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Forget_PluginOtherError_Maps502(t *testing.T) {
|
||||
plugin := &fakePlugin{
|
||||
forgetErr: &contract.Error{Code: contract.ErrorCodeInternal, Message: "db dead"},
|
||||
}
|
||||
h := newWiredHandler(plugin, &fakeNSResolver{})
|
||||
|
||||
rec := doRequest(t, h, http.MethodDelete, "/workspaces/ws-a/v2/memories/m-1",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}, {Key: "memoryId", Value: "m-1"}})
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
t.Errorf("expected 502, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Forget_NonContractError_Maps502(t *testing.T) {
|
||||
// A raw error (e.g. transport failure) — not a contract.Error —
|
||||
// also bubbles up as 502.
|
||||
plugin := &fakePlugin{forgetErr: errors.New("connection reset")}
|
||||
h := newWiredHandler(plugin, &fakeNSResolver{})
|
||||
|
||||
rec := doRequest(t, h, http.MethodDelete, "/workspaces/ws-a/v2/memories/m-1",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}, {Key: "memoryId", Value: "m-1"}})
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
t.Errorf("expected 502, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoriesV2_Forget_MissingMemoryID_400(t *testing.T) {
|
||||
h := newWiredHandler(&fakePlugin{}, &fakeNSResolver{})
|
||||
rec := doRequest(t, h, http.MethodDelete, "/workspaces/ws-a/v2/memories/",
|
||||
gin.Params{{Key: "id", Value: "ws-a"}, {Key: "memoryId", Value: ""}})
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// View-shaping unit tests — pin individual helpers
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestNamespaceLabel_AllKinds(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
kind contract.NamespaceKind
|
||||
want string
|
||||
}{
|
||||
{"workspace:abcdefghij", contract.NamespaceKindWorkspace, "Workspace (abcdefgh)"}, // truncated to 8
|
||||
{"workspace:abc", contract.NamespaceKindWorkspace, "Workspace (abc)"}, // shorter than 8, kept as-is
|
||||
{"team:t-99", contract.NamespaceKindTeam, "Team (t-99)"},
|
||||
{"org:acme", contract.NamespaceKindOrg, "Org (acme)"},
|
||||
{"custom:my-ns", contract.NamespaceKindCustom, "my-ns"},
|
||||
{"custom:", contract.NamespaceKindCustom, "custom:"}, // empty suffix → fallback to raw name
|
||||
{"weird-no-colon", contract.NamespaceKindWorkspace, "Workspace ()"},
|
||||
{"unknown:x", contract.NamespaceKind("future"), "unknown:x"}, // unknown kind → fallback to raw name
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := namespaceLabel(tc.name, tc.kind)
|
||||
if got != tc.want {
|
||||
t.Errorf("namespaceLabel(%q, %q) = %q, want %q", tc.name, tc.kind, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseLimit(t *testing.T) {
|
||||
cases := []struct {
|
||||
raw string
|
||||
want int
|
||||
}{
|
||||
{"", memoriesV2DefaultLimit},
|
||||
{"10", 10},
|
||||
{"0", memoriesV2DefaultLimit}, // ≤0 → default, not error
|
||||
{"-5", memoriesV2DefaultLimit}, // negative → default
|
||||
{"abc", memoriesV2DefaultLimit}, // non-numeric → default
|
||||
{"99999", memoriesV2MaxLimit}, // over cap → clamped
|
||||
{"100", memoriesV2MaxLimit}, // exactly cap → kept
|
||||
{"99", 99}, // just under cap → kept
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run("raw="+tc.raw, func(t *testing.T) {
|
||||
if got := parseLimit(tc.raw); got != tc.want {
|
||||
t.Errorf("parseLimit(%q) = %d, want %d", tc.raw, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMemoryToView_AllFieldsPropagated(t *testing.T) {
|
||||
now := time.Now()
|
||||
exp := now.Add(time.Hour)
|
||||
score := 0.95
|
||||
m := contract.Memory{
|
||||
ID: "m-1",
|
||||
Namespace: "team:t-1",
|
||||
Content: "hello",
|
||||
Kind: contract.MemoryKindSummary,
|
||||
Source: contract.MemorySourceUser,
|
||||
Pin: true,
|
||||
ExpiresAt: &exp,
|
||||
CreatedAt: now,
|
||||
Score: &score,
|
||||
Propagation: map[string]interface{}{
|
||||
"source_workspace_id": "ws-other",
|
||||
},
|
||||
}
|
||||
v := memoryToView(m)
|
||||
if v.ID != m.ID || v.Namespace != m.Namespace || v.Content != m.Content {
|
||||
t.Errorf("basic fields: %+v", v)
|
||||
}
|
||||
if v.Kind != contract.MemoryKindSummary || v.Source != contract.MemorySourceUser {
|
||||
t.Errorf("kind/source: %+v", v)
|
||||
}
|
||||
if !v.Pin || v.ExpiresAt == nil || v.Score == nil || *v.Score != 0.95 {
|
||||
t.Errorf("pin/expires/score: %+v", v)
|
||||
}
|
||||
if v.SourceWorkspaceID != "ws-other" {
|
||||
t.Errorf("SourceWorkspaceID=%q, want ws-other", v.SourceWorkspaceID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNamespacesToViews_PreservesOrder(t *testing.T) {
|
||||
in := []namespace.Namespace{
|
||||
{Name: "team:t1", Kind: contract.NamespaceKindTeam},
|
||||
{Name: "workspace:w1", Kind: contract.NamespaceKindWorkspace},
|
||||
}
|
||||
out := namespacesToViews(in)
|
||||
if len(out) != 2 {
|
||||
t.Fatalf("len=%d", len(out))
|
||||
}
|
||||
// Resolver determines order; we just preserve it. (Sorting can be
|
||||
// added at the resolver layer if the canvas needs it.)
|
||||
if out[0].Name != "team:t1" || out[1].Name != "workspace:w1" {
|
||||
t.Errorf("order not preserved: %+v", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNamespacesToViews_EmptyInput_EmptySlice(t *testing.T) {
|
||||
out := namespacesToViews(nil)
|
||||
if out == nil {
|
||||
t.Error("expected empty slice, not nil — JSON-marshals as null otherwise")
|
||||
}
|
||||
if len(out) != 0 {
|
||||
t.Errorf("expected len 0, got %d", len(out))
|
||||
}
|
||||
}
|
||||
|
||||
func TestIndexOfColon(t *testing.T) {
|
||||
cases := []struct {
|
||||
s string
|
||||
want int
|
||||
}{
|
||||
{"abc:def", 3},
|
||||
{":foo", 0},
|
||||
{"nocolon", -1},
|
||||
{"", -1},
|
||||
{"a:b:c", 1}, // first colon only
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := indexOfColon(tc.s); got != tc.want {
|
||||
t.Errorf("indexOfColon(%q) = %d, want %d", tc.s, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestWithMemoryV2_FluentReturnsReceiver(t *testing.T) {
|
||||
// WithMemoryV2 is the production wiring path (takes *client.Client +
|
||||
// *namespace.Resolver). withMemoryV2APIs is the test path. The
|
||||
// production call is structural — assigns the two fields and
|
||||
// returns the receiver — but we still want a 100% coverage gate
|
||||
// to catch a future refactor that accidentally drops the fluent
|
||||
// return (breaking the boot-time chain in router.go).
|
||||
//
|
||||
// We can't pass nil for the typed pointers and call available()
|
||||
// here because Go interface-with-nil-pointer is non-nil at the
|
||||
// interface level — `available()` would not detect that as
|
||||
// "unwired". The unwired-plugin behaviour is exhaustively
|
||||
// covered by TestMemoriesV2_PluginUnwired_All503; this test just
|
||||
// pins the fluent contract.
|
||||
h := NewMemoriesV2Handler()
|
||||
got := h.WithMemoryV2(nil, nil)
|
||||
if got != h {
|
||||
t.Error("WithMemoryV2 must return receiver for fluent chaining")
|
||||
}
|
||||
}
|
||||
|
||||
func TestShortID(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"": "",
|
||||
"short": "short",
|
||||
"exactly8": "exactly8",
|
||||
"longer-than-eight": "longer-t",
|
||||
"abc-1234-5678-90ab": "abc-1234",
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := shortID(in); got != want {
|
||||
t.Errorf("shortID(%q) = %q, want %q", in, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -232,6 +232,20 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
|
||||
wsAuth.DELETE("/memories/:memoryId", memsh.Delete)
|
||||
wsAuth.PATCH("/memories/:memoryId", memsh.Update)
|
||||
|
||||
// Memory v2 — canvas reads through the plugin so the Memory
|
||||
// tab surfaces post-cutover state (memory_records) instead
|
||||
// of the frozen agent_memories table that memsh.Search hits.
|
||||
// Wired only when MEMORY_PLUGIN_URL is configured; absent
|
||||
// plugin → endpoints return 503 with a clear hint instead
|
||||
// of nil-deref crashing the canvas.
|
||||
memv2 := handlers.NewMemoriesV2Handler()
|
||||
if memBundle != nil {
|
||||
memv2.WithMemoryV2(memBundle.Plugin, memBundle.Resolver)
|
||||
}
|
||||
wsAuth.GET("/v2/namespaces", memv2.Namespaces)
|
||||
wsAuth.GET("/v2/memories", memv2.Search)
|
||||
wsAuth.DELETE("/v2/memories/:memoryId", memv2.Forget)
|
||||
|
||||
// Approvals
|
||||
apph := handlers.NewApprovalsHandler(broadcaster)
|
||||
wsAuth.POST("/approvals", apph.Create)
|
||||
|
||||
@ -584,6 +584,24 @@ async def send_a2a_message(peer_id: str, message: str, source_workspace_id: str
|
||||
else:
|
||||
detail = "JSON-RPC error with no message"
|
||||
return f"{_A2A_ERROR_PREFIX}{detail} [target={target_url}]"
|
||||
elif data.get("status") == "queued" and data.get("delivery_mode") == "poll":
|
||||
# Workspace-server's poll-mode short-circuit envelope
|
||||
# (workspace-server/internal/handlers/a2a_proxy.go ~line 402).
|
||||
# The peer is poll-mode and has no URL to dispatch to, so
|
||||
# the server queued the message for the peer's next inbox
|
||||
# poll instead of forwarding it. Delivery is acknowledged
|
||||
# but pending consumption.
|
||||
#
|
||||
# Pre-fix this fell through to the "unexpected response
|
||||
# shape" error path → callers logged false failures, then
|
||||
# delegate_task retried, and the peer received duplicate
|
||||
# delegations. Issue #2967.
|
||||
method = data.get("method") or "message/send"
|
||||
logger.info(
|
||||
"send_a2a_message: queued for poll-mode peer (method=%s, target=%s)",
|
||||
method, target_url,
|
||||
)
|
||||
return f"queued for poll-mode peer (method={method})"
|
||||
return f"{_A2A_ERROR_PREFIX}unexpected response shape (no result, no error): {str(data)[:200]} [target={target_url}]"
|
||||
except _TRANSIENT_HTTP_ERRORS as e:
|
||||
last_exc = e
|
||||
|
||||
@ -93,7 +93,34 @@ def main() -> None:
|
||||
``{"id": ..., "token": ...}`` entries. One register + heartbeat
|
||||
+ inbox poller per entry; messages from any workspace land in
|
||||
the same agent inbox tagged with ``arrival_workspace_id``.
|
||||
|
||||
Subcommand:
|
||||
``molecule-mcp doctor`` runs an onboarding diagnostic against the
|
||||
current shell environment + platform reachability and exits.
|
||||
Closes Ryan's #2934 item 6.
|
||||
"""
|
||||
# Subcommand dispatch — must come BEFORE env-var validation so
|
||||
# `molecule-mcp doctor` can run on a partially-configured shell
|
||||
# and tell the operator what's missing. Argv shapes:
|
||||
# molecule-mcp → run server (this function's main path)
|
||||
# molecule-mcp doctor → run diagnostic, exit
|
||||
# molecule-mcp --help → defer to doctor for now (no other
|
||||
# flags are supported yet)
|
||||
if len(sys.argv) > 1:
|
||||
if sys.argv[1] in ("doctor", "--doctor"):
|
||||
import mcp_doctor
|
||||
sys.exit(mcp_doctor.run())
|
||||
if sys.argv[1] in ("--help", "-h", "help"):
|
||||
print(
|
||||
"molecule-mcp — Molecule AI universal MCP server\n\n"
|
||||
"Usage:\n"
|
||||
" molecule-mcp Run the MCP stdio server (registers + heartbeats)\n"
|
||||
" molecule-mcp doctor Run onboarding diagnostic + exit\n\n"
|
||||
"Required env: PLATFORM_URL, WORKSPACE_ID (or MOLECULE_WORKSPACES),\n"
|
||||
" MOLECULE_WORKSPACE_TOKEN (or MOLECULE_WORKSPACE_TOKEN_FILE)\n",
|
||||
)
|
||||
sys.exit(0)
|
||||
|
||||
if not os.environ.get("PLATFORM_URL", "").strip():
|
||||
_print_missing_env_help(
|
||||
["PLATFORM_URL"],
|
||||
|
||||
426
workspace/mcp_doctor.py
Normal file
426
workspace/mcp_doctor.py
Normal file
@ -0,0 +1,426 @@
|
||||
"""molecule-mcp doctor — diagnostic subcommand for first-run install.
|
||||
|
||||
Run via ``molecule-mcp doctor``. Prints a checklist of common
|
||||
onboarding failure modes and concrete next-step suggestions for each
|
||||
failed check.
|
||||
|
||||
Closes Ryan's #2934 item 6 ("Add a molecule-mcp doctor subcommand —
|
||||
this single command would have saved me 30 of the 45 minutes").
|
||||
Pairs with #2935 (Python>=3.11 callout, PATH guidance, TOKEN_FILE
|
||||
support) — those fixed the snippet, this gives the operator a way to
|
||||
self-diagnose when something still goes wrong.
|
||||
|
||||
Six checks, in operator-encounter order:
|
||||
|
||||
1. Python version — wheel requires >=3.11 (pip says
|
||||
"no versions found" on older).
|
||||
2. Wheel install — molecule_runtime importable + version reported.
|
||||
3. PATH for molecule-mcp — pip user-site installs land at
|
||||
~/Library/Python/3.X/bin which isn't on
|
||||
PATH on a fresh macOS shell. Most common
|
||||
"claude mcp add can't find molecule-mcp"
|
||||
cause.
|
||||
4. Env vars — PLATFORM_URL set + reachable;
|
||||
WORKSPACE_ID set; auth token resolvable
|
||||
(env or *_FILE or .auth_token).
|
||||
5. Platform health — GET ${PLATFORM_URL}/healthz returns 2xx.
|
||||
Catches DNS/firewall/wrong-scheme issues
|
||||
before the operator hits the real
|
||||
register call.
|
||||
6. Token auth — POST ${PLATFORM_URL}/registry/heartbeat
|
||||
with the resolved workspace_id+token
|
||||
returns 2xx. End-to-end auth verification.
|
||||
Uses heartbeat (idempotent timestamp
|
||||
update) instead of register (UPSERT —
|
||||
would clobber agent_card metadata) so
|
||||
the doctor is safe to run against a
|
||||
live workspace.
|
||||
|
||||
Each check prints one of:
|
||||
[OK] <one-line status>
|
||||
[WARN] <one-line status> next: <fix suggestion>
|
||||
[FAIL] <one-line status> next: <fix suggestion>
|
||||
|
||||
Exit 0 if all pass or only WARNs; exit 1 if any FAIL — so the
|
||||
subcommand is scriptable from CI / install-checks too.
|
||||
|
||||
Out of scope for now (deferred follow-ups):
|
||||
- Claude Code-specific checks (parse ~/.claude.json, verify each
|
||||
MCP entry is plugin-sourced + dev-channels flag is set). That's
|
||||
a separate Claude-Code-specific doctor and lives in the
|
||||
claude-code-channel plugin, not the universal-MCP doctor.
|
||||
- Automated remediation (running the suggested fix). Doctor is
|
||||
a diagnostic tool — it tells the operator what's wrong + how
|
||||
to fix it, doesn't apply changes.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import importlib.metadata
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
from typing import Optional
|
||||
|
||||
# urllib avoids a hard dep on `requests` for the doctor — the real
|
||||
# CLI already imports requests via mcp_heartbeat, but doctor should
|
||||
# keep working even on a partial install where requests is missing
|
||||
# (that itself is a finding worth surfacing).
|
||||
from urllib import request as urllib_request
|
||||
from urllib.error import URLError
|
||||
|
||||
|
||||
# ANSI colors are friendly on TTYs; auto-disable on pipe / NO_COLOR
|
||||
# for CI logs where the escape sequences clutter the diff.
|
||||
def _color(name: str) -> str:
|
||||
if not sys.stdout.isatty() or os.environ.get("NO_COLOR"):
|
||||
return ""
|
||||
return {
|
||||
"green": "\033[32m",
|
||||
"yellow": "\033[33m",
|
||||
"red": "\033[31m",
|
||||
"dim": "\033[2m",
|
||||
"reset": "\033[0m",
|
||||
}.get(name, "")
|
||||
|
||||
|
||||
def _ok(label: str, msg: str) -> None:
|
||||
print(f" {_color('green')}[OK]{_color('reset')} {label}: {msg}")
|
||||
|
||||
|
||||
def _warn(label: str, msg: str, fix: str) -> None:
|
||||
print(f" {_color('yellow')}[WARN]{_color('reset')} {label}: {msg}")
|
||||
print(f" {_color('dim')}next:{_color('reset')} {fix}")
|
||||
|
||||
|
||||
def _fail(label: str, msg: str, fix: str) -> None:
|
||||
print(f" {_color('red')}[FAIL]{_color('reset')} {label}: {msg}")
|
||||
print(f" {_color('dim')}next:{_color('reset')} {fix}")
|
||||
|
||||
|
||||
# Each check returns a "ok" | "warn" | "fail" verdict so the caller
|
||||
# can compute an exit code without re-walking the print stream.
|
||||
Verdict = str # "ok" | "warn" | "fail"
|
||||
|
||||
|
||||
def check_python_version() -> Verdict:
|
||||
label = "Python version"
|
||||
major, minor = sys.version_info[:2]
|
||||
if (major, minor) >= (3, 11):
|
||||
_ok(label, f"Python {major}.{minor} (wheel requires >=3.11)")
|
||||
return "ok"
|
||||
_fail(
|
||||
label,
|
||||
f"Python {major}.{minor} is below the wheel's >=3.11 floor",
|
||||
"upgrade Python (brew install python@3.12 / apt install python3.12) "
|
||||
"or run molecule-mcp via a 3.11+ venv.",
|
||||
)
|
||||
return "fail"
|
||||
|
||||
|
||||
def check_wheel_install() -> Verdict:
|
||||
label = "Wheel install"
|
||||
try:
|
||||
version = importlib.metadata.version("molecule-ai-workspace-runtime")
|
||||
except importlib.metadata.PackageNotFoundError:
|
||||
_fail(
|
||||
label,
|
||||
"molecule-ai-workspace-runtime not found in this interpreter's site-packages",
|
||||
"pip install molecule-ai-workspace-runtime "
|
||||
"(or pipx install molecule-ai-workspace-runtime to get the "
|
||||
"binary on PATH automatically).",
|
||||
)
|
||||
return "fail"
|
||||
try:
|
||||
importlib.import_module("molecule_runtime.mcp_cli")
|
||||
except ImportError as e:
|
||||
_fail(
|
||||
label,
|
||||
f"package found ({version}) but `molecule_runtime.mcp_cli` won't import: {e}",
|
||||
"reinstall the wheel (pip install --force-reinstall "
|
||||
"molecule-ai-workspace-runtime); if it still fails, file "
|
||||
"a bug with the traceback.",
|
||||
)
|
||||
return "fail"
|
||||
_ok(label, f"molecule-ai-workspace-runtime=={version}")
|
||||
return "ok"
|
||||
|
||||
|
||||
def check_path_for_binary() -> Verdict:
|
||||
label = "PATH for molecule-mcp"
|
||||
found = shutil.which("molecule-mcp")
|
||||
if found:
|
||||
_ok(label, f"resolves to {found}")
|
||||
return "ok"
|
||||
# Not on PATH — work out where pip put it so the suggestion is
|
||||
# actionable instead of generic.
|
||||
user_base = os.environ.get("PYTHONUSERBASE")
|
||||
if not user_base:
|
||||
try:
|
||||
import site
|
||||
user_base = site.getuserbase()
|
||||
except Exception:
|
||||
user_base = None
|
||||
hint = (
|
||||
f"add `{user_base}/bin` to PATH"
|
||||
if user_base
|
||||
else "switch to `pipx install molecule-ai-workspace-runtime` so the "
|
||||
"binary lands in pipx's managed bin/ on PATH"
|
||||
)
|
||||
_fail(
|
||||
label,
|
||||
"molecule-mcp not found on PATH",
|
||||
f"{hint}, or invoke via `python -m molecule_runtime.mcp_cli` directly.",
|
||||
)
|
||||
return "fail"
|
||||
|
||||
|
||||
def _resolve_token() -> tuple[Optional[str], Optional[str]]:
|
||||
"""Return ``(token_value, source_label)`` if the operator's
|
||||
environment exposes a token, else ``(None, None)``.
|
||||
|
||||
Single source of truth used by both ``check_env_vars()`` (which
|
||||
only needs the source label) and ``check_register()`` (which
|
||||
needs the actual value to send a Bearer header). Keeping these
|
||||
in one place means a future env-var addition only updates the
|
||||
resolver — not two parallel readers that can drift.
|
||||
"""
|
||||
val = os.environ.get("MOLECULE_WORKSPACE_TOKEN", "").strip()
|
||||
if val:
|
||||
return val, "env MOLECULE_WORKSPACE_TOKEN"
|
||||
file_var = os.environ.get("MOLECULE_WORKSPACE_TOKEN_FILE", "").strip()
|
||||
if file_var:
|
||||
if os.path.isfile(file_var):
|
||||
try:
|
||||
from pathlib import Path as _Path
|
||||
return (
|
||||
_Path(file_var).read_text().strip(),
|
||||
f"file {file_var} (via MOLECULE_WORKSPACE_TOKEN_FILE)",
|
||||
)
|
||||
except OSError:
|
||||
return None, None
|
||||
return None, None
|
||||
# Per-runtime container path used by the in-platform path; rarely
|
||||
# set on external setups but check anyway so the message is
|
||||
# accurate for both shapes.
|
||||
try:
|
||||
import configs_dir
|
||||
candidate = configs_dir.resolve() / ".auth_token"
|
||||
if candidate.is_file():
|
||||
try:
|
||||
return candidate.read_text().strip(), f"file {candidate}"
|
||||
except OSError:
|
||||
return None, None
|
||||
except Exception:
|
||||
pass
|
||||
return None, None
|
||||
|
||||
|
||||
def _resolve_token_summary() -> Optional[str]:
|
||||
"""Return just the source label (no secret value). Convenience
|
||||
wrapper around :func:`_resolve_token` for callers that don't
|
||||
need the value itself.
|
||||
"""
|
||||
_, label = _resolve_token()
|
||||
return label
|
||||
|
||||
|
||||
def check_env_vars() -> Verdict:
|
||||
label = "Env vars"
|
||||
missing: list[str] = []
|
||||
if not os.environ.get("PLATFORM_URL", "").strip():
|
||||
missing.append("PLATFORM_URL")
|
||||
if not os.environ.get("WORKSPACE_ID", "").strip() and not os.environ.get(
|
||||
"MOLECULE_WORKSPACES", "",
|
||||
).strip():
|
||||
missing.append("WORKSPACE_ID (or MOLECULE_WORKSPACES)")
|
||||
token_summary = _resolve_token_summary()
|
||||
if not token_summary and not os.environ.get("MOLECULE_WORKSPACES", "").strip():
|
||||
# MOLECULE_WORKSPACES is a JSON-array env that bundles its
|
||||
# own per-workspace tokens — if it's set we trust the
|
||||
# resolver to validate.
|
||||
missing.append(
|
||||
"MOLECULE_WORKSPACE_TOKEN (or MOLECULE_WORKSPACE_TOKEN_FILE, or "
|
||||
"/configs/.auth_token)",
|
||||
)
|
||||
if missing:
|
||||
_fail(
|
||||
label,
|
||||
f"unset: {', '.join(missing)}",
|
||||
"see the canvas Connect-External-Agent modal — the snippet "
|
||||
"exports all three. Use MOLECULE_WORKSPACE_TOKEN_FILE for the "
|
||||
"token to keep secrets out of shell history.",
|
||||
)
|
||||
return "fail"
|
||||
_ok(
|
||||
label,
|
||||
f"PLATFORM_URL + WORKSPACE_ID set; token from {token_summary or 'MOLECULE_WORKSPACES'}",
|
||||
)
|
||||
return "ok"
|
||||
|
||||
|
||||
def _http_get(url: str, timeout: float = 5.0) -> tuple[Optional[int], Optional[str]]:
|
||||
"""Best-effort GET that swallows transport errors and returns
|
||||
(status, error_message). Status is None when the request couldn't
|
||||
complete; error_message is None when the request returned 2xx.
|
||||
"""
|
||||
try:
|
||||
# Origin header — staging tenants enforce same-origin via WAF;
|
||||
# /healthz tolerates either way but matching production headers
|
||||
# surfaces auth-style 401s correctly during the doctor run.
|
||||
req = urllib_request.Request(
|
||||
url,
|
||||
headers={"Origin": os.environ.get("PLATFORM_URL", "").rstrip("/")},
|
||||
)
|
||||
with urllib_request.urlopen(req, timeout=timeout) as resp:
|
||||
return resp.status, None
|
||||
except URLError as e:
|
||||
return None, str(e.reason if hasattr(e, "reason") else e)
|
||||
except Exception as e:
|
||||
return None, str(e)
|
||||
|
||||
|
||||
def check_platform_health() -> Verdict:
|
||||
label = "Platform reachability"
|
||||
base = os.environ.get("PLATFORM_URL", "").strip().rstrip("/")
|
||||
if not base:
|
||||
_warn(label, "skipped (PLATFORM_URL unset — see Env vars)", "set PLATFORM_URL first")
|
||||
return "warn"
|
||||
if not base.startswith(("http://", "https://")):
|
||||
_fail(
|
||||
label,
|
||||
f"PLATFORM_URL missing scheme: {base!r}",
|
||||
"set PLATFORM_URL to include https:// — e.g. "
|
||||
"PLATFORM_URL=https://your-tenant.staging.moleculesai.app",
|
||||
)
|
||||
return "fail"
|
||||
if base.endswith("/"):
|
||||
_warn(
|
||||
label,
|
||||
"PLATFORM_URL has trailing slash (will be stripped automatically)",
|
||||
"remove the trailing slash to match the snippet shape",
|
||||
)
|
||||
status, err = _http_get(f"{base}/healthz")
|
||||
if status is None:
|
||||
_fail(label, f"GET {base}/healthz failed: {err}", "check DNS + firewall + scheme")
|
||||
return "fail"
|
||||
if not (200 <= status < 300):
|
||||
_fail(label, f"GET {base}/healthz returned HTTP {status}", "verify the tenant subdomain is correct + provisioned")
|
||||
return "fail"
|
||||
_ok(label, f"GET {base}/healthz → {status}")
|
||||
return "ok"
|
||||
|
||||
|
||||
def check_token_auth() -> Verdict:
|
||||
"""Light auth check via POST /registry/heartbeat.
|
||||
|
||||
Why heartbeat and not register: register is an UPSERT — sending
|
||||
it from doctor would clobber the workspace's actual agent_card
|
||||
(name, description, version) until the real agent next calls
|
||||
register. That's an invisible production-disruption: someone
|
||||
runs ``molecule-mcp doctor`` against a live workspace and the
|
||||
canvas briefly displays "doctor-probe" as the agent name.
|
||||
|
||||
Heartbeat only updates last_heartbeat_at (and clears
|
||||
awaiting_agent if needed) — that's exactly what a normal
|
||||
molecule-mcp boot does every 20s, so an extra heartbeat from
|
||||
the doctor is indistinguishable from background traffic.
|
||||
|
||||
Skipped when env vars failed earlier so the operator isn't shown
|
||||
a redundant 401.
|
||||
"""
|
||||
label = "Token auth"
|
||||
base = os.environ.get("PLATFORM_URL", "").strip().rstrip("/")
|
||||
workspace_id = os.environ.get("WORKSPACE_ID", "").strip()
|
||||
token, source_label = _resolve_token()
|
||||
if not (base and workspace_id and token):
|
||||
_warn(label, "skipped (Env vars must pass first)", "fix Env vars, re-run")
|
||||
return "warn"
|
||||
import json
|
||||
body = json.dumps({"id": workspace_id}).encode()
|
||||
req = urllib_request.Request(
|
||||
f"{base}/registry/heartbeat",
|
||||
data=body,
|
||||
method="POST",
|
||||
headers={
|
||||
"Authorization": f"Bearer {token}",
|
||||
"Content-Type": "application/json",
|
||||
"Origin": base,
|
||||
},
|
||||
)
|
||||
try:
|
||||
with urllib_request.urlopen(req, timeout=8.0) as resp:
|
||||
status = resp.status
|
||||
except URLError as e:
|
||||
# Pull HTTP code from HTTPError; transport errors don't have one.
|
||||
status = getattr(e, "code", None)
|
||||
err = str(e.reason if hasattr(e, "reason") else e)
|
||||
if status is None:
|
||||
_fail(label, f"POST {base}/registry/heartbeat failed: {err}", "check network")
|
||||
return "fail"
|
||||
except Exception as e:
|
||||
_fail(label, f"POST heartbeat failed: {e}", "check network")
|
||||
return "fail"
|
||||
if status == 401:
|
||||
_fail(
|
||||
label,
|
||||
"401 Unauthorized — token rejected",
|
||||
"tokens are shown only once at workspace-create time; "
|
||||
"re-create the workspace OR rotate via canvas Tokens tab.",
|
||||
)
|
||||
return "fail"
|
||||
if status == 404:
|
||||
_fail(
|
||||
label,
|
||||
f"404 — workspace_id {workspace_id} not found on {base}",
|
||||
"verify WORKSPACE_ID matches a real workspace + the tenant "
|
||||
"subdomain in PLATFORM_URL.",
|
||||
)
|
||||
return "fail"
|
||||
if not (200 <= status < 300):
|
||||
_fail(label, f"POST heartbeat returned HTTP {status}", "see platform logs")
|
||||
return "fail"
|
||||
_ok(label, f"POST {base}/registry/heartbeat → {status} (token from {source_label})")
|
||||
return "ok"
|
||||
|
||||
|
||||
# Back-compat alias: the previous name was check_register, but the
|
||||
# implementation switched to a non-mutating heartbeat probe (see
|
||||
# check_token_auth's docstring). Kept so external test suites or
|
||||
# pinned-import scripts don't break on the rename.
|
||||
check_register = check_token_auth
|
||||
|
||||
|
||||
CHECKS = [
|
||||
check_python_version,
|
||||
check_wheel_install,
|
||||
check_path_for_binary,
|
||||
check_env_vars,
|
||||
check_platform_health,
|
||||
check_token_auth,
|
||||
]
|
||||
|
||||
|
||||
def run() -> int:
|
||||
"""Run all checks and return a process exit code (0 ok, 1 if any fail)."""
|
||||
print("molecule-mcp doctor — onboarding diagnostic")
|
||||
print()
|
||||
verdicts = []
|
||||
for chk in CHECKS:
|
||||
try:
|
||||
verdicts.append(chk())
|
||||
except Exception as e:
|
||||
# A buggy check shouldn't kill the rest of the doctor run.
|
||||
print(f" [BUG] {chk.__name__}: unexpected {type(e).__name__}: {e}")
|
||||
verdicts.append("fail")
|
||||
print()
|
||||
fails = sum(1 for v in verdicts if v == "fail")
|
||||
warns = sum(1 for v in verdicts if v == "warn")
|
||||
if fails:
|
||||
print(f"{fails} check(s) failed, {warns} warning(s). Fix the FAIL items above and re-run.")
|
||||
return 1
|
||||
if warns:
|
||||
print(f"All required checks passed; {warns} warning(s) — review the next-step hints.")
|
||||
return 0
|
||||
print("All checks passed.")
|
||||
return 0
|
||||
@ -273,6 +273,87 @@ class TestSendA2AMessage:
|
||||
assert _TEST_PEER_ID in result
|
||||
assert "/workspaces/" in result and "/a2a" in result
|
||||
|
||||
async def test_poll_queued_envelope_returns_success_string(self):
|
||||
"""Issue #2967: workspace-server's poll-mode short-circuit returns
|
||||
{status:"queued", delivery_mode:"poll", method:...} when the peer
|
||||
has no URL to dispatch to. Pre-fix the bare send_a2a_message parser
|
||||
only knew about JSON-RPC {result, error} keys, so this fell through
|
||||
to the 'unexpected response shape' error path → callers retried,
|
||||
peer got duplicate delegations.
|
||||
|
||||
Pin: poll-queued envelope returns a clean success string that does
|
||||
NOT start with _A2A_ERROR_PREFIX, so callers route it through the
|
||||
normal-outcome path. Verified discriminating: assert_NOT_startswith
|
||||
the error prefix would FAIL on the old code (which returned an
|
||||
error-prefixed string) and PASSES on the new code.
|
||||
"""
|
||||
import a2a_client
|
||||
|
||||
resp = _make_response(200, {
|
||||
"status": "queued",
|
||||
"delivery_mode": "poll",
|
||||
"method": "message/send",
|
||||
})
|
||||
mock_client = _make_mock_client(post_resp=resp)
|
||||
|
||||
with patch("a2a_client.httpx.AsyncClient", return_value=mock_client):
|
||||
result = await a2a_client.send_a2a_message(_TEST_PEER_ID, "task")
|
||||
|
||||
# Discriminating: pre-fix returned a string that startswith
|
||||
# _A2A_ERROR_PREFIX, so this assertion would have FAILED on the
|
||||
# old code. New code returns a queued-success string.
|
||||
assert not result.startswith(a2a_client._A2A_ERROR_PREFIX), (
|
||||
f"poll-queued envelope must not be tagged as A2A error; got: {result!r}"
|
||||
)
|
||||
assert "queued" in result.lower()
|
||||
assert "poll" in result.lower()
|
||||
# The method is included so a structured-log scraper can route by
|
||||
# protocol verb if needed.
|
||||
assert "message/send" in result
|
||||
|
||||
async def test_poll_queued_envelope_with_other_method(self):
|
||||
"""Same envelope but a different a2a_method (the future could add
|
||||
message/sendStream or similar). Pin that the parser doesn't hardcode
|
||||
message/send — whatever method the server echoed is preserved.
|
||||
"""
|
||||
import a2a_client
|
||||
|
||||
resp = _make_response(200, {
|
||||
"status": "queued",
|
||||
"delivery_mode": "poll",
|
||||
"method": "message/sendStream",
|
||||
})
|
||||
mock_client = _make_mock_client(post_resp=resp)
|
||||
|
||||
with patch("a2a_client.httpx.AsyncClient", return_value=mock_client):
|
||||
result = await a2a_client.send_a2a_message(_TEST_PEER_ID, "task")
|
||||
|
||||
assert not result.startswith(a2a_client._A2A_ERROR_PREFIX)
|
||||
assert "message/sendStream" in result
|
||||
|
||||
async def test_status_queued_without_poll_mode_still_falls_through(self):
|
||||
"""Defensive: only the {status:"queued", delivery_mode:"poll"} pair
|
||||
triggers the queued-success branch. A response with status:"queued"
|
||||
but a different delivery_mode (or none) is still 'unexpected' —
|
||||
we don't want to silently swallow a future server bug that emits
|
||||
a partial envelope. Pin both keys are required.
|
||||
"""
|
||||
import a2a_client
|
||||
|
||||
resp = _make_response(200, {
|
||||
"status": "queued",
|
||||
# delivery_mode missing
|
||||
"method": "message/send",
|
||||
})
|
||||
mock_client = _make_mock_client(post_resp=resp)
|
||||
|
||||
with patch("a2a_client.httpx.AsyncClient", return_value=mock_client):
|
||||
result = await a2a_client.send_a2a_message(_TEST_PEER_ID, "task")
|
||||
|
||||
# Falls through — must STILL be tagged as error.
|
||||
assert result.startswith(a2a_client._A2A_ERROR_PREFIX)
|
||||
assert "unexpected response shape" in result
|
||||
|
||||
async def test_exception_returns_error_prefix_and_message(self):
|
||||
"""Network exception → returns _A2A_ERROR_PREFIX + exception text."""
|
||||
import a2a_client
|
||||
|
||||
198
workspace/tests/test_mcp_doctor.py
Normal file
198
workspace/tests/test_mcp_doctor.py
Normal file
@ -0,0 +1,198 @@
|
||||
"""Tests for the molecule-mcp doctor subcommand (#2934 item 6).
|
||||
|
||||
Each `check_*` function is unit-tested in isolation via env
|
||||
manipulation. The integration test (`test_run_no_env_returns_1`) pins
|
||||
the end-to-end exit code on a stripped environment — what an operator
|
||||
running the command for the first time on an untouched shell sees.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
# Workspace tests run from the workspace/ directory; mcp_doctor is
|
||||
# imported with the same `import mcp_doctor` shape as the rest of
|
||||
# the runtime (per pyproject's package layout).
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
import mcp_doctor # noqa: E402
|
||||
|
||||
|
||||
def test_module_exposes_six_checks():
|
||||
"""The doctor's checklist is six items today. Pin the count so
|
||||
a future PR that drops a check (e.g. silently merges two) gets
|
||||
flagged in review.
|
||||
"""
|
||||
assert len(mcp_doctor.CHECKS) == 6
|
||||
|
||||
|
||||
def test_check_python_version_passes_on_311_plus():
|
||||
"""Pin the floor at 3.11 (matches the wheel's requires_python)."""
|
||||
with mock.patch.object(sys, "version_info", (3, 11, 0, "final", 0)):
|
||||
assert mcp_doctor.check_python_version() == "ok"
|
||||
with mock.patch.object(sys, "version_info", (3, 12, 5, "final", 0)):
|
||||
assert mcp_doctor.check_python_version() == "ok"
|
||||
|
||||
|
||||
def test_check_python_version_fails_on_310():
|
||||
"""3.10 is below the wheel's >=3.11 floor — must FAIL, not WARN.
|
||||
pip silently filters the wheel out on 3.10 with `from versions:
|
||||
none`, which reads as "package missing" — operators have spent
|
||||
45min chasing that. The doctor's job is to call this out
|
||||
explicitly.
|
||||
"""
|
||||
with mock.patch.object(sys, "version_info", (3, 10, 12, "final", 0)):
|
||||
assert mcp_doctor.check_python_version() == "fail"
|
||||
|
||||
|
||||
def test_check_env_vars_fails_when_all_unset(monkeypatch):
|
||||
monkeypatch.delenv("PLATFORM_URL", raising=False)
|
||||
monkeypatch.delenv("WORKSPACE_ID", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACES", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False)
|
||||
assert mcp_doctor.check_env_vars() == "fail"
|
||||
|
||||
|
||||
def test_check_env_vars_passes_with_token_env(monkeypatch):
|
||||
monkeypatch.setenv("PLATFORM_URL", "https://x.moleculesai.app")
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-test")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "tok-abc")
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACES", raising=False)
|
||||
assert mcp_doctor.check_env_vars() == "ok"
|
||||
|
||||
|
||||
def test_check_env_vars_passes_with_token_file(monkeypatch, tmp_path):
|
||||
"""Ryan #2934 item 3 fix: token from a file (or keychain shim)
|
||||
instead of inline env var so secrets stay out of shell history.
|
||||
The doctor must accept that path equally with the inline form.
|
||||
"""
|
||||
token_path = tmp_path / "token"
|
||||
token_path.write_text("tok-from-file")
|
||||
monkeypatch.setenv("PLATFORM_URL", "https://x.moleculesai.app")
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-test")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN_FILE", str(token_path))
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACES", raising=False)
|
||||
assert mcp_doctor.check_env_vars() == "ok"
|
||||
|
||||
|
||||
def test_check_platform_health_warns_when_url_unset(monkeypatch):
|
||||
monkeypatch.delenv("PLATFORM_URL", raising=False)
|
||||
assert mcp_doctor.check_platform_health() == "warn"
|
||||
|
||||
|
||||
def test_check_platform_health_fails_on_missing_scheme(monkeypatch):
|
||||
"""A bare hostname is the second-most-common config error after
|
||||
missing-token (per the snippet's NOTE on Origin/PLATFORM_URL).
|
||||
The error message must say 'missing scheme' — not 'DNS error' —
|
||||
so the operator can diagnose without inspecting the URL string.
|
||||
"""
|
||||
monkeypatch.setenv("PLATFORM_URL", "x.moleculesai.app")
|
||||
assert mcp_doctor.check_platform_health() == "fail"
|
||||
|
||||
|
||||
def test_check_register_skipped_without_env(monkeypatch):
|
||||
monkeypatch.delenv("PLATFORM_URL", raising=False)
|
||||
monkeypatch.delenv("WORKSPACE_ID", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN", raising=False)
|
||||
# Skipped (warn), NOT failed — failing here would double-count
|
||||
# the env-vars failure noise.
|
||||
assert mcp_doctor.check_register() == "warn"
|
||||
|
||||
|
||||
def test_check_token_auth_uses_heartbeat_endpoint(monkeypatch):
|
||||
"""Pin: doctor MUST hit /registry/heartbeat, not /registry/register.
|
||||
|
||||
register is an UPSERT — using it from doctor would clobber the
|
||||
workspace's actual agent_card metadata until the real agent next
|
||||
calls register. heartbeat only updates last_heartbeat_at, which
|
||||
a normal molecule-mcp boot does every 20s anyway, so the doctor's
|
||||
extra heartbeat is indistinguishable from background traffic.
|
||||
|
||||
This test pins the URL via a urllib mock so a future refactor
|
||||
that accidentally re-routes through /registry/register fails
|
||||
here at PR-review time, not after operators report
|
||||
"doctor-probe" briefly appearing as their agent name in canvas.
|
||||
"""
|
||||
monkeypatch.setenv("PLATFORM_URL", "https://x.moleculesai.app")
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-test")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "tok-abc")
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False)
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
class _FakeResp:
|
||||
status = 200
|
||||
def __enter__(self): return self
|
||||
def __exit__(self, *a): pass
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
captured["full_url"] = req.full_url
|
||||
captured["method"] = req.get_method()
|
||||
return _FakeResp()
|
||||
|
||||
monkeypatch.setattr(mcp_doctor.urllib_request, "urlopen", fake_urlopen)
|
||||
verdict = mcp_doctor.check_token_auth()
|
||||
assert verdict == "ok"
|
||||
assert captured["method"] == "POST"
|
||||
# The load-bearing assertion — must use heartbeat, never register.
|
||||
assert captured["full_url"].endswith("/registry/heartbeat"), (
|
||||
f"doctor must use /registry/heartbeat (idempotent), not register "
|
||||
f"(UPSERT — clobbers agent_card). Got: {captured['full_url']}"
|
||||
)
|
||||
assert "/registry/register" not in str(captured["full_url"]), (
|
||||
"doctor must NEVER POST to /registry/register — that's a UPSERT "
|
||||
"that overwrites agent_card metadata until the real agent next "
|
||||
"calls register."
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_token_returns_value_and_label_for_env(monkeypatch):
|
||||
"""The single resolver returns both the value (for Bearer header)
|
||||
and a non-secret label (for the env-vars summary). Drift between
|
||||
label and value is the previous bug shape."""
|
||||
monkeypatch.setenv("PLATFORM_URL", "https://x.moleculesai.app")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "secret-tok-abc")
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False)
|
||||
val, label = mcp_doctor._resolve_token()
|
||||
assert val == "secret-tok-abc"
|
||||
assert label == "env MOLECULE_WORKSPACE_TOKEN"
|
||||
# Summary helper must agree with the resolver's source.
|
||||
assert mcp_doctor._resolve_token_summary() == label
|
||||
|
||||
|
||||
def test_resolve_token_returns_none_when_missing(monkeypatch):
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN", raising=False)
|
||||
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False)
|
||||
val, label = mcp_doctor._resolve_token()
|
||||
assert val is None
|
||||
assert label is None
|
||||
|
||||
|
||||
def test_run_returns_1_when_any_fail(monkeypatch, capsys):
|
||||
"""End-to-end: stripped environment → at least one FAIL →
|
||||
exit 1. Pin the exit-code contract so this is scriptable from
|
||||
CI / install-checks too.
|
||||
"""
|
||||
for k in (
|
||||
"PLATFORM_URL",
|
||||
"WORKSPACE_ID",
|
||||
"MOLECULE_WORKSPACES",
|
||||
"MOLECULE_WORKSPACE_TOKEN",
|
||||
"MOLECULE_WORKSPACE_TOKEN_FILE",
|
||||
):
|
||||
monkeypatch.delenv(k, raising=False)
|
||||
code = mcp_doctor.run()
|
||||
out = capsys.readouterr().out
|
||||
assert code == 1
|
||||
# The summary line must mention at least one failure count so
|
||||
# an automated wrapper can grep for it.
|
||||
assert "check(s) failed" in out
|
||||
# And the human-facing label must be present so someone reading
|
||||
# CI logs sees what the section is about, not a wall of [FAIL].
|
||||
assert "molecule-mcp doctor" in out
|
||||
Loading…
Reference in New Issue
Block a user