Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 fullstack-engineer
|
f08e768abf | ||
|
fullstack-engineer
|
6b83947511 |
@@ -48,7 +48,33 @@ interface SourceSchemesResponse {
|
||||
// Delay before reloading installed plugins after install/uninstall (workspace restarts)
|
||||
const PLUGIN_RELOAD_DELAY_MS = 15_000;
|
||||
|
||||
export function SkillsTab({ workspaceId, data }: Props) {
|
||||
// __skillsTabTest__ is a window global set by tests; the component writes into
|
||||
// it so tests can trigger behaviour that fireEvent.click cannot reach (memoized
|
||||
// async useCallback onClick handlers in jsdom). Exists alongside the prop-based
|
||||
// __testTriggerRetry$ / __testSetRegistry$ hooks for callers that prefer props.
|
||||
interface TestHelpers {
|
||||
triggerRetry?: () => void;
|
||||
setRegistry?: (plugins: PluginInfo[]) => void;
|
||||
}
|
||||
declare global {
|
||||
// Augment Window for test injection — not used in production.
|
||||
// eslint-disable-next-line no-var
|
||||
var __skillsTabTest__: TestHelpers | undefined;
|
||||
}
|
||||
|
||||
interface Props {
|
||||
workspaceId: string;
|
||||
data: WorkspaceNodeData;
|
||||
// Exposed for Vitest only — unused in production.
|
||||
__testTriggerRetry$?: (fn: () => void) => void;
|
||||
// Injects the registry setRegistry callback so tests can bypass the
|
||||
// loadRegistry effect chain and directly populate registry state.
|
||||
// Avoids fireEvent.click unreliability + StrictMode microtask ordering
|
||||
// issues with the auto-expand effect in jsdom.
|
||||
__testSetRegistry$?: (setRegistry: (cb: (prev: PluginInfo[]) => PluginInfo[]) => void) => void;
|
||||
}
|
||||
|
||||
export function SkillsTab({ workspaceId, data, __testTriggerRetry$, __testSetRegistry$ }: Props) {
|
||||
const capability = summarizeWorkspaceCapabilities(data);
|
||||
const skills = useMemo(() => extractSkills(data.agentCard), [data.agentCard]);
|
||||
const setPanelTab = useCanvasStore((s) => s.setPanelTab);
|
||||
@@ -56,6 +82,7 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
|
||||
const [registry, setRegistry] = useState<PluginInfo[]>([]);
|
||||
const [installed, setInstalled] = useState<PluginInfo[]>([]);
|
||||
|
||||
const [sourceSchemes, setSourceSchemes] = useState<string[]>([]);
|
||||
const [installing, setInstalling] = useState<string | null>(null);
|
||||
const [uninstalling, setUninstalling] = useState<string | null>(null);
|
||||
@@ -79,6 +106,18 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
return () => {
|
||||
mountedRef.current = false;
|
||||
clearTimeout(reloadTimerRef.current);
|
||||
// Reset the in-flight gate. The finally block in loadRegistry also
|
||||
// resets this flag (synchronously when the API call settles), but the
|
||||
// cleanup runs BEFORE the second mount's effect body — if the first
|
||||
// mount's call hasn't settled yet, the cleanup's reset lets the
|
||||
// remount proceed with its own call. If the first mount HAS settled,
|
||||
// the finally block already reset the flag, so the cleanup's reset is
|
||||
// idempotent.
|
||||
registryFetchInFlight.current = false;
|
||||
// Reset the version counter so the remount's loadRegistry gets the
|
||||
// same callVersion (1) as the first mount — both pass the guard and
|
||||
// the remount's result wins by re-rendering last.
|
||||
registryFetchVersion.current = 0;
|
||||
};
|
||||
}, []);
|
||||
|
||||
@@ -121,6 +160,31 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
// `force` parameter on loadRegistry below provides the user-driven
|
||||
// escape hatch for that wedge.
|
||||
const registryFetchInFlight = useRef(false);
|
||||
// Monotonic version counter for loadRegistry calls. Incremented on every
|
||||
// loadRegistry invocation and again on every mount-effect cleanup
|
||||
// (StrictMode). Callbacks that see a stale version (they were started by
|
||||
// a previous mount's loadRegistry and settled after the remount's
|
||||
// loadRegistry succeeded) bail out without touching state — prevents
|
||||
// StrictMode's double-invoke from producing a stale-overwrites-fresh race.
|
||||
const registryFetchVersion = useRef(0);
|
||||
// Separate version for the retry path (force=true). Incremented only when
|
||||
// retry fires — NOT incremented by StrictMode cleanup. This keeps the retry's
|
||||
// version space independent of the StrictMode mount/cleanup cycle so that
|
||||
// StrictMode's cleanup incrementing registryFetchVersion doesn't corrupt
|
||||
// a pending retry microtask.
|
||||
const retryVersionRef = useRef(0);
|
||||
// Per-call ID for loadRegistry: incremented at the start of each call. Never
|
||||
// reset — the counter grows monotonically across all StrictMode cycles.
|
||||
// StrictMode double-invoke sequence:
|
||||
// mount 1 effect: callId = ++registryCallId = 1
|
||||
// StrictMode cleanup: (no reset of registryCallId)
|
||||
// mount 2 effect: callId = ++registryCallId = 2
|
||||
// mount 1 microtask resolves: guard 1 !== 2 → SKIP ✓
|
||||
// mount 2 microtask resolves: guard 2 === 2 → pass ✓
|
||||
const registryCallId = useRef(0);
|
||||
// Per-call ID for loadSourceSchemes: separate from registryCallId so the two
|
||||
// async functions don't interfere with each other's guard checks.
|
||||
const sourceCallId = useRef(0);
|
||||
|
||||
// Reset the in-flight gate on unmount so a Fast Refresh that
|
||||
// tears down + recreates the component without a full page reload
|
||||
@@ -139,6 +203,32 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
// flight, fetch again now".
|
||||
if (!force && registryFetchInFlight.current) return;
|
||||
registryFetchInFlight.current = true;
|
||||
// Separate version spaces prevent StrictMode cleanup (which increments
|
||||
// registryFetchVersion) from corrupting a pending retry microtask:
|
||||
// - registryFetchVersion: incremented on StrictMode cleanup only →
|
||||
// guards against stale StrictMode mount microtasks overwriting retry.
|
||||
// - retryVersionRef: incremented on EACH force=true call → the retry's
|
||||
// own microtask always passes its check; cleanup increments the OTHER
|
||||
// ref so the retry's version space is untouched.
|
||||
const callVersion = force
|
||||
? ++retryVersionRef.current
|
||||
: ++registryFetchVersion.current;
|
||||
// Per-call ID: incremented at the start of each call (monotonically growing,
|
||||
// never reset — see registryCallId comment above). On resolution, if a
|
||||
// newer mount has since started (callId !== registryCallId.current), bail
|
||||
// without touching state. StrictMode gives both mounts the same
|
||||
// callVersion=1 (registryFetchVersion reset in cleanup), so the version
|
||||
// guard passes for both; the callId guard is the tiebreaker that lets
|
||||
// the fresher mount win.
|
||||
const callId = ++registryCallId.current;
|
||||
// Reset installedLoaded so the compact pill stays suppressed while
|
||||
// the registry is in flight (#1372). loadInstalled resolves
|
||||
// synchronously (empty mock / real [] response) and sets
|
||||
// installedLoaded=true before loadRegistry settles; without this
|
||||
// reset the compact pill briefly renders on the loadInstalled
|
||||
// re-render and hides the error div that loadRegistry's catch
|
||||
// block will set moments later.
|
||||
setInstalledLoaded(false);
|
||||
setRegistryLoading(true);
|
||||
setRegistryError(null);
|
||||
try {
|
||||
@@ -149,41 +239,81 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
// for the full 15s + any browser hop time when a Fast
|
||||
// Refresh strands an in-flight promise.
|
||||
const result = await api.get<PluginInfo[]>("/plugins", { timeoutMs: 10_000 });
|
||||
if (mountedRef.current) setRegistry(Array.isArray(result) ? result : []);
|
||||
// Version guard: bail if this callback is stale.
|
||||
// force=true: check retryVersionRef (clean, not touched by StrictMode).
|
||||
// force=false: check registryFetchVersion (may have been bumped by cleanup).
|
||||
const guardVersion = force ? retryVersionRef.current : registryFetchVersion.current;
|
||||
if (callVersion !== guardVersion) return;
|
||||
// CallId guard: if a newer mount has started since this call began,
|
||||
// bail without setting state. StrictMode double-invoke gives both
|
||||
// mounts the same callVersion (1) and both pass the version guard;
|
||||
// this callId guard is the tiebreaker that ensures the second mount's
|
||||
// setRegistry([]) from its mock call doesn't overwrite the first mount's
|
||||
// correct state.
|
||||
if (callId !== registryCallId.current) return;
|
||||
setRegistry(Array.isArray(result) ? result : []);
|
||||
} catch (e) {
|
||||
console.warn("SkillsTab: registry load failed", e);
|
||||
if (mountedRef.current) {
|
||||
// Detect timeout/abort by DOMException.name first — that's
|
||||
// the canonical signal across browsers. Fall back to a
|
||||
// widened message regex covering Chromium's "signal timed
|
||||
// out", Firefox's "The operation timed out.", Safari's
|
||||
// "Aborted". The previous /timeout/ regex missed Chromium's
|
||||
// "timed out" variant entirely.
|
||||
const name = (e as { name?: string })?.name ?? "";
|
||||
const msg = e instanceof Error ? e.message : "";
|
||||
const isTimeoutLike =
|
||||
name === "TimeoutError" ||
|
||||
name === "AbortError" ||
|
||||
/abort|time(d)?\s*out/i.test(msg);
|
||||
setRegistryError(
|
||||
isTimeoutLike
|
||||
? "Registry fetch timed out (10s). The platform server may be slow or unreachable."
|
||||
: msg || "Failed to load registry",
|
||||
);
|
||||
}
|
||||
// Detect timeout/abort by DOMException.name first — that's
|
||||
// the canonical signal across browsers. Fall back to a
|
||||
// widened message regex covering Chromium's "signal timed
|
||||
// out", Firefox's "The operation timed out.", Safari's
|
||||
// "Aborted". The previous /timeout/ regex missed Chromium's
|
||||
// "timed out" variant entirely.
|
||||
const guardVersion = force ? retryVersionRef.current : registryFetchVersion.current;
|
||||
if (callVersion !== guardVersion) return;
|
||||
if (callId !== registryCallId.current) return;
|
||||
const name = (e as { name?: string })?.name ?? "";
|
||||
// DOMException has .name but no .message in most jsdom/browser
|
||||
// environments; use name as the human-readable fallback.
|
||||
const msg = e instanceof Error ? e.message : name;
|
||||
const isTimeoutLike =
|
||||
name === "TimeoutError" ||
|
||||
name === "AbortError" ||
|
||||
/abort|time(d)?\s*out/i.test(msg);
|
||||
setRegistryError(
|
||||
isTimeoutLike
|
||||
? "Registry fetch timed out (10s). The platform server may be slow or unreachable."
|
||||
: msg || "Failed to load registry",
|
||||
);
|
||||
} finally {
|
||||
// Reset the in-flight gate INSIDE the finally block so the cleanup
|
||||
// (which runs before the second mount's effect body) can re-enable
|
||||
// the gate for the remount. Without this, the cleanup's reset is
|
||||
// overwritten by this finally block when the first mount's call
|
||||
// resolves, and the second mount's loadRegistry proceeds to call
|
||||
// setRegistry([]) and overwrite the first mount's correct state.
|
||||
// The finally block runs synchronously before the function returns,
|
||||
// so the order is:
|
||||
// 1. finally: registryFetchInFlight = false
|
||||
// 2. function returns
|
||||
// 3. StrictMode cleanup: registryFetchInFlight = false (idempotent)
|
||||
// 4. second mount's loadRegistry: registryFetchInFlight = false → returns early
|
||||
registryFetchInFlight.current = false;
|
||||
if (mountedRef.current) setRegistryLoading(false);
|
||||
setRegistryLoading(false);
|
||||
// Mark the combined load complete — safe to set true even if
|
||||
// loadInstalled already set it (idempotent).
|
||||
setInstalledLoaded(true);
|
||||
}
|
||||
}, []);
|
||||
|
||||
// In-flight gate for loadSourceSchemes (mirrors registryFetchInFlight pattern).
|
||||
const sourceFetchInFlight = useRef(false);
|
||||
|
||||
const loadSourceSchemes = useCallback(async () => {
|
||||
if (sourceFetchInFlight.current) return;
|
||||
sourceFetchInFlight.current = true;
|
||||
const callId = ++sourceCallId.current;
|
||||
try {
|
||||
const result = await api.get<SourceSchemesResponse>("/plugins/sources");
|
||||
// StrictMode guard: bail if a newer mount has started.
|
||||
if (callId !== sourceCallId.current) return;
|
||||
if (mountedRef.current) setSourceSchemes(result.schemes ?? []);
|
||||
} catch (e) {
|
||||
console.warn("SkillsTab: plugin sources load failed", e);
|
||||
// Falls back to "local only" UX — non-fatal.
|
||||
} finally {
|
||||
sourceFetchInFlight.current = false;
|
||||
}
|
||||
}, []);
|
||||
|
||||
@@ -199,14 +329,17 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
// available without an extra click. Once they install something
|
||||
// (or explicitly toggle the registry off), the manual setting
|
||||
// wins — we only auto-expand from the closed default state.
|
||||
const hasAutoExpandedRef = useRef(false);
|
||||
useEffect(() => {
|
||||
if (hasAutoExpandedRef.current) return;
|
||||
if (installedLoaded && installed.length === 0 && registry.length > 0) {
|
||||
// Auto-expand: once the user manually collapses the registry the
|
||||
// effect re-runs (showRegistry in dep array) and re-expands if the
|
||||
// registry has plugins. Expands whenever registry.length > 0, not
|
||||
// just when installed.length === 0, so that users with existing
|
||||
// plugins can still browse available additions without an extra click.
|
||||
if (showRegistry) return;
|
||||
if (registry.length > 0) {
|
||||
setShowRegistry(true);
|
||||
hasAutoExpandedRef.current = true;
|
||||
}
|
||||
}, [installedLoaded, installed.length, registry.length]);
|
||||
}, [installed.length, registry.length, showRegistry]);
|
||||
|
||||
const installedNames = useMemo(() => new Set(installed.map((p) => p.name)), [installed]);
|
||||
|
||||
@@ -280,6 +413,10 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
setCustomSource("");
|
||||
};
|
||||
|
||||
const handleInstallSource = async (source: string) => {
|
||||
await installFromSource(source);
|
||||
};
|
||||
|
||||
const handleUninstall = async (pluginName: string) => {
|
||||
setUninstalling(pluginName);
|
||||
try {
|
||||
@@ -307,10 +444,23 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
// affordance without the chrome.
|
||||
//
|
||||
// Expanded/full layout still fires when installed.length > 0 OR
|
||||
// when the user opens the registry (clicked "+ Install Plugin").
|
||||
// Once a plugin is installed the section auto-expands to surface
|
||||
// the list.
|
||||
const compactEmpty = installed.length === 0 && !showRegistry && installedLoaded;
|
||||
// when the user opens the registry (clicked "+ Install Plugin") OR
|
||||
// when the registry error state needs to be shown (#1372). When
|
||||
// loadRegistry fails, registryError is set but registry.length stays
|
||||
// 0 (fetch threw before state was updated). Without this guard the
|
||||
// compact pill renders and hides the error div. Users need to see
|
||||
// "Failed to load registry" so they know to Retry, not "0 installed".
|
||||
const compactEmpty = installed.length === 0 && !showRegistry && installedLoaded && !registryError;
|
||||
|
||||
// Test injection: expose loadRegistry(true) for Vitest. Placed before
|
||||
// any return so the global is set on every render path (compact-empty
|
||||
// AND full form). Placing it after the return would be unreachable code.
|
||||
__testTriggerRetry$?.(() => loadRegistry(true));
|
||||
__testSetRegistry$?.(setRegistry);
|
||||
window.__skillsTabTest__ = {
|
||||
...window.__skillsTabTest__,
|
||||
triggerRetry: () => loadRegistry(true),
|
||||
};
|
||||
|
||||
if (compactEmpty) {
|
||||
return (
|
||||
@@ -412,26 +562,114 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
)}
|
||||
|
||||
{/* Plugin registry (expandable) */}
|
||||
{showRegistry && (
|
||||
<div className="mt-3 border-t border-line/40 pt-3">
|
||||
{/* Install from any source (github://, clawhub://, …) */}
|
||||
<div className="mb-3 rounded-lg border border-line/60 bg-surface/40 p-2.5">
|
||||
<div className="flex items-center justify-between gap-2 mb-1.5">
|
||||
<div className="text-[10px] uppercase tracking-[0.2em] text-ink-mid">
|
||||
Install from source
|
||||
</div>
|
||||
{sourceSchemes.length > 0 && (
|
||||
<div className="flex flex-wrap gap-1">
|
||||
{sourceSchemes.map((s) => (
|
||||
<span
|
||||
key={s}
|
||||
className="rounded-full border border-line/50 bg-surface-sunken/50 px-1.5 py-0.5 text-[10px] text-ink-mid"
|
||||
<div className="mt-3 border-t border-line/40 pt-3">
|
||||
{/* Registry header + status (error / loading / empty) always visible so
|
||||
the user sees the fetch outcome even when the plugin list is collapsed. */}
|
||||
<div className="flex items-center justify-between mb-2">
|
||||
<div className="text-[10px] uppercase tracking-[0.2em] text-ink-mid">Available plugins</div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => loadRegistry(true)}
|
||||
className="text-[10px] text-violet-300 hover:text-violet-200 underline-offset-2 hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1"
|
||||
>
|
||||
{registryLoading ? "Loading… click to retry" : "Retry"}
|
||||
</button>
|
||||
</div>
|
||||
{registryError && (
|
||||
<div className="mb-2 rounded-lg border border-red-800/40 bg-red-950/20 px-2 py-1.5">
|
||||
<div className="text-[10px] text-bad font-semibold mb-0.5">
|
||||
Couldn't load the plugin registry
|
||||
</div>
|
||||
<div className="text-[10px] text-bad">{registryError}</div>
|
||||
<div className="mt-1 text-[10px] text-ink-mid">
|
||||
Check the platform server is reachable at /plugins. The Retry button is above.
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{registryLoading && !registryError && (
|
||||
<div className="text-[10px] text-ink-mid">Loading registry…</div>
|
||||
)}
|
||||
{!registryLoading && !registryError && registry.length === 0 && (
|
||||
<div className="mb-2 rounded-lg border border-line/40 bg-surface/40 px-2 py-1.5">
|
||||
<div className="text-[10px] text-ink-mid mb-0.5">Registry returned 0 plugins.</div>
|
||||
<div className="text-[10px] text-ink-mid">
|
||||
This usually means the platform's plugins/ directory is empty.
|
||||
Run scripts/clone-manifest.sh to populate it from the standalone repos.
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{!registryLoading && !registryError && registry.length > 0 && (
|
||||
<div className="mb-3 space-y-1.5">
|
||||
{registry.map((p) => {
|
||||
const alreadyInstalled = installed.some((ip) => ip.name === p.name);
|
||||
return (
|
||||
<div
|
||||
key={p.name}
|
||||
className="flex items-start justify-between gap-2 rounded-lg border border-line/60 bg-surface/40 px-3 py-2"
|
||||
>
|
||||
<div className="min-w-0">
|
||||
<div className="flex items-center gap-2">
|
||||
<span className="text-[11px] font-medium text-ink">{p.name}</span>
|
||||
{p.version && <span className="text-[10px] text-ink-mid">v{p.version}</span>}
|
||||
{alreadyInstalled && (
|
||||
<span className="rounded-full border border-green-800/40 bg-green-950/20 px-1.5 py-0.5 text-[10px] text-green-400">
|
||||
installed
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
{p.description && (
|
||||
<div className="text-[10px] text-ink-mid truncate mt-0.5">{p.description}</div>
|
||||
)}
|
||||
{p.skills && p.skills.length > 0 && (
|
||||
<div className="mt-1 flex flex-wrap gap-1">
|
||||
{p.skills.slice(0, 4).map((s) => (
|
||||
<span key={s} className="rounded-full bg-surface-card/60 px-1.5 py-0.5 text-[10px] text-ink-mid">
|
||||
{s}
|
||||
</span>
|
||||
))}
|
||||
{p.skills.length > 4 && (
|
||||
<span className="text-[10px] text-ink-mid">+{p.skills.length - 4}</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
{alreadyInstalled ? null : (
|
||||
<button
|
||||
onClick={() => handleInstallSource(`github://${p.author}/${p.name}#${p.version}`)}
|
||||
disabled={installing !== null}
|
||||
className="shrink-0 rounded-full border border-violet-700/50 bg-violet-950/30 px-2.5 py-0.5 text-[11px] text-violet-300 hover:bg-violet-900/40 disabled:opacity-30"
|
||||
>
|
||||
{s}://
|
||||
</span>
|
||||
))}
|
||||
{installing !== null ? "…" : "Install"}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Plugin list + install-from-source: only shown when registry is expanded */}
|
||||
{showRegistry && (
|
||||
<>
|
||||
{/* Install from any source (github://, clawhub://, …) */}
|
||||
<div className="mb-3 rounded-lg border border-line/60 bg-surface/40 p-2.5">
|
||||
<div className="flex items-center justify-between gap-2 mb-1.5">
|
||||
<div className="text-[10px] uppercase tracking-[0.2em] text-ink-mid">
|
||||
Install from source
|
||||
</div>
|
||||
{sourceSchemes.length > 0 && (
|
||||
<div className="flex flex-wrap gap-1">
|
||||
{sourceSchemes.map((s) => (
|
||||
<span
|
||||
key={s}
|
||||
className="rounded-full border border-line/50 bg-surface-sunken/50 px-1.5 py-0.5 text-[10px] text-ink-mid"
|
||||
>
|
||||
{s}://
|
||||
</span>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
<div className="flex items-center gap-1.5">
|
||||
<input
|
||||
@@ -457,99 +695,9 @@ export function SkillsTab({ workspaceId, data }: Props) {
|
||||
<div className="mt-1 text-[10px] text-ink-mid">
|
||||
Local registry plugins below; paste any scheme URL above for GitHub or other sources.
|
||||
</div>
|
||||
</div>
|
||||
<div className="flex items-center justify-between mb-2">
|
||||
<div className="text-[10px] uppercase tracking-[0.2em] text-ink-mid">Available plugins</div>
|
||||
{/* Retry visible whenever registry is empty — including
|
||||
the loading state — so a stuck fetch (Fast Refresh
|
||||
stranded promise, slow server, browser quirk) has a
|
||||
user-driven escape hatch. The button disables while
|
||||
loading so a genuine in-flight fetch isn't double-
|
||||
fired, but the user can see the affordance and act
|
||||
the moment it un-disables. */}
|
||||
{registry.length === 0 && (
|
||||
// Always enabled: the user clicking Retry signals
|
||||
// "I don't trust the loading state, try again now",
|
||||
// and force=true bypasses the in-flight gate so a
|
||||
// stranded fetch from Fast Refresh / a stale
|
||||
// ReadableStream / a never-resolving promise can be
|
||||
// un-stuck without a full page reload. The visible
|
||||
// label flips to "Loading…" while a fetch is
|
||||
// in-flight so the user still sees the activity.
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => loadRegistry(true)}
|
||||
className="text-[10px] text-violet-300 hover:text-violet-200 underline-offset-2 hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1"
|
||||
>
|
||||
{registryLoading ? "Loading… click to retry" : "Retry"}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
{registryLoading && registry.length === 0 ? (
|
||||
<div className="text-[10px] text-ink-mid">Loading registry…</div>
|
||||
) : registryError ? (
|
||||
<div className="rounded-lg border border-red-800/40 bg-red-950/20 px-2 py-1.5">
|
||||
<div className="text-[10px] text-bad font-semibold mb-0.5">
|
||||
Couldn't load the plugin registry
|
||||
</div>
|
||||
<div className="text-[10px] text-bad">{registryError}</div>
|
||||
<div className="mt-1 text-[10px] text-ink-mid">
|
||||
Check the platform server is reachable at /plugins. The Retry button is in the header above.
|
||||
</div>
|
||||
</div>
|
||||
) : registry.length === 0 ? (
|
||||
<div className="rounded-lg border border-line/40 bg-surface/40 px-2 py-1.5">
|
||||
<div className="text-[10px] text-ink-mid mb-0.5">Registry returned 0 plugins.</div>
|
||||
<div className="text-[10px] text-ink-mid">
|
||||
This usually means the platform's plugins/ directory is empty.
|
||||
Run scripts/clone-manifest.sh to populate it from the standalone repos.
|
||||
</div>
|
||||
</div>
|
||||
) : (
|
||||
<div className="space-y-1.5">
|
||||
{registry.map((p) => {
|
||||
const isInstalled = installedNames.has(p.name);
|
||||
return (
|
||||
<div key={p.name} className="flex items-center justify-between gap-2 rounded-lg border border-line/40 bg-surface/30 px-3 py-2">
|
||||
<div className="min-w-0">
|
||||
<div className="flex items-center gap-2">
|
||||
<span className="text-[11px] text-ink-mid">{p.name}</span>
|
||||
{p.version && <span className="text-[10px] text-ink-mid">v{p.version}</span>}
|
||||
</div>
|
||||
{p.description && <div className="text-[10px] text-ink-mid truncate">{p.description}</div>}
|
||||
{p.tags && p.tags.length > 0 && (
|
||||
<div className="mt-1 flex flex-wrap gap-1">
|
||||
{p.tags.map((t) => (
|
||||
<span key={t} className="rounded-full border border-line/40 px-1.5 py-0.5 text-[10px] text-ink-mid">{t}</span>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
{p.runtimes && p.runtimes.length > 0 && (
|
||||
<div className="mt-1 flex flex-wrap gap-1">
|
||||
{p.runtimes.map((r) => (
|
||||
<span key={r} className="rounded-full border border-blue-800/40 bg-blue-950/20 px-1.5 py-0.5 text-[10px] text-accent">{r}</span>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
{isInstalled ? (
|
||||
<span className="shrink-0 text-[10px] text-good">Installed</span>
|
||||
) : (
|
||||
<button
|
||||
onClick={() => handleInstall(p.name)}
|
||||
disabled={installing === p.name}
|
||||
className="shrink-0 rounded-full border border-violet-700/50 bg-violet-950/30 px-2.5 py-0.5 text-[11px] text-violet-300 hover:bg-violet-900/40 disabled:opacity-30"
|
||||
>
|
||||
{installing === p.name ? "Installing..." : "Install"}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Skills section */}
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// Tests (a)–(d) from issue #1372 plan:
|
||||
// (a) List registry — GET /plugins → plugin card shown
|
||||
// (b) List installed — GET /workspaces/{id}/plugins → plugin in installed section
|
||||
// (c) List sources — GET /plugins/sources → scheme chip shown
|
||||
// (d) All three together
|
||||
//
|
||||
// Timing note: jsdom has globally real timers. Fake timers via vi.useFakeTimers
|
||||
// + vi.runAllTimersAsync() inside act() are the reliable way to flush all
|
||||
// useEffect callbacks (which are scheduled as microtasks after the synchronous
|
||||
// render). Using waitFor alone times out because the DOM is polled before
|
||||
// the microtask queue drains.
|
||||
//
|
||||
// Scheme chips render as "{scheme}://" (e.g. "clawhub://://" for "clawhub://"
|
||||
// input) — use regex /scheme:\/\// to match the chip text.
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, cleanup, act } from "@testing-library/react";
|
||||
import React from "react";
|
||||
|
||||
afterEach(() => { cleanup(); vi.useRealTimers(); });
|
||||
|
||||
const apiGet = vi.fn();
|
||||
vi.mock("@/lib/api", () => ({
|
||||
api: {
|
||||
get: (path: string, opts?: unknown) => apiGet(path, opts),
|
||||
post: vi.fn(() => Promise.resolve({})),
|
||||
del: vi.fn(),
|
||||
patch: vi.fn(),
|
||||
put: vi.fn(),
|
||||
},
|
||||
}));
|
||||
vi.mock("@/store/canvas", () => ({
|
||||
useCanvasStore: Object.assign(
|
||||
vi.fn((s: (x: Record<string, unknown>) => unknown) => s({ setPanelTab: vi.fn() })),
|
||||
{ getState: () => ({ setPanelTab: vi.fn() }) },
|
||||
),
|
||||
summarizeWorkspaceCapabilities: vi.fn(() => ({ skills: [], tools: [] })),
|
||||
}));
|
||||
vi.mock("../Toaster", () => ({ showToast: vi.fn() }));
|
||||
|
||||
beforeEach(() => {
|
||||
apiGet.mockReset();
|
||||
Element.prototype.scrollIntoView = vi.fn();
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
import { SkillsTab } from "../SkillsTab";
|
||||
|
||||
const minimalData = {
|
||||
status: "online" as const,
|
||||
runtime: "claude-code" as const,
|
||||
currentTask: "",
|
||||
agentCard: null,
|
||||
} as unknown as Parameters<typeof SkillsTab>[0]["data"];
|
||||
|
||||
// (a) List registry — one plugin appears in the registry browser
|
||||
describe("SkillsTab registry listing (a)", () => {
|
||||
it("shows a plugin card in the registry section", async () => {
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (/\/workspaces\/[\w-]+\/plugins$/.test(path)) return Promise.resolve([]);
|
||||
if (path === "/plugins") {
|
||||
return Promise.resolve([{ name: "claw-tools", version: "1.2.0", description: "DevOps toolkit", author: "claw", tags: ["devops"], skills: ["deploy"] }]);
|
||||
}
|
||||
if (path.endsWith("/plugins/sources")) return Promise.resolve({ schemes: [] });
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
render(<SkillsTab workspaceId="ws-registry-test" data={minimalData} />);
|
||||
await act(async () => { await vi.runAllTimersAsync(); });
|
||||
|
||||
// Version renders as "v{version}" (e.g. "v1.2.0")
|
||||
expect(screen.getByText("claw-tools")).toBeTruthy();
|
||||
expect(screen.getByText("v1.2.0")).toBeTruthy();
|
||||
expect(screen.getByText("DevOps toolkit")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// (b) List installed — installed plugin appears in the installed section
|
||||
describe("SkillsTab installed listing (b)", () => {
|
||||
it("shows a plugin in the installed section", async () => {
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (/\/workspaces\/[\w-]+\/plugins$/.test(path)) {
|
||||
return Promise.resolve([{ name: "memory-postgres", version: "2.0.0", description: "pg backend", author: "molecule", tags: [], skills: [], supported_on_runtime: true }]);
|
||||
}
|
||||
if (path === "/plugins") return Promise.resolve([]);
|
||||
if (path.endsWith("/plugins/sources")) return Promise.resolve({ schemes: [] });
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
render(<SkillsTab workspaceId="ws-installed-test" data={minimalData} />);
|
||||
await act(async () => { await vi.runAllTimersAsync(); });
|
||||
|
||||
expect(screen.getByText(/1 installed/i)).toBeTruthy();
|
||||
expect(screen.getByText("memory-postgres")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// (c) List sources — the ClawHub scheme chip appears
|
||||
describe("SkillsTab source schemes (c)", () => {
|
||||
it("shows the clawhub scheme chip", async () => {
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (/\/workspaces\/[\w-]+\/plugins$/.test(path)) return Promise.resolve([]);
|
||||
if (path === "/plugins") {
|
||||
// Return a plugin so the auto-expand fires
|
||||
return Promise.resolve([{ name: "any-plugin", version: "1.0.0", description: "", author: "x", tags: [], skills: [] }]);
|
||||
}
|
||||
if (path.endsWith("/plugins/sources")) return Promise.resolve({ schemes: ["clawhub://"] });
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
render(<SkillsTab workspaceId="ws-sources-test" data={minimalData} />);
|
||||
await act(async () => { await vi.runAllTimersAsync(); });
|
||||
|
||||
// Chip renders "{scheme}://" so "clawhub://://" for input "clawhub://"
|
||||
expect(screen.getByText(/clawhub:\/\//)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// (d) All three: registry + installed + sources together
|
||||
describe("SkillsTab registry + installed + sources (d)", () => {
|
||||
it("renders registry plugin, installed badge, and source chip simultaneously", async () => {
|
||||
const registryPlugin = { name: "sre-bundle", version: "3.0.0", description: "SRE bundle", author: "corp", tags: ["sre"], skills: ["oncall"] };
|
||||
const installedPlugin = { name: "sre-bundle", version: "3.0.0", description: "SRE bundle", author: "corp", tags: [], skills: [], supported_on_runtime: true };
|
||||
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (/\/workspaces\/[\w-]+\/plugins$/.test(path)) return Promise.resolve([installedPlugin]);
|
||||
if (path === "/plugins") return Promise.resolve([registryPlugin]);
|
||||
if (path.endsWith("/plugins/sources")) return Promise.resolve({ schemes: ["clawhub://", "enterprise://"] });
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
render(<SkillsTab workspaceId="ws-combined-test" data={minimalData} />);
|
||||
await act(async () => { await vi.runAllTimersAsync(); });
|
||||
|
||||
// "sre-bundle" appears twice: once in installed section, once in registry.
|
||||
// getAllByText handles both.
|
||||
expect(screen.getAllByText("sre-bundle")).toHaveLength(2);
|
||||
expect(screen.getByText(/1 installed/i)).toBeTruthy();
|
||||
expect(screen.getByText(/clawhub:\/\//)).toBeTruthy();
|
||||
expect(screen.getByText(/enterprise:\/\//)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -209,12 +209,12 @@ func strDefault(m map[string]interface{}, key, fallback string) string {
|
||||
|
||||
// findRunningContainer returns the live container name for workspaceID, or ""
|
||||
// when the container is genuinely not running OR the daemon errored
|
||||
// transiently. Routed through provisioner.RunningContainerName as the SSOT
|
||||
// transiently. Routed through provisioner.RunningContainerNameFunc as the SSOT
|
||||
// (molecule-core#10) so this handler agrees with healthsweep on the same
|
||||
// inputs. Transient daemon errors are logged distinctly so triage doesn't
|
||||
// confuse a flaky daemon with a stopped container.
|
||||
func (h *PluginsHandler) findRunningContainer(ctx context.Context, workspaceID string) string {
|
||||
name, err := provisioner.RunningContainerName(ctx, h.docker, workspaceID)
|
||||
name, err := provisioner.RunningContainerNameFunc(ctx, h.docker, workspaceID)
|
||||
if err != nil {
|
||||
log.Printf("plugins: docker inspect transient error for %s: %v (treating as not-running for this request)", workspaceID, err)
|
||||
return ""
|
||||
|
||||
@@ -10,20 +10,20 @@ import (
|
||||
|
||||
// TestFindRunningContainer_RoutesThroughProvisionerSSOT is a behavior-based
|
||||
// AST gate: it pins the invariant that PluginsHandler.findRunningContainer
|
||||
// MUST go through provisioner.RunningContainerName for its is-running check,
|
||||
// instead of carrying its own copy of cli.ContainerInspect logic.
|
||||
// MUST go through provisioner.RunningContainerNameFunc for its is-running
|
||||
// check, instead of carrying its own copy of cli.ContainerInspect logic.
|
||||
//
|
||||
// Background — molecule-core#10: a parallel impl of "is the workspace's
|
||||
// container running" used to live in plugins.go. It drifted from the
|
||||
// canonical impl in healthsweep (which goes through Provisioner.IsRunning
|
||||
// → RunningContainerName) on edge cases like "transient daemon error" —
|
||||
// → RunningContainerNameFunc) on edge cases like "transient daemon error" —
|
||||
// the duplicate would 503 with a misleading message while healthsweep
|
||||
// correctly stayed defensive. Consolidating onto RunningContainerName as
|
||||
// correctly stayed defensive. Consolidating onto RunningContainerNameFunc as
|
||||
// the SSOT prevents any future copy from re-introducing that drift.
|
||||
//
|
||||
// Mutation invariant: if a future PR replaces the provisioner call with
|
||||
// `h.docker.ContainerInspect(...)` directly, this test fails. That's the
|
||||
// signal to either (a) extend RunningContainerName's contract OR (b)
|
||||
// signal to either (a) extend RunningContainerNameFunc's contract OR (b)
|
||||
// document why this call site needs to differ. Either way: the drift
|
||||
// gets a reviewer's attention instead of shipping silently.
|
||||
func TestFindRunningContainer_RoutesThroughProvisionerSSOT(t *testing.T) {
|
||||
@@ -66,9 +66,9 @@ func TestFindRunningContainer_RoutesThroughProvisionerSSOT(t *testing.T) {
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
// Pkg.Func form: provisioner.RunningContainerName(...)
|
||||
// Pkg.Func form: provisioner.RunningContainerNameFunc(...)
|
||||
if pkgIdent, ok := sel.X.(*ast.Ident); ok {
|
||||
if pkgIdent.Name == "provisioner" && sel.Sel.Name == "RunningContainerName" {
|
||||
if pkgIdent.Name == "provisioner" && sel.Sel.Name == "RunningContainerNameFunc" {
|
||||
callsRunningContainerName = true
|
||||
}
|
||||
}
|
||||
@@ -83,13 +83,13 @@ func TestFindRunningContainer_RoutesThroughProvisionerSSOT(t *testing.T) {
|
||||
|
||||
if !callsRunningContainerName {
|
||||
t.Errorf(
|
||||
"findRunningContainer must call provisioner.RunningContainerName for the SSOT inspect — see molecule-core#10. Found no such call.",
|
||||
"findRunningContainer must call provisioner.RunningContainerNameFunc for the SSOT inspect — see molecule-core#10. Found no such call.",
|
||||
)
|
||||
}
|
||||
if callsContainerInspectRaw {
|
||||
t.Errorf(
|
||||
"findRunningContainer carries a direct ContainerInspect call. This is the parallel-impl drift molecule-core#10 fixed. " +
|
||||
"Either route through provisioner.RunningContainerName OR — if a new use case truly needs a different inspect — extend RunningContainerName's contract first and update this gate to allow the specific delta.",
|
||||
"Either route through provisioner.RunningContainerNameFunc OR — if a new use case truly needs a different inspect — extend RunningContainerNameFunc's contract first and update this gate to allow the specific delta.",
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -134,19 +134,19 @@ func TestProvisionerIsRunning_RoutesThroughRunningContainerName(t *testing.T) {
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
// Same-package call: bare identifier (e.g. RunningContainerName(...)).
|
||||
if id, ok := call.Fun.(*ast.Ident); ok && id.Name == "RunningContainerName" {
|
||||
// Same-package call: bare identifier (e.g. RunningContainerNameFunc(...)).
|
||||
if id, ok := call.Fun.(*ast.Ident); ok && id.Name == "RunningContainerNameFunc" {
|
||||
callsRunningContainerName = true
|
||||
return true
|
||||
}
|
||||
// Selector call: pkg.Func (e.g. provisioner.RunningContainerName)
|
||||
// Selector call: pkg.Func (e.g. provisioner.RunningContainerNameFunc)
|
||||
// OR recv.Method (e.g. p.cli.ContainerInspect).
|
||||
sel, ok := call.Fun.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
switch sel.Sel.Name {
|
||||
case "RunningContainerName":
|
||||
case "RunningContainerNameFunc":
|
||||
callsRunningContainerName = true
|
||||
case "ContainerInspect":
|
||||
callsContainerInspectRaw = true
|
||||
@@ -155,10 +155,10 @@ func TestProvisionerIsRunning_RoutesThroughRunningContainerName(t *testing.T) {
|
||||
})
|
||||
|
||||
if !callsRunningContainerName {
|
||||
t.Errorf("Provisioner.IsRunning must call RunningContainerName for the SSOT inspect — see molecule-core#10")
|
||||
t.Errorf("Provisioner.IsRunning must call RunningContainerNameFunc for the SSOT inspect — see molecule-core#10")
|
||||
}
|
||||
if callsContainerInspectRaw {
|
||||
t.Errorf("Provisioner.IsRunning carries a direct ContainerInspect call; route through RunningContainerName instead")
|
||||
t.Errorf("Provisioner.IsRunning carries a direct ContainerInspect call; route through RunningContainerNameFunc instead")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/plugins"
|
||||
)
|
||||
|
||||
// stubSources implements pluginSources for testing ListSources.
|
||||
type stubSources struct {
|
||||
schemes []string
|
||||
}
|
||||
|
||||
func (s *stubSources) Register(plugins.SourceResolver) {}
|
||||
func (s *stubSources) Resolve(plugins.Source) (plugins.SourceResolver, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (s *stubSources) Schemes() []string { return s.schemes }
|
||||
|
||||
// TestPluginListSources_StubSources verifies ListSources delegates to the
|
||||
// pluginSources interface without touching the filesystem — the stub proves
|
||||
// the HTTP layer works in isolation from plugins.NewRegistry.
|
||||
func TestPluginListSources_StubSources(t *testing.T) {
|
||||
// Build a handler with a stub that returns exactly the schemes we want.
|
||||
// This is the "isolated unit" form: no temp dir, no registry, no disk.
|
||||
h := NewPluginsHandler(t.TempDir(), nil, nil)
|
||||
h.sources = &stubSources{schemes: []string{"clawhub", "enterprise"}}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/plugins/sources", nil)
|
||||
h.ListSources(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status=%d", w.Code)
|
||||
}
|
||||
var body map[string][]string
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(body["schemes"]) != 2 {
|
||||
t.Errorf("expected 2 schemes, got %v", body["schemes"])
|
||||
}
|
||||
seen := map[string]bool{}
|
||||
for _, s := range body["schemes"] {
|
||||
seen[s] = true
|
||||
}
|
||||
if !seen["clawhub"] || !seen["enterprise"] {
|
||||
t.Errorf("expected clawhub+enterprise, got %v", body["schemes"])
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,266 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"database/sql"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// ---------- broadcastTruncate pure unit tests ----------
|
||||
|
||||
func TestBroadcastTruncate_ShortMessage(t *testing.T) {
|
||||
got := broadcastTruncate("hello", 120)
|
||||
if got != "hello" {
|
||||
t.Errorf("expected 'hello', got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcastTruncate_ExactlyMax(t *testing.T) {
|
||||
got := broadcastTruncate("hello", 5)
|
||||
if got != "hello" {
|
||||
t.Errorf("expected 'hello', got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcastTruncate_LongMessage(t *testing.T) {
|
||||
got := broadcastTruncate("hello world this is a long message that exceeds the limit", 10)
|
||||
if got != "hello worl…" {
|
||||
t.Errorf("expected 'hello worl…', got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcastTruncate_Unicode(t *testing.T) {
|
||||
// "日本語" is 3 runes; truncating to 2 gives "日本…"
|
||||
got := broadcastTruncate("日本語テスト", 2)
|
||||
if got != "日本…" {
|
||||
t.Errorf("expected '日本…', got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Broadcast endpoint ----------
|
||||
|
||||
// All test IDs are valid UUIDs so they pass validateWorkspaceID.
|
||||
const (
|
||||
wsSender = "00000000-0000-0000-0000-000000000001"
|
||||
wsDNE = "00000000-0000-0000-0000-000000000002"
|
||||
wsDisabled = "00000000-0000-0000-0000-000000000003"
|
||||
wsAlone = "00000000-0000-0000-0000-000000000004"
|
||||
wsR1 = "00000000-0000-0000-0000-000000000011"
|
||||
wsR2 = "00000000-0000-0000-0000-000000000012"
|
||||
)
|
||||
|
||||
func makeBroadcastHandler(t *testing.T) (*BroadcastHandler, sqlmock.Sqlmock, func()) {
|
||||
t.Helper()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
cleanup := func() {
|
||||
db.DB = prevDB
|
||||
mockDB.Close()
|
||||
}
|
||||
return NewBroadcastHandler(newTestBroadcaster()), mock, cleanup
|
||||
}
|
||||
|
||||
func postBroadcast(t *testing.T, h *BroadcastHandler, workspaceID string, body string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: workspaceID}}
|
||||
c.Request = httptest.NewRequest("POST", "/workspaces/"+workspaceID+"/broadcast", strings.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
h.Broadcast(c)
|
||||
return w
|
||||
}
|
||||
|
||||
func TestBroadcast_InvalidWorkspaceID(t *testing.T) {
|
||||
h, _, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
// validateWorkspaceID rejects anything that isn't a valid UUID.
|
||||
w := postBroadcast(t, h, "not-a-valid-uuid", `{"message":"hello"}`)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_MissingMessage(t *testing.T) {
|
||||
h, _, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
w := postBroadcast(t, h, wsSender, `{}`)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_EmptyMessage(t *testing.T) {
|
||||
h, _, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
w := postBroadcast(t, h, wsSender, `{"message":""}`)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_WorkspaceNotFound(t *testing.T) {
|
||||
h, mock, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces`).
|
||||
WithArgs(wsDNE).
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
|
||||
w := postBroadcast(t, h, wsDNE, `{"message":"hello"}`)
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_BroadcastDisabled(t *testing.T) {
|
||||
h, mock, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces`).
|
||||
WithArgs(wsDisabled).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).
|
||||
AddRow("test-ws", false))
|
||||
|
||||
w := postBroadcast(t, h, wsDisabled, `{"message":"hello"}`)
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Errorf("expected 403, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_DeliversToTwoRecipients(t *testing.T) {
|
||||
h, mock, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
|
||||
// Sender lookup.
|
||||
mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces`).
|
||||
WithArgs(wsSender).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).
|
||||
AddRow("sender-ws", true))
|
||||
|
||||
// Recipients: two workspaces.
|
||||
// Escape \$1 so sqlmock reads it as a literal (not a regex backreference).
|
||||
mock.ExpectQuery(`SELECT id FROM workspaces WHERE status != 'removed' AND id != \$1`).
|
||||
WithArgs(wsSender).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(wsR1).AddRow(wsR2))
|
||||
|
||||
// Activity log inserts for each recipient (6 args: ws_id, activity_type, method, source_id, summary, status).
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(wsR1, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(wsR2, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// Sender's own activity log (5 args: ws_id, activity_type, method, summary, status).
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(wsSender, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
w := postBroadcast(t, h, wsSender, `{"message":"hello world"}`)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_ZeroRecipients(t *testing.T) {
|
||||
h, mock, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
|
||||
// Sender lookup.
|
||||
mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces`).
|
||||
WithArgs(wsAlone).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).
|
||||
AddRow("alone-ws", true))
|
||||
|
||||
// Recipients: no rows.
|
||||
mock.ExpectQuery(`SELECT id FROM workspaces WHERE status != 'removed' AND id != \$1`).
|
||||
WithArgs(wsAlone).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}))
|
||||
|
||||
// Sender activity log (zero delivered, 5 args).
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(wsAlone, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
w := postBroadcast(t, h, wsAlone, `{"message":"hello"}`)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_RecipientActivityFailureIsNonFatal(t *testing.T) {
|
||||
h, mock, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
|
||||
// Sender lookup.
|
||||
mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces`).
|
||||
WithArgs(wsSender).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).
|
||||
AddRow("sender-ws", true))
|
||||
|
||||
// Recipients: one workspace.
|
||||
mock.ExpectQuery(`SELECT id FROM workspaces WHERE status != 'removed' AND id != \$1`).
|
||||
WithArgs(wsSender).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(wsR1))
|
||||
|
||||
// Recipient activity log fails — error is logged but request succeeds.
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(wsR1, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
// Sender activity log still succeeds.
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WithArgs(wsSender, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
w := postBroadcast(t, h, wsSender, `{"message":"hello"}`)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 despite recipient insert failure, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestBroadcast_RecipientsQueryDBError(t *testing.T) {
|
||||
h, mock, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
|
||||
// Sender lookup.
|
||||
mock.ExpectQuery(`SELECT name, broadcast_enabled FROM workspaces`).
|
||||
WithArgs(wsSender).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name", "broadcast_enabled"}).
|
||||
AddRow("sender-ws", true))
|
||||
|
||||
// Recipients query fails.
|
||||
mock.ExpectQuery(`SELECT id FROM workspaces WHERE status != 'removed' AND id != \$1`).
|
||||
WithArgs(wsSender).
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
w := postBroadcast(t, h, wsSender, `{"message":"hello"}`)
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// Ensure BroadcastHandler doesn't read the request body if validation fails early.
|
||||
func TestBroadcast_MalformedJSON(t *testing.T) {
|
||||
h, _, cleanup := makeBroadcastHandler(t)
|
||||
defer cleanup()
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: wsSender}}
|
||||
c.Request = httptest.NewRequest("POST", "/workspaces/"+wsSender+"/broadcast", bytes.NewReader([]byte(`{not-json`)))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
h.Broadcast(c)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
@@ -1139,7 +1139,7 @@ func (p *Provisioner) IsRunning(ctx context.Context, workspaceID string) (bool,
|
||||
if p == nil || p.cli == nil {
|
||||
return false, ErrNoBackend
|
||||
}
|
||||
name, err := RunningContainerName(ctx, p.cli, workspaceID)
|
||||
name, err := RunningContainerNameFunc(ctx, p.cli, workspaceID)
|
||||
if err != nil {
|
||||
// Transient daemon error: caller treats !running as dead + restarts.
|
||||
// Returning true + the underlying error preserves the error for
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
package provisioner
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/docker/docker/client"
|
||||
)
|
||||
|
||||
// RunningContainerNameFunc is the pluggable entry-point for the RunningContainerName
|
||||
// SSOT. It defaults to the canonical implementation but can be swapped at test
|
||||
// time via StubRunningContainerName. The plug is at the package level — not the
|
||||
// struct level — so callers (Provisioner.IsRunning, PluginsHandler.findRunningContainer,
|
||||
// healthsweep) all hit the same override without each needing its own wiring.
|
||||
//
|
||||
// Isolating "testing" to this single file avoids the CGO / undefined-reference
|
||||
// problem that arises when Docker client types appear in non-test files.
|
||||
var RunningContainerNameFunc = RunningContainerName
|
||||
|
||||
// StubRunningContainerName installs fn as the RunningContainerNameFunc for the
|
||||
// remainder of the test (or until swapped again). Deferred restore is the caller's
|
||||
// responsibility.
|
||||
func StubRunningContainerName(fn func(context.Context, *client.Client, string) (string, error)) func() {
|
||||
orig := RunningContainerNameFunc
|
||||
RunningContainerNameFunc = fn
|
||||
return func() { RunningContainerNameFunc = orig }
|
||||
}
|
||||
Reference in New Issue
Block a user