Compare commits
34 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 85f8d78ac0 | |||
| a7c3f6b7ed | |||
| d0c8dd8be8 | |||
| 1d37c0c44b | |||
| aebaef07dd | |||
| 3d295309b0 | |||
| 3018fdee2f | |||
| a5f8a6a4e0 | |||
| 88acde1197 | |||
| 1a7402a8aa | |||
| 9526ca537e | |||
| 7a7eafa991 | |||
| beb65b6c5c | |||
| dbbfb52cbd | |||
| 7e130daef2 | |||
| 63867d5ea5 | |||
| 212471798c | |||
| 5ca1911906 | |||
| b278623662 | |||
| f3b168b867 | |||
| c3ba26ead2 | |||
| 3a1654818c | |||
| 5d6dccfb18 | |||
| b1fac110f2 | |||
| cd83022365 | |||
| 8ba12898d6 | |||
| 04b4135741 | |||
| d996d7bdce | |||
| bbb7a3f57e | |||
| e1112880fe | |||
| b8583ef019 | |||
| 31fedd50af | |||
| b6eecb58d7 | |||
| 159b3978c1 |
@@ -0,0 +1,55 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for formatAuditRelativeTime exported from AuditTrailPanel.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { formatAuditRelativeTime } from "../AuditTrailPanel";
|
||||
|
||||
describe("formatAuditRelativeTime", () => {
|
||||
const now = new Date("2026-05-18T12:00:00Z").getTime();
|
||||
|
||||
it('returns "just now" for timestamps less than 60s ago', () => {
|
||||
const ts = new Date(now - 30_000).toISOString(); // 30s ago
|
||||
expect(formatAuditRelativeTime(ts, now)).toBe("just now");
|
||||
});
|
||||
|
||||
it("returns minutes for timestamps under 1h", () => {
|
||||
const ts = new Date(now - 5 * 60_000).toISOString(); // 5m ago
|
||||
expect(formatAuditRelativeTime(ts, now)).toBe("5m ago");
|
||||
});
|
||||
|
||||
it("returns hours for timestamps under 24h", () => {
|
||||
const ts = new Date(now - 3 * 3_600_000).toISOString(); // 3h ago
|
||||
expect(formatAuditRelativeTime(ts, now)).toBe("3h ago");
|
||||
});
|
||||
|
||||
it("returns locale date for timestamps older than 24h", () => {
|
||||
const ts = new Date(now - 2 * 86_400_000).toISOString(); // 2d ago
|
||||
const result = formatAuditRelativeTime(ts, now);
|
||||
// Returns a locale date string; just verify it's a non-empty string
|
||||
expect(typeof result).toBe("string");
|
||||
expect(result.length).toBeGreaterThan(0);
|
||||
expect(result).not.toBe("just now");
|
||||
expect(result).not.toMatch(/m ago$/);
|
||||
expect(result).not.toMatch(/h ago$/);
|
||||
});
|
||||
|
||||
it("handles exactly 60s boundary as minutes", () => {
|
||||
const ts = new Date(now - 60_000).toISOString(); // exactly 1m ago
|
||||
expect(formatAuditRelativeTime(ts, now)).toBe("1m ago");
|
||||
});
|
||||
|
||||
it("handles exactly 3600s boundary as hours", () => {
|
||||
const ts = new Date(now - 3_600_000).toISOString(); // exactly 1h ago
|
||||
expect(formatAuditRelativeTime(ts, now)).toBe("1h ago");
|
||||
});
|
||||
|
||||
it("handles exactly 86400s boundary", () => {
|
||||
const ts = new Date(now - 86_400_000).toISOString(); // exactly 24h ago
|
||||
const result = formatAuditRelativeTime(ts, now);
|
||||
// Exactly 24h should fall into the "days" branch
|
||||
expect(typeof result).toBe("string");
|
||||
expect(result).not.toMatch(/m ago$/);
|
||||
expect(result).not.toMatch(/h ago$/);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,82 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for exported helpers from MemoryInspectorPanel:
|
||||
* isPluginUnavailableError, formatTTL.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { isPluginUnavailableError, formatTTL } from "../MemoryInspectorPanel";
|
||||
|
||||
describe("isPluginUnavailableError", () => {
|
||||
it("returns true when error message contains MEMORY_PLUGIN_URL", () => {
|
||||
const err = new Error("MEMORY_PLUGIN_URL is not configured");
|
||||
expect(isPluginUnavailableError(err)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when error message does not contain MEMORY_PLUGIN_URL", () => {
|
||||
const err = new Error("Connection refused");
|
||||
expect(isPluginUnavailableError(err)).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for non-Error values", () => {
|
||||
expect(isPluginUnavailableError("string error")).toBe(false);
|
||||
expect(isPluginUnavailableError(null)).toBe(false);
|
||||
expect(isPluginUnavailableError(undefined)).toBe(false);
|
||||
expect(isPluginUnavailableError({})).toBe(false);
|
||||
});
|
||||
|
||||
it("handles Error with empty message", () => {
|
||||
expect(isPluginUnavailableError(new Error(""))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("formatTTL", () => {
|
||||
// Freeze time at 2026-05-18T12:00:00Z for deterministic tests.
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-05-18T12:00:00Z"));
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("returns empty string for null", () => {
|
||||
expect(formatTTL(null)).toBe("");
|
||||
});
|
||||
|
||||
it("returns empty string for undefined", () => {
|
||||
expect(formatTTL(undefined)).toBe("");
|
||||
});
|
||||
|
||||
it("returns empty string for empty string", () => {
|
||||
expect(formatTTL("")).toBe("");
|
||||
});
|
||||
|
||||
it("returns 'expired' for past timestamps", () => {
|
||||
const past = new Date(Date.now() - 60_000).toISOString();
|
||||
expect(formatTTL(past)).toBe("expired");
|
||||
});
|
||||
|
||||
it("returns seconds for sub-minute future TTLs", () => {
|
||||
const future = new Date(Date.now() + 30_000).toISOString();
|
||||
expect(formatTTL(future)).toBe("30s");
|
||||
});
|
||||
|
||||
it("returns minutes for sub-hour future TTLs", () => {
|
||||
const future = new Date(Date.now() + 5 * 60_000).toISOString();
|
||||
expect(formatTTL(future)).toBe("5m");
|
||||
});
|
||||
|
||||
it("returns hours for sub-day future TTLs", () => {
|
||||
const future = new Date(Date.now() + 3 * 3_600_000).toISOString();
|
||||
expect(formatTTL(future)).toBe("3h");
|
||||
});
|
||||
|
||||
it("returns days for TTLs longer than 24h", () => {
|
||||
const future = new Date(Date.now() + 2 * 86_400_000).toISOString();
|
||||
expect(formatTTL(future)).toBe("2d");
|
||||
});
|
||||
|
||||
it("returns empty string for invalid date string", () => {
|
||||
expect(formatTTL("not-a-date")).toBe("");
|
||||
});
|
||||
});
|
||||
@@ -242,6 +242,8 @@ export function MobileChat({
|
||||
|
||||
useChatSocket(agentId, {
|
||||
onAgentMessage: appendMessageDeduped,
|
||||
// Fan-out user's own outbound message to all sessions (issue #228).
|
||||
onUserMessage: appendMessageDeduped,
|
||||
onSendComplete: releaseSendGuards,
|
||||
});
|
||||
|
||||
|
||||
@@ -143,6 +143,12 @@ function MyChatPanel({ workspaceId, data }: Props) {
|
||||
releaseSendGuards();
|
||||
}
|
||||
},
|
||||
// Fan-out of user's own outbound message to all sessions (issue #228).
|
||||
// Uses appendMessageDeduped so the originating session collapses its
|
||||
// optimistic copy (same role + content within 3-second window).
|
||||
onUserMessage: (msg) => {
|
||||
history.setMessages((prev) => appendMessageDeduped(prev, msg));
|
||||
},
|
||||
onActivityLog: (entry) => {
|
||||
if (!sending) return;
|
||||
setActivityLog((prev) => appendActivityLine(prev, entry));
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for USER_MESSAGE event handling in useChatSocket.
|
||||
*
|
||||
* Covers issue #228: a canvas user's own outbound message was not fanned
|
||||
* out to other sessions — the originating session inserted it optimistically,
|
||||
* but other sessions only saw it after a manual refresh.
|
||||
*
|
||||
* The server now broadcasts USER_MESSAGE on canvas message/send. This test
|
||||
* verifies the canvas side consumes and forwards it to onUserMessage.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import React from "react";
|
||||
import { useChatSocket, type UseChatSocketCallbacks } from "../hooks/useChatSocket";
|
||||
import { emitSocketEvent, _resetSocketEventListenersForTests } from "@/store/socket-events";
|
||||
import type { WSMessage } from "@/store/socket";
|
||||
|
||||
// Silence React StrictMode double-invoke noise — we care about final state.
|
||||
const WARN = console.warn;
|
||||
beforeEach(() => { console.warn = () => {}; });
|
||||
afterEach(() => { console.warn = WARN; });
|
||||
|
||||
beforeEach(() => {
|
||||
_resetSocketEventListenersForTests();
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-05-18T10:00:00Z"));
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
_resetSocketEventListenersForTests();
|
||||
});
|
||||
|
||||
const WORKSPACE_ID = "00000000-0000-0000-0000-000000000001";
|
||||
|
||||
function makeUserMessageEvent(
|
||||
workspaceId: string,
|
||||
overrides: Partial<{
|
||||
message: string;
|
||||
attachments: Array<{ uri: string; name: string; mimeType?: string; size?: number }>;
|
||||
messageId: string;
|
||||
}> = {},
|
||||
): WSMessage {
|
||||
const { message = "Hello, agent!", attachments, messageId } = overrides;
|
||||
const payload: Record<string, unknown> = { message };
|
||||
if (attachments) payload.attachments = attachments;
|
||||
if (messageId) payload.messageId = messageId;
|
||||
return {
|
||||
event: "USER_MESSAGE",
|
||||
workspace_id: workspaceId,
|
||||
timestamp: "2026-05-18T10:00:00Z",
|
||||
payload,
|
||||
};
|
||||
}
|
||||
|
||||
describe("useChatSocket USER_MESSAGE handling", () => {
|
||||
it("calls onUserMessage with a ChatMessage when USER_MESSAGE arrives for matching workspace", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
const { result } = renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(makeUserMessageEvent(WORKSPACE_ID, { message: "Hello!" }));
|
||||
});
|
||||
|
||||
expect(onUserMessage).toHaveBeenCalledTimes(1);
|
||||
const msg = onUserMessage.mock.calls[0][0];
|
||||
expect(msg.role).toBe("user");
|
||||
expect(msg.content).toBe("Hello!");
|
||||
expect(typeof msg.id).toBe("string");
|
||||
expect(msg.timestamp).toBe("2026-05-18T10:00:00.000Z");
|
||||
});
|
||||
|
||||
it("calls onUserMessage with attachments extracted from the payload", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(
|
||||
makeUserMessageEvent(WORKSPACE_ID, {
|
||||
message: "Here is the file",
|
||||
attachments: [
|
||||
{ uri: "workspace:/uploads/report.pdf", name: "report.pdf", mimeType: "application/pdf", size: 4096 },
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
expect(onUserMessage).toHaveBeenCalledTimes(1);
|
||||
const msg = onUserMessage.mock.calls[0][0];
|
||||
expect(msg.role).toBe("user");
|
||||
expect(msg.content).toBe("Here is the file");
|
||||
expect(msg.attachments).toHaveLength(1);
|
||||
expect(msg.attachments![0].uri).toBe("workspace:/uploads/report.pdf");
|
||||
expect(msg.attachments![0].name).toBe("report.pdf");
|
||||
expect(msg.attachments![0].mimeType).toBe("application/pdf");
|
||||
expect(msg.attachments![0].size).toBe(4096);
|
||||
});
|
||||
|
||||
it("does NOT call onUserMessage when workspace_id does not match", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(
|
||||
makeUserMessageEvent("00000000-0000-0000-0000-000000000099", { message: "wrong workspace" }),
|
||||
);
|
||||
});
|
||||
|
||||
expect(onUserMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT call onUserMessage when message is empty and no attachments", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(makeUserMessageEvent(WORKSPACE_ID, { message: "" }));
|
||||
});
|
||||
|
||||
expect(onUserMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores USER_MESSAGE when onUserMessage callback is undefined", () => {
|
||||
const callbacks: UseChatSocketCallbacks = { onAgentMessage: vi.fn() };
|
||||
// Should not throw — undefined callback is guarded
|
||||
expect(() =>
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks)),
|
||||
).not.toThrow();
|
||||
|
||||
const { result } = renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
act(() => {
|
||||
emitSocketEvent(makeUserMessageEvent(WORKSPACE_ID, { message: "Hello" }));
|
||||
});
|
||||
// No error thrown even without onUserMessage
|
||||
});
|
||||
|
||||
it("other event types do NOT trigger onUserMessage", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent({
|
||||
event: "A2A_RESPONSE",
|
||||
workspace_id: WORKSPACE_ID,
|
||||
timestamp: "2026-05-18T10:00:00Z",
|
||||
payload: {},
|
||||
});
|
||||
});
|
||||
|
||||
expect(onUserMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("re-fires onUserMessage for each USER_MESSAGE event received", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(makeUserMessageEvent(WORKSPACE_ID, { message: "First message" }));
|
||||
});
|
||||
act(() => {
|
||||
emitSocketEvent(makeUserMessageEvent(WORKSPACE_ID, { message: "Second message" }));
|
||||
});
|
||||
|
||||
expect(onUserMessage).toHaveBeenCalledTimes(2);
|
||||
expect(onUserMessage.mock.calls[0][0].content).toBe("First message");
|
||||
expect(onUserMessage.mock.calls[1][0].content).toBe("Second message");
|
||||
});
|
||||
|
||||
it("handles USER_MESSAGE with messageId in payload", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(
|
||||
makeUserMessageEvent(WORKSPACE_ID, { message: "With ID", messageId: "msg-id-abc" }),
|
||||
);
|
||||
});
|
||||
|
||||
expect(onUserMessage).toHaveBeenCalledTimes(1);
|
||||
const msg = onUserMessage.mock.calls[0][0];
|
||||
expect(msg.content).toBe("With ID");
|
||||
});
|
||||
|
||||
it("filters out attachments with empty uri or name (defence-in-depth)", () => {
|
||||
const onUserMessage = vi.fn();
|
||||
const callbacks: UseChatSocketCallbacks = { onUserMessage };
|
||||
renderHook(() => useChatSocket(WORKSPACE_ID, callbacks));
|
||||
|
||||
act(() => {
|
||||
emitSocketEvent(
|
||||
makeUserMessageEvent(WORKSPACE_ID, {
|
||||
message: "Mixed attachments",
|
||||
attachments: [
|
||||
{ uri: "workspace:/uploads/good.pdf", name: "good.pdf" },
|
||||
{ uri: "", name: "bad.pdf" }, // empty uri — dropped
|
||||
{ uri: "workspace:/uploads/also-bad", name: "" }, // empty name — dropped
|
||||
{ uri: "workspace:/uploads/also-good.txt", name: "also-good.txt" },
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
expect(onUserMessage).toHaveBeenCalledTimes(1);
|
||||
const msg = onUserMessage.mock.calls[0][0];
|
||||
expect(msg.attachments).toHaveLength(2);
|
||||
expect(msg.attachments![0].name).toBe("good.pdf");
|
||||
expect(msg.attachments![1].name).toBe("also-good.txt");
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { api } from "@/lib/api";
|
||||
import { subscribeSocketResume } from "@/store/socket-events";
|
||||
import { type ChatMessage, appendMessageDeduped as appendMessageDedupedFn } from "../types";
|
||||
|
||||
const INITIAL_HISTORY_LIMIT = 10;
|
||||
@@ -82,6 +83,23 @@ export function useChatHistory(
|
||||
loadInitial();
|
||||
}, [loadInitial]);
|
||||
|
||||
// Back-fill on socket resume. The singleton WS emits this when it
|
||||
// recovers from a down period (ordinary drop, or — the case this
|
||||
// fixes — a mobile-browser background-suspend that silently killed
|
||||
// the socket while the page was frozen). While the socket was dead
|
||||
// every AGENT_MESSAGE / A2A_RESPONSE for this thread was missed, and
|
||||
// the store's rehydrate() only re-pulls /workspaces status, not chat.
|
||||
// Re-running loadInitial() re-fetches the latest persisted history —
|
||||
// exactly what a navigate-away-and-back (remount) does today, but
|
||||
// without the user having to do it. Shared by desktop ChatTab and
|
||||
// MobileChat (both consume this hook), so the realtime path stays
|
||||
// unified across surfaces rather than forked for mobile.
|
||||
useEffect(() => {
|
||||
return subscribeSocketResume(() => {
|
||||
loadInitial();
|
||||
});
|
||||
}, [loadInitial]);
|
||||
|
||||
const loadOlder = useCallback(async () => {
|
||||
if (inflightRef.current || !hasMoreRef.current) return;
|
||||
const oldest = oldestMessageRef.current;
|
||||
|
||||
@@ -7,6 +7,10 @@ import { createMessage, type ChatMessage } from "../types";
|
||||
|
||||
export interface UseChatSocketCallbacks {
|
||||
onAgentMessage?: (msg: ChatMessage) => void;
|
||||
/** Called when another session sent a user message — used to fan out
|
||||
* the user's own outbound text to all sessions so a second device
|
||||
* sees the question live without a manual refresh (issue #228). */
|
||||
onUserMessage?: (msg: ChatMessage) => void;
|
||||
onActivityLog?: (entry: string) => void;
|
||||
onSendComplete?: () => void;
|
||||
onSendError?: (error: string) => void;
|
||||
@@ -43,6 +47,33 @@ export function useChatSocket(
|
||||
|
||||
useSocketEvent((msg) => {
|
||||
try {
|
||||
if (msg.event === "USER_MESSAGE" && msg.workspace_id === workspaceId) {
|
||||
const p = msg.payload || {};
|
||||
const message = typeof p.message === "string" ? p.message : "";
|
||||
const rawAttachments = p.attachments;
|
||||
const attachments =
|
||||
Array.isArray(rawAttachments)
|
||||
? (rawAttachments as Array<{ uri?: unknown; name?: unknown; mimeType?: unknown; size?: unknown }>)
|
||||
.filter(
|
||||
(a) =>
|
||||
typeof a?.uri === "string" && a.uri.length > 0 &&
|
||||
typeof a?.name === "string" && a.name.length > 0,
|
||||
)
|
||||
.map((a) => ({
|
||||
uri: a.uri as string,
|
||||
name: a.name as string,
|
||||
mimeType: typeof a.mimeType === "string" ? a.mimeType : undefined,
|
||||
size: typeof a.size === "number" ? a.size : undefined,
|
||||
}))
|
||||
: undefined;
|
||||
if (message || (attachments && attachments.length > 0)) {
|
||||
callbacksRef.current.onUserMessage?.(
|
||||
createMessage("user", message, attachments),
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (msg.event === "ACTIVITY_LOGGED") {
|
||||
if (msg.workspace_id !== workspaceId) return;
|
||||
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for useKeyboardShortcut.
|
||||
*
|
||||
* Strategy: use renderHook from @testing-library/react so useEffect fires
|
||||
* before dispatch. We spy on window.addEventListener to capture the registered
|
||||
* handler. Events are dispatched by calling the captured handler directly
|
||||
* with a KeyboardEvent that has metaKey/ctrlKey overridden via
|
||||
* Object.defineProperty (jsdom's built-in modifier-key event is unreliable).
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { cleanup, act, renderHook } from "@testing-library/react";
|
||||
import { useState, useCallback } from "react";
|
||||
import { useKeyboardShortcut } from "../use-keyboard-shortcut";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
// Capture the most-recently registered keydown handler so tests can dispatch through it.
|
||||
let registeredHandler: ((e: KeyboardEvent) => void) | null = null;
|
||||
|
||||
const addSpy = vi.spyOn(window, "addEventListener").mockImplementation(
|
||||
(event: string, handler: EventListener) => {
|
||||
if (event === "keydown") {
|
||||
registeredHandler = handler as (e: KeyboardEvent) => void;
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
const removeSpy = vi.spyOn(window, "removeEventListener").mockImplementation(
|
||||
(event: string) => {
|
||||
if (event === "keydown") {
|
||||
registeredHandler = null;
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
registeredHandler = null;
|
||||
addSpy.mockClear();
|
||||
removeSpy.mockClear();
|
||||
});
|
||||
|
||||
/**
|
||||
* Dispatch a keydown event through the captured handler.
|
||||
* Wrapped in act() so React flushes any state updates synchronously.
|
||||
* Bypasses jsdom's internal event routing (which doesn't go through
|
||||
* window.EventTarget.prototype.addEventListener for fireEvent dispatch).
|
||||
*/
|
||||
function dispatchKeydown(
|
||||
key: string,
|
||||
{ meta = false, ctrl = false }: { meta?: boolean; ctrl?: boolean } = {},
|
||||
) {
|
||||
act(() => {
|
||||
const e = new KeyboardEvent("keydown", { key, bubbles: true });
|
||||
Object.defineProperty(e, "metaKey", { value: meta });
|
||||
Object.defineProperty(e, "ctrlKey", { value: ctrl });
|
||||
registeredHandler?.(e);
|
||||
});
|
||||
}
|
||||
|
||||
describe("useKeyboardShortcut", () => {
|
||||
describe("enabled=false", () => {
|
||||
it("does not register a keydown listener", () => {
|
||||
renderHook(() =>
|
||||
useKeyboardShortcut("k", vi.fn(), { enabled: false }),
|
||||
);
|
||||
expect(addSpy).not.toHaveBeenCalledWith("keydown", expect.any(Function));
|
||||
});
|
||||
});
|
||||
|
||||
describe("meta modifier", () => {
|
||||
it("fires callback on Cmd+K", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, { meta: true }));
|
||||
dispatchKeydown("k", { meta: true });
|
||||
expect(cb).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does NOT fire on Ctrl+K when only meta=true", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, { meta: true }));
|
||||
dispatchKeydown("k", { ctrl: true });
|
||||
expect(cb).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT fire on plain K even with meta=true", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, { meta: true }));
|
||||
dispatchKeydown("k", { meta: false, ctrl: false });
|
||||
expect(cb).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("ctrl modifier", () => {
|
||||
it("fires callback on Ctrl+K", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, { ctrl: true }));
|
||||
dispatchKeydown("k", { ctrl: true });
|
||||
expect(cb).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does NOT fire on Cmd+K when only ctrl=true", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, { ctrl: true }));
|
||||
dispatchKeydown("k", { meta: true });
|
||||
expect(cb).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("no-modifier guard", () => {
|
||||
it("does not fire when no modifier is held", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, {}));
|
||||
dispatchKeydown("k", { meta: false, ctrl: false });
|
||||
expect(cb).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("key mismatch", () => {
|
||||
it("does not fire when wrong key is pressed", () => {
|
||||
const cb = vi.fn();
|
||||
renderHook(() => useKeyboardShortcut("k", cb, { meta: true }));
|
||||
dispatchKeydown("j", { meta: true });
|
||||
expect(cb).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("count reflects shortcut fires", () => {
|
||||
it("increments when Cmd+K fires", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const [count, setCount] = useState(0);
|
||||
const cb = useCallback(() => setCount((c) => c + 1), []);
|
||||
useKeyboardShortcut("k", cb, { meta: true });
|
||||
return count;
|
||||
});
|
||||
expect(result.current).toBe(0);
|
||||
dispatchKeydown("k", { meta: true });
|
||||
expect(result.current).toBe(1);
|
||||
dispatchKeydown("k", { meta: true });
|
||||
expect(result.current).toBe(2);
|
||||
});
|
||||
|
||||
it("does not increment on wrong modifier", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const [count, setCount] = useState(0);
|
||||
const cb = useCallback(() => setCount((c) => c + 1), []);
|
||||
useKeyboardShortcut("k", cb, { meta: true });
|
||||
return count;
|
||||
});
|
||||
dispatchKeydown("k", { ctrl: true }); // wrong modifier
|
||||
expect(result.current).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("cleanup on unmount", () => {
|
||||
it("removes the keydown listener on unmount", () => {
|
||||
const cb = vi.fn();
|
||||
const { unmount } = renderHook(() =>
|
||||
useKeyboardShortcut("k", cb, { meta: true }),
|
||||
);
|
||||
expect(removeSpy).not.toHaveBeenCalled();
|
||||
unmount();
|
||||
expect(removeSpy).toHaveBeenCalledWith("keydown", expect.any(Function));
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,84 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for useSocketEvent.
|
||||
*
|
||||
* Covers:
|
||||
* - subscribeSocketEvents is called on mount
|
||||
* - Unsubscribe is called on unmount
|
||||
* - subscribeSocketEvents is called only once (ref-based, not render-based)
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, cleanup } from "@testing-library/react";
|
||||
import React from "react";
|
||||
import { useSocketEvent } from "../useSocketEvent";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
// Mutable ref shared between vi.mock factory and test helpers
|
||||
const state = {
|
||||
handler: null as ((msg: unknown) => void) | null,
|
||||
unsubscribe: null as (() => void) | null,
|
||||
};
|
||||
|
||||
// Module-level mock — factory uses the state object so beforeEach can update it
|
||||
vi.mock("@/store/socket-events", () => ({
|
||||
subscribeSocketEvents: vi.fn().mockImplementation(() => {
|
||||
if (state.unsubscribe) return state.unsubscribe;
|
||||
const fn = vi.fn();
|
||||
state.unsubscribe = fn;
|
||||
return fn;
|
||||
}),
|
||||
}));
|
||||
|
||||
import { subscribeSocketEvents } from "@/store/socket-events";
|
||||
|
||||
beforeEach(() => {
|
||||
state.handler = null;
|
||||
state.unsubscribe = null;
|
||||
vi.mocked(subscribeSocketEvents).mockImplementation(() => {
|
||||
const fn = vi.fn();
|
||||
state.unsubscribe = fn;
|
||||
return fn;
|
||||
});
|
||||
});
|
||||
|
||||
// Dispatch a message through the subscribed handler
|
||||
function dispatchMsg(msg: unknown) {
|
||||
if (state.handler) {
|
||||
state.handler(msg);
|
||||
}
|
||||
}
|
||||
|
||||
// Consumer component that stores the handler ref
|
||||
function SocketConsumer({ cb }: { cb: (msg: unknown) => void }) {
|
||||
useSocketEvent(cb as (msg: unknown) => void);
|
||||
// Store the handler so tests can dispatch through it
|
||||
// We do this by re-mocking to capture the handler
|
||||
return <div data-testid="consumer" />;
|
||||
}
|
||||
|
||||
describe("useSocketEvent", () => {
|
||||
it("calls subscribeSocketEvents on mount", () => {
|
||||
render(<SocketConsumer cb={vi.fn()} />);
|
||||
expect(subscribeSocketEvents).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("calls the unsubscribe function on unmount", () => {
|
||||
const unsubscribe = vi.fn();
|
||||
vi.mocked(subscribeSocketEvents).mockReturnValueOnce(unsubscribe);
|
||||
const { unmount } = render(<SocketConsumer cb={vi.fn()} />);
|
||||
unmount();
|
||||
expect(unsubscribe).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("subscribeSocketEvents is called only once on re-renders", () => {
|
||||
const { rerender } = render(<SocketConsumer cb={vi.fn()} />);
|
||||
const initial = vi.mocked(subscribeSocketEvents).mock.calls.length;
|
||||
|
||||
rerender(<SocketConsumer cb={vi.fn()} />);
|
||||
rerender(<SocketConsumer cb={vi.fn()} />);
|
||||
rerender(<SocketConsumer cb={vi.fn()} />);
|
||||
|
||||
expect(vi.mocked(subscribeSocketEvents).mock.calls.length).toBe(initial);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,98 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for useWorkspaceName.
|
||||
*
|
||||
* Tests that the hook correctly resolves workspace IDs to names
|
||||
* using the canvas store's nodes.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { renderHook, cleanup } from "@testing-library/react";
|
||||
import React from "react";
|
||||
import { useWorkspaceName } from "../useWorkspaceName";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
const mockNodes = [
|
||||
{ id: "ws-1", data: { name: "Alpha Workspace" } },
|
||||
{ id: "ws-2", data: { name: "Beta Workspace" } },
|
||||
{ id: "ws-3", data: {} }, // node without name
|
||||
{ id: "ws-4", data: { name: "" } }, // empty name
|
||||
] as const;
|
||||
|
||||
// Stable reference so useCallback deps are stable across re-renders
|
||||
const stableNodes = [...mockNodes];
|
||||
|
||||
vi.mock("@/store/canvas", () => ({
|
||||
useCanvasStore: Object.assign(
|
||||
vi.fn((selector?: (s: { nodes: typeof stableNodes }) => unknown) => {
|
||||
if (typeof selector === "function") {
|
||||
return selector({ nodes: stableNodes });
|
||||
}
|
||||
return { nodes: stableNodes };
|
||||
}),
|
||||
{ getState: vi.fn(() => ({ nodes: stableNodes })) },
|
||||
),
|
||||
}));
|
||||
|
||||
import { useCanvasStore } from "@/store/canvas";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(useCanvasStore).mockClear();
|
||||
});
|
||||
|
||||
describe("useWorkspaceName", () => {
|
||||
it("returns the workspace name for a known ID", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const resolve = useWorkspaceName();
|
||||
return resolve("ws-1");
|
||||
});
|
||||
expect(result.current).toBe("Alpha Workspace");
|
||||
});
|
||||
|
||||
it("returns the workspace name for another known ID", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const resolve = useWorkspaceName();
|
||||
return resolve("ws-2");
|
||||
});
|
||||
expect(result.current).toBe("Beta Workspace");
|
||||
});
|
||||
|
||||
it("returns empty string for null", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const resolve = useWorkspaceName();
|
||||
return resolve(null);
|
||||
});
|
||||
expect(result.current).toBe("");
|
||||
});
|
||||
|
||||
it("falls back to first 8 chars of ID when node has no name", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const resolve = useWorkspaceName();
|
||||
return resolve("ws-3");
|
||||
});
|
||||
expect(result.current).toBe("ws-3".slice(0, 8));
|
||||
});
|
||||
|
||||
it("falls back to first 8 chars of ID when name is empty string", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const resolve = useWorkspaceName();
|
||||
return resolve("ws-4");
|
||||
});
|
||||
expect(result.current).toBe("ws-4".slice(0, 8));
|
||||
});
|
||||
|
||||
it("falls back to first 8 chars of ID for unknown workspace", () => {
|
||||
const { result } = renderHook(() => {
|
||||
const resolve = useWorkspaceName();
|
||||
return resolve("ws-999");
|
||||
});
|
||||
expect(result.current).toBe("ws-999".slice(0, 8));
|
||||
});
|
||||
|
||||
it("callback is memoized — same reference across renders", () => {
|
||||
const { result, rerender } = renderHook(() => useWorkspaceName());
|
||||
const first = result.current;
|
||||
rerender();
|
||||
expect(result.current).toBe(first);
|
||||
});
|
||||
});
|
||||
@@ -1,67 +1,32 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for cssVar — maps ColorToken to a CSS variable string.
|
||||
*
|
||||
* Exists for the rare case where an inline style="" or SVG fill needs
|
||||
* a token value rather than a Tailwind class. The returned var(--color-foo)
|
||||
* string follows the live theme without re-renders.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { cssVar } from "../theme";
|
||||
import type { ColorToken } from "../theme";
|
||||
import { cssVar, type ColorToken } from "../theme";
|
||||
|
||||
describe("cssVar", () => {
|
||||
it("returns 'var(--color-surface)' for 'surface'", () => {
|
||||
expect(cssVar("surface")).toBe("var(--color-surface)");
|
||||
});
|
||||
const tokens: ColorToken[] = [
|
||||
"surface", "surface-elevated", "surface-sunken", "surface-card",
|
||||
"line", "line-soft", "ink", "ink-mid", "ink-soft",
|
||||
"accent", "accent-strong", "warm", "good", "bad",
|
||||
"bg", "bg-elev", "bg-card", "line-strong",
|
||||
"ink-mute", "ink-dim", "accent-dim", "plasma", "warn",
|
||||
];
|
||||
|
||||
it("returns 'var(--color-ink)' for 'ink'", () => {
|
||||
expect(cssVar("ink")).toBe("var(--color-ink)");
|
||||
});
|
||||
|
||||
it("returns 'var(--color-accent)' for 'accent'", () => {
|
||||
expect(cssVar("accent")).toBe("var(--color-accent)");
|
||||
});
|
||||
|
||||
it("returns 'var(--color-good)' for 'good'", () => {
|
||||
expect(cssVar("good")).toBe("var(--color-good)");
|
||||
});
|
||||
|
||||
it("returns 'var(--color-bad)' for 'bad'", () => {
|
||||
expect(cssVar("bad")).toBe("var(--color-bad)");
|
||||
});
|
||||
|
||||
it("returns 'var(--color-warn)' for 'warn'", () => {
|
||||
expect(cssVar("warn")).toBe("var(--color-warn)");
|
||||
});
|
||||
|
||||
it("handles all surface variants", () => {
|
||||
const surfaces: ColorToken[] = ["surface", "surface-elevated", "surface-sunken", "surface-card"];
|
||||
for (const t of surfaces) {
|
||||
expect(cssVar(t)).toBe(`var(--color-${t})`);
|
||||
it("returns a CSS variable string for every colour token", () => {
|
||||
for (const token of tokens) {
|
||||
expect(cssVar(token)).toBe(`var(--color-${token})`);
|
||||
}
|
||||
});
|
||||
|
||||
it("handles all ink variants", () => {
|
||||
const inks: ColorToken[] = ["ink", "ink-mid", "ink-soft", "ink-mute", "ink-dim"];
|
||||
for (const t of inks) {
|
||||
expect(cssVar(t)).toBe(`var(--color-${t})`);
|
||||
}
|
||||
it("returned string can be used as an inline style value", () => {
|
||||
const el = document.createElement("div");
|
||||
el.style.color = cssVar("ink");
|
||||
el.style.backgroundColor = cssVar("surface");
|
||||
expect(el.style.color).toBe("var(--color-ink)");
|
||||
expect(el.style.backgroundColor).toBe("var(--color-surface)");
|
||||
});
|
||||
|
||||
it("handles always-dark tokens", () => {
|
||||
const dark: ColorToken[] = ["bg", "bg-elev", "bg-card", "line-strong", "accent-dim", "plasma"];
|
||||
for (const t of dark) {
|
||||
expect(cssVar(t)).toBe(`var(--color-${t})`);
|
||||
}
|
||||
});
|
||||
|
||||
it("is a pure function — same input always returns same output", () => {
|
||||
const tokens: ColorToken[] = ["surface", "accent", "good", "bad", "warm"];
|
||||
for (const t of tokens) {
|
||||
for (let i = 0; i < 3; i++) {
|
||||
expect(cssVar(t)).toBe(`var(--color-${t})`);
|
||||
}
|
||||
}
|
||||
it("returned string contains the token name verbatim", () => {
|
||||
expect(cssVar("accent-strong")).toContain("accent-strong");
|
||||
expect(cssVar("ink-dim")).toContain("ink-dim");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* Tests for ThemeProvider and useTheme.
|
||||
*
|
||||
* Uses renderHook so useEffect fires before assertions.
|
||||
* matchMedia is stubbed via Object.defineProperty in beforeEach.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, renderHook, cleanup, act } from "@testing-library/react";
|
||||
import React from "react";
|
||||
import { ThemeProvider, useTheme } from "../theme-provider";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
function makeMatcher(prefersDark: boolean) {
|
||||
return {
|
||||
matches: prefersDark,
|
||||
media: "(prefers-color-scheme: dark)",
|
||||
onchange: null,
|
||||
addListener: vi.fn(),
|
||||
removeListener: vi.fn(),
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn(),
|
||||
dispatchEvent: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(window, "matchMedia", {
|
||||
writable: true,
|
||||
configurable: true,
|
||||
value: vi.fn().mockImplementation(() => makeMatcher(false)),
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("useTheme", () => {
|
||||
it("returns noopTheme when no provider is in the tree", () => {
|
||||
const { result } = renderHook(() => useTheme());
|
||||
expect(result.current).toMatchObject({
|
||||
theme: "system",
|
||||
resolvedTheme: "light",
|
||||
});
|
||||
expect(typeof result.current.setTheme).toBe("function");
|
||||
});
|
||||
});
|
||||
|
||||
describe("ThemeProvider", () => {
|
||||
it("initialises with the initialTheme prop", () => {
|
||||
const { result } = renderHook(() => useTheme(), {
|
||||
wrapper: ({ children }) => (
|
||||
<ThemeProvider initialTheme="dark">{children}</ThemeProvider>
|
||||
),
|
||||
});
|
||||
expect(result.current).toMatchObject({
|
||||
theme: "dark",
|
||||
resolvedTheme: "dark",
|
||||
});
|
||||
expect(document.documentElement.dataset.theme).toBe("dark");
|
||||
});
|
||||
|
||||
it("reflects system preference when theme=system", () => {
|
||||
Object.defineProperty(window, "matchMedia", {
|
||||
writable: true,
|
||||
configurable: true,
|
||||
value: vi.fn().mockImplementation(() => makeMatcher(true)),
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useTheme(), {
|
||||
wrapper: ({ children }) => (
|
||||
<ThemeProvider initialTheme="system">{children}</ThemeProvider>
|
||||
),
|
||||
});
|
||||
expect(result.current).toMatchObject({
|
||||
theme: "system",
|
||||
resolvedTheme: "dark",
|
||||
});
|
||||
expect(document.documentElement.dataset.theme).toBe("dark");
|
||||
});
|
||||
|
||||
it("resolvedTheme follows explicit theme, not system, when theme != system", () => {
|
||||
Object.defineProperty(window, "matchMedia", {
|
||||
writable: true,
|
||||
configurable: true,
|
||||
value: vi.fn().mockImplementation(() => makeMatcher(true)),
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useTheme(), {
|
||||
wrapper: ({ children }) => (
|
||||
<ThemeProvider initialTheme="light">{children}</ThemeProvider>
|
||||
),
|
||||
});
|
||||
expect(result.current).toMatchObject({
|
||||
theme: "light",
|
||||
resolvedTheme: "light",
|
||||
});
|
||||
expect(document.documentElement.dataset.theme).toBe("light");
|
||||
});
|
||||
|
||||
it("setTheme updates theme state", () => {
|
||||
let setThemeRef: ((t: string) => void) | null = null;
|
||||
|
||||
const { result } = renderHook(() => {
|
||||
const ctx = useTheme();
|
||||
// Capture setTheme on first render
|
||||
if (!setThemeRef) setThemeRef = ctx.setTheme;
|
||||
return ctx;
|
||||
}, {
|
||||
wrapper: ({ children }) => (
|
||||
<ThemeProvider initialTheme="light">{children}</ThemeProvider>
|
||||
),
|
||||
});
|
||||
|
||||
expect(result.current.theme).toBe("light");
|
||||
|
||||
act(() => { setThemeRef!("dark"); });
|
||||
expect(result.current.theme).toBe("dark");
|
||||
expect(document.documentElement.dataset.theme).toBe("dark");
|
||||
});
|
||||
|
||||
it("sets document.documentElement.dataset.theme to resolvedTheme on mount", () => {
|
||||
render(
|
||||
<ThemeProvider initialTheme="dark">
|
||||
<div />
|
||||
</ThemeProvider>,
|
||||
);
|
||||
// renderHook already flushed effects; plain render also needs act
|
||||
act(() => {});
|
||||
expect(document.documentElement.dataset.theme).toBe("dark");
|
||||
});
|
||||
});
|
||||
@@ -21,12 +21,22 @@ vi.mock("../canvas", () => ({
|
||||
class MockWebSocket {
|
||||
static instances: MockWebSocket[] = [];
|
||||
|
||||
// Mirror the real WebSocket readyState constants — socket.ts's wake
|
||||
// path reads WebSocket.OPEN / WebSocket.CONNECTING and this.ws.readyState.
|
||||
static readonly CONNECTING = 0;
|
||||
static readonly OPEN = 1;
|
||||
static readonly CLOSING = 2;
|
||||
static readonly CLOSED = 3;
|
||||
|
||||
url: string;
|
||||
onopen: (() => void) | null = null;
|
||||
onmessage: ((event: { data: string }) => void) | null = null;
|
||||
onclose: (() => void) | null = null;
|
||||
onerror: (() => void) | null = null;
|
||||
closeCallCount = 0;
|
||||
// Starts OPEN once triggerOpen runs; tests flip this to simulate a
|
||||
// mobile background-suspend that left a dead/half-open socket.
|
||||
readyState = MockWebSocket.CONNECTING;
|
||||
|
||||
constructor(url: string) {
|
||||
this.url = url;
|
||||
@@ -35,10 +45,12 @@ class MockWebSocket {
|
||||
|
||||
close() {
|
||||
this.closeCallCount++;
|
||||
this.readyState = MockWebSocket.CLOSED;
|
||||
}
|
||||
|
||||
// Helpers to trigger events in tests
|
||||
triggerOpen() {
|
||||
this.readyState = MockWebSocket.OPEN;
|
||||
this.onopen?.();
|
||||
}
|
||||
|
||||
@@ -59,6 +71,46 @@ class MockWebSocket {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Minimal DOM stub (vitest environment is 'node' — no window/document).
|
||||
// socket.ts's wake-recovery attaches visibilitychange/pageshow/online/
|
||||
// focus listeners; under node it self-no-ops via a typeof guard, so to
|
||||
// exercise the path we inject just enough of window/document here, the
|
||||
// same way WebSocket is stubbed above. Kept tiny on purpose — a single
|
||||
// listener registry keyed by event name, plus a settable
|
||||
// visibilityState.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
interface FakeTarget {
|
||||
_l: Record<string, Array<() => void>>;
|
||||
addEventListener: (type: string, fn: () => void) => void;
|
||||
removeEventListener: (type: string, fn: () => void) => void;
|
||||
dispatch: (type: string) => void;
|
||||
}
|
||||
|
||||
function makeFakeTarget(): FakeTarget {
|
||||
const l: Record<string, Array<() => void>> = {};
|
||||
return {
|
||||
_l: l,
|
||||
addEventListener(type, fn) {
|
||||
(l[type] ||= []).push(fn);
|
||||
},
|
||||
removeEventListener(type, fn) {
|
||||
l[type] = (l[type] || []).filter((f) => f !== fn);
|
||||
},
|
||||
dispatch(type) {
|
||||
for (const fn of l[type] || []) fn();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const fakeWindow = makeFakeTarget();
|
||||
const fakeDocument = Object.assign(makeFakeTarget(), {
|
||||
visibilityState: "visible" as string,
|
||||
});
|
||||
(globalThis as unknown as Record<string, unknown>).window = fakeWindow;
|
||||
(globalThis as unknown as Record<string, unknown>).document = fakeDocument;
|
||||
|
||||
// Install mock WebSocket globally before importing socket module
|
||||
(globalThis as unknown as Record<string, unknown>).WebSocket = MockWebSocket;
|
||||
|
||||
@@ -328,6 +380,153 @@ describe("WebSocket onerror", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Wake recovery — mobile background-suspend regression (mobile chat not
|
||||
// updating in real time until refresh). Simulates: connect → open →
|
||||
// the OS freezes the page and silently kills the WS WITHOUT firing
|
||||
// onclose → user returns (visibilitychange / pageshow / online /
|
||||
// focus) → assert the dead socket is replaced AND, on the new socket's
|
||||
// open, the resume signal fires so chat history back-fills the missed
|
||||
// AGENT_MESSAGE / A2A_RESPONSE events.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
import {
|
||||
subscribeSocketResume,
|
||||
_resetSocketResumeListenersForTests,
|
||||
} from "../socket-events";
|
||||
|
||||
describe("wake recovery (mobile background-suspend)", () => {
|
||||
beforeEach(() => {
|
||||
_resetSocketResumeListenersForTests();
|
||||
fakeDocument.visibilityState = "visible";
|
||||
});
|
||||
|
||||
function suspendKill(ws: MockWebSocket) {
|
||||
// Mobile background-suspend: the OS tore the transport down but the
|
||||
// page was frozen so onclose never ran. The socket object survives
|
||||
// with a CLOSED readyState and no reconnect was scheduled.
|
||||
ws.readyState = MockWebSocket.CLOSED;
|
||||
}
|
||||
|
||||
it("reconnects on visibilitychange when the socket was silently killed", () => {
|
||||
connectSocket();
|
||||
const ws = getLastWS();
|
||||
ws.triggerOpen();
|
||||
expect(MockWebSocket.instances).toHaveLength(1);
|
||||
|
||||
suspendKill(ws);
|
||||
fakeDocument.dispatch("visibilitychange");
|
||||
|
||||
// A fresh socket must have been created — the stale one is not
|
||||
// reused.
|
||||
expect(MockWebSocket.instances.length).toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
it("does NOT reconnect on visibilitychange while the socket is still healthy", () => {
|
||||
connectSocket();
|
||||
const ws = getLastWS();
|
||||
ws.triggerOpen();
|
||||
expect(MockWebSocket.instances).toHaveLength(1);
|
||||
|
||||
// Healthy OPEN socket + a spurious visibilitychange (e.g. quick tab
|
||||
// peek that never actually suspended) → no churn.
|
||||
fakeDocument.dispatch("visibilitychange");
|
||||
expect(MockWebSocket.instances).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("ignores visibilitychange when the page is hidden (the hide transition)", () => {
|
||||
connectSocket();
|
||||
const ws = getLastWS();
|
||||
ws.triggerOpen();
|
||||
suspendKill(ws);
|
||||
|
||||
fakeDocument.visibilityState = "hidden";
|
||||
fakeDocument.dispatch("visibilitychange");
|
||||
// Hidden → must not reconnect (would defeat the purpose; we only
|
||||
// re-arm when the user is actually looking at the page again).
|
||||
expect(MockWebSocket.instances).toHaveLength(1);
|
||||
});
|
||||
|
||||
it.each(["pageshow", "online", "focus"])(
|
||||
"reconnects on window '%s' after a silent kill",
|
||||
(evt) => {
|
||||
connectSocket();
|
||||
const ws = getLastWS();
|
||||
ws.triggerOpen();
|
||||
suspendKill(ws);
|
||||
|
||||
fakeWindow.dispatch(evt);
|
||||
expect(MockWebSocket.instances.length).toBeGreaterThan(1);
|
||||
},
|
||||
);
|
||||
|
||||
it("emits the resume signal once the recovered socket re-opens (so chat back-fills missed messages)", () => {
|
||||
const onResume = vi.fn();
|
||||
const unsub = subscribeSocketResume(onResume);
|
||||
|
||||
connectSocket();
|
||||
const ws1 = getLastWS();
|
||||
ws1.triggerOpen();
|
||||
// First open must NOT fire resume — the mount-time chat-history
|
||||
// fetch already covers the initial load.
|
||||
expect(onResume).not.toHaveBeenCalled();
|
||||
|
||||
// Background-suspend silently kills the socket, then the user
|
||||
// returns.
|
||||
suspendKill(ws1);
|
||||
fakeDocument.dispatch("visibilitychange");
|
||||
|
||||
// The wake handler force-reconnected; the new socket completing its
|
||||
// handshake is what signals "we recovered from a gap — re-fetch".
|
||||
const ws2 = getLastWS();
|
||||
expect(ws2).not.toBe(ws1);
|
||||
ws2.triggerOpen();
|
||||
|
||||
expect(onResume).toHaveBeenCalledTimes(1);
|
||||
unsub();
|
||||
});
|
||||
|
||||
it("does not emit resume on the very first connect", () => {
|
||||
const onResume = vi.fn();
|
||||
const unsub = subscribeSocketResume(onResume);
|
||||
connectSocket();
|
||||
getLastWS().triggerOpen();
|
||||
expect(onResume).not.toHaveBeenCalled();
|
||||
unsub();
|
||||
});
|
||||
|
||||
it("emits resume after an ordinary onclose-driven reconnect too (desktop path unchanged)", () => {
|
||||
const onResume = vi.fn();
|
||||
const unsub = subscribeSocketResume(onResume);
|
||||
|
||||
connectSocket();
|
||||
const ws1 = getLastWS();
|
||||
ws1.triggerOpen();
|
||||
// Ordinary network drop — onclose fires normally.
|
||||
ws1.triggerClose();
|
||||
vi.advanceTimersByTime(1100); // past the 1s backoff
|
||||
const ws2 = getLastWS();
|
||||
expect(ws2).not.toBe(ws1);
|
||||
ws2.triggerOpen();
|
||||
|
||||
expect(onResume).toHaveBeenCalledTimes(1);
|
||||
unsub();
|
||||
});
|
||||
|
||||
it("detaches wake listeners on disconnect (no reconnect after teardown)", () => {
|
||||
connectSocket();
|
||||
const ws = getLastWS();
|
||||
ws.triggerOpen();
|
||||
disconnectSocket();
|
||||
|
||||
const countAfterDisconnect = MockWebSocket.instances.length;
|
||||
// A wake event after teardown must be inert.
|
||||
fakeDocument.dispatch("visibilitychange");
|
||||
fakeWindow.dispatch("focus");
|
||||
expect(MockWebSocket.instances.length).toBe(countAfterDisconnect);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Health check (startHealthCheck / stopHealthCheck via onopen / disconnect)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -61,3 +61,53 @@ export function subscribeSocketEvents(listener: Listener): () => void {
|
||||
export function _resetSocketEventListenersForTests(): void {
|
||||
listeners.clear();
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Socket-resume signal
|
||||
// ---------------------------------------------------------------------------
|
||||
//
|
||||
// Fired by the ReconnectingSocket when the WS comes back up AFTER having
|
||||
// been down (drop, or a mobile-browser background-suspend that silently
|
||||
// killed the socket while the page was frozen). Distinct from the raw
|
||||
// event bus above: while the socket was dead the page missed every
|
||||
// AGENT_MESSAGE / A2A_RESPONSE, and the store's rehydrate() only re-pulls
|
||||
// /workspaces status — it does NOT back-fill chat messages. Components
|
||||
// that render a live message thread (desktop ChatTab + MobileChat, both
|
||||
// via useChatHistory) subscribe here to re-fetch their history on resume
|
||||
// so missed agent replies appear without the user having to navigate
|
||||
// away+back or hard-refresh. Shared by desktop and mobile — the recovery
|
||||
// is in the singleton socket, not forked per-surface.
|
||||
|
||||
type ResumeListener = () => void;
|
||||
|
||||
const resumeListeners = new Set<ResumeListener>();
|
||||
|
||||
/** Notify every resume subscriber that the socket just recovered from a
|
||||
* down period. Called by ReconnectingSocket.onopen, but only when the
|
||||
* open follows a prior loss (not the very first connect — the initial
|
||||
* mount-time history fetch already covers that). */
|
||||
export function emitSocketResume(): void {
|
||||
for (const listener of resumeListeners) {
|
||||
try {
|
||||
listener();
|
||||
} catch (err) {
|
||||
if (typeof console !== "undefined") {
|
||||
console.error("socket-resume listener threw:", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Register a resume subscriber. Returns an unsubscribe function the
|
||||
* caller must invoke from its effect cleanup. */
|
||||
export function subscribeSocketResume(listener: ResumeListener): () => void {
|
||||
resumeListeners.add(listener);
|
||||
return () => {
|
||||
resumeListeners.delete(listener);
|
||||
};
|
||||
}
|
||||
|
||||
/** Test-only: drop all resume subscribers. */
|
||||
export function _resetSocketResumeListenersForTests(): void {
|
||||
resumeListeners.clear();
|
||||
}
|
||||
|
||||
+117
-1
@@ -1,6 +1,6 @@
|
||||
import { useCanvasStore } from "./canvas";
|
||||
import { deriveWsBaseUrl } from "@/lib/ws-url";
|
||||
import { emitSocketEvent } from "./socket-events";
|
||||
import { emitSocketEvent, emitSocketResume } from "./socket-events";
|
||||
|
||||
// If explicit WS_URL is set, use it as-is (may include custom path).
|
||||
// Otherwise derive base + append /ws.
|
||||
@@ -98,9 +98,107 @@ class ReconnectingSocket {
|
||||
// caller can fire-and-forget without coordinating.
|
||||
private rehydrateInFlight: Promise<void> | null = null;
|
||||
private rehydrateDedup = new RehydrateDedup(REHYDRATE_DEDUP_WINDOW_MS);
|
||||
// True once any onopen has fired. Gates the resume signal so the very
|
||||
// first connect doesn't fire it (the mount-time chat-history fetch
|
||||
// already covers the initial load — a resume here would be a wasted
|
||||
// duplicate). Set on the first successful open and stays true.
|
||||
private everConnected = false;
|
||||
// True between a loss (onclose / wake-detected stale socket) and the
|
||||
// next successful onopen. Only when this is set does onopen emit the
|
||||
// resume signal — i.e. we recovered from a real gap during which
|
||||
// AGENT_MESSAGE / A2A_RESPONSE events may have been missed.
|
||||
private wasDown = false;
|
||||
// Bound wake handler. iOS Safari / Chrome-mobile freeze the page and
|
||||
// its timers when the tab is backgrounded or the device locks, and
|
||||
// tear the WS down WITHOUT reliably firing onclose before the freeze.
|
||||
// On thaw nothing re-arms: onclose never ran so no reconnect was
|
||||
// scheduled, and the health-check / fallback-poll intervals were
|
||||
// suspended. The socket is silently dead until a manual refresh. This
|
||||
// handler force-reconnects on any wake signal when the socket isn't
|
||||
// healthy. Stored so disconnect() can detach the listeners.
|
||||
private onWake: (() => void) | null = null;
|
||||
|
||||
constructor(url: string) {
|
||||
this.url = url;
|
||||
this.installWakeListeners();
|
||||
}
|
||||
|
||||
/** Attach page-lifecycle listeners that force a reconnect when the
|
||||
* page returns to the foreground / regains connectivity and the
|
||||
* socket is not OPEN. Shared by desktop and mobile — desktop rarely
|
||||
* hits the stale-socket path (its onclose fires promptly) so this is
|
||||
* effectively a no-op there, while mobile depends on it because the
|
||||
* background-suspend kills the socket without an onclose. */
|
||||
private installWakeListeners() {
|
||||
if (typeof window === "undefined" || typeof document === "undefined") {
|
||||
return;
|
||||
}
|
||||
const wake = () => {
|
||||
if (this.disposed) return;
|
||||
// Only act on a visible page — visibilitychange also fires on the
|
||||
// hide transition, which we must ignore (closing here would defeat
|
||||
// the point).
|
||||
if (
|
||||
typeof document.visibilityState === "string" &&
|
||||
document.visibilityState !== "visible"
|
||||
) {
|
||||
return;
|
||||
}
|
||||
// Healthy socket → nothing to do. A stale/half-open socket on
|
||||
// mobile reports CLOSED or CLOSING (the OS tore the transport
|
||||
// down); CONNECTING is also unhealthy from the user's POV but a
|
||||
// reconnect attempt is already in flight, so leave it.
|
||||
const live =
|
||||
this.ws !== null &&
|
||||
(this.ws.readyState === WebSocket.OPEN ||
|
||||
this.ws.readyState === WebSocket.CONNECTING);
|
||||
if (live) return;
|
||||
// Tear down any zombie and reconnect immediately. Mark wasDown so
|
||||
// the subsequent onopen emits the resume signal and chat threads
|
||||
// back-fill the messages missed while frozen.
|
||||
this.wasDown = true;
|
||||
this.forceReconnect();
|
||||
};
|
||||
this.onWake = wake;
|
||||
document.addEventListener("visibilitychange", wake);
|
||||
window.addEventListener("pageshow", wake);
|
||||
window.addEventListener("online", wake);
|
||||
window.addEventListener("focus", wake);
|
||||
}
|
||||
|
||||
private removeWakeListeners() {
|
||||
if (!this.onWake) return;
|
||||
if (typeof window !== "undefined" && typeof document !== "undefined") {
|
||||
document.removeEventListener("visibilitychange", this.onWake);
|
||||
window.removeEventListener("pageshow", this.onWake);
|
||||
window.removeEventListener("online", this.onWake);
|
||||
window.removeEventListener("focus", this.onWake);
|
||||
}
|
||||
this.onWake = null;
|
||||
}
|
||||
|
||||
/** Detach the current (presumed dead/stale) socket without routing
|
||||
* through its onclose, cancel any pending backoff timer, and
|
||||
* reconnect now. Used by the wake path: the browser already killed
|
||||
* the transport, so the exponential backoff that onclose would have
|
||||
* scheduled is both absent and undesirable — the user is looking at
|
||||
* the page and wants it live immediately. */
|
||||
private forceReconnect() {
|
||||
if (this.disposed) return;
|
||||
if (this.reconnectTimer) {
|
||||
clearTimeout(this.reconnectTimer);
|
||||
this.reconnectTimer = null;
|
||||
}
|
||||
if (this.ws) {
|
||||
this.ws.onopen = null;
|
||||
this.ws.onmessage = null;
|
||||
this.ws.onclose = null;
|
||||
this.ws.onerror = null;
|
||||
try { this.ws.close(); } catch { /* noop */ }
|
||||
this.ws = null;
|
||||
}
|
||||
this.attempt = 0;
|
||||
this.connect();
|
||||
}
|
||||
|
||||
connect() {
|
||||
@@ -132,6 +230,18 @@ class ReconnectingSocket {
|
||||
this.stopFallbackPoll();
|
||||
this.rehydrate();
|
||||
this.startHealthCheck();
|
||||
// If this open follows a real loss (drop, or a mobile background-
|
||||
// suspend that the wake handler recovered from), signal resume so
|
||||
// live message threads re-fetch the AGENT_MESSAGE / A2A_RESPONSE
|
||||
// history they missed while the socket was dead — rehydrate()
|
||||
// above only refreshes /workspaces status, not chat. Gate on
|
||||
// everConnected so the very first open (covered by the mount-time
|
||||
// history fetch) doesn't fire a redundant resume.
|
||||
if (this.everConnected && this.wasDown) {
|
||||
emitSocketResume();
|
||||
}
|
||||
this.everConnected = true;
|
||||
this.wasDown = false;
|
||||
};
|
||||
|
||||
ws.onmessage = (event) => {
|
||||
@@ -157,6 +267,11 @@ class ReconnectingSocket {
|
||||
// corresponds to the WS we just tore down (prevents a stale
|
||||
// onclose from a zombie socket from re-arming the loop).
|
||||
if (this.disposed || this.ws !== ws) return;
|
||||
// We had a live socket and lost it — mark down so the next onopen
|
||||
// emits the resume signal and chat threads back-fill missed
|
||||
// messages. (The wake path also sets this; setting it here covers
|
||||
// the ordinary network-drop case.)
|
||||
this.wasDown = true;
|
||||
this.stopHealthCheck();
|
||||
useCanvasStore.getState().setWsStatus("connecting");
|
||||
this.startFallbackPoll();
|
||||
@@ -247,6 +362,7 @@ class ReconnectingSocket {
|
||||
|
||||
disconnect() {
|
||||
this.disposed = true;
|
||||
this.removeWakeListeners();
|
||||
this.stopHealthCheck();
|
||||
this.stopFallbackPoll();
|
||||
if (this.reconnectTimer) {
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
# 5-Axis Review: PR #3029 (fix #2989) + PR #3033 (docs refresh)
|
||||
|
||||
**Reviewer:** Kimi / Engineer-A
|
||||
**Date:** 2026-05-31
|
||||
**Scope:** Local review (CR2 auth-down, filling review gap per PM dispatch)
|
||||
|
||||
---
|
||||
|
||||
## PR #3029 — CP orphan sweeper + registry prefix abstraction
|
||||
|
||||
### Correctness ✅ (with 1 semantic conflict to resolve)
|
||||
|
||||
**cp_orphan_sweeper.go** — The deprovision split-write race fix is sound:
|
||||
- SELECT `status='removed' AND instance_id IS NOT NULL AND instance_id != ''` correctly targets leaked EC2s.
|
||||
- Stop → clear instance_id is idempotent; on Stop failure the row stays targeted for retry.
|
||||
- `ORDER BY updated_at DESC` + `LIMIT $1` + `UPDATE updated_at = now()` creates fair round-robin drain across cycles.
|
||||
- `supervised.RunWithRecover` wiring in `cmd/server/main.go` mirrors the Docker sweeper pattern.
|
||||
|
||||
**provisioner/registry.go** — Clean env-driven prefix abstraction:
|
||||
- `RegistryPrefix()` respects `MOLECULE_IMAGE_REGISTRY` override; falls back to GHCR OSS default.
|
||||
- `RuntimeImage()` returns `""` for unknown runtimes, forcing explicit fallback at call sites.
|
||||
- `computeRuntimeImages()` runs at init; captures prefix active at boot.
|
||||
|
||||
** provisioner.go migration** — Hardcoded map → `computeRuntimeImages()` is a safe refactor; no behavioral change for OSS default.
|
||||
|
||||
**admin_workspace_images.go** — `TemplateImageRef()` now uses `provisioner.RegistryPrefix()`; keeps admin ops and provisioner pulls consistent.
|
||||
|
||||
### Security ✅
|
||||
|
||||
- Sweeper SQL has no user-input surface; parameters are internal LIMIT constant and DB-generated IDs.
|
||||
- `RegistryPrefix()` reads env only; comment correctly notes it is deploy-time trusted (operator-set, not user-supplied).
|
||||
- No new secrets, auth tokens, or credential exposure.
|
||||
|
||||
### Performance ✅
|
||||
|
||||
- 60s tick / 30s deadline / LIMIT 100 is conservative and safe.
|
||||
- Sequential Stop calls share the 30s parent context; with typical CP DELETE latency (<1s), 100 orphans finish well within budget.
|
||||
- If CP is degraded, deadline expires, UPDATEs don't fire, and next cycle retries — no stampede.
|
||||
|
||||
### Style / Readability ✅
|
||||
|
||||
- Excellent docstrings; the `#2989` race narrative is clearly documented for future maintainers.
|
||||
- `CPOrphanReaper` interface is minimal and testable.
|
||||
- Nil-reaper and nil-DB guards follow existing patterns.
|
||||
- One minor nit: `cpSweepOnce` could return `[]string` of processed IDs to make post-hoc assertions easier, but the fake-reaper test pattern works fine as-is.
|
||||
|
||||
### Tests ✅ (excellent coverage)
|
||||
|
||||
| Scenario | Covered |
|
||||
|---|---|
|
||||
| Happy path: Stop succeeds, instance_id cleared | ✅ |
|
||||
| Stop fails, instance_id retained for retry | ✅ |
|
||||
| Empty result set (steady state) | ✅ |
|
||||
| Multiple orphans, partial failure, others proceed | ✅ |
|
||||
| DB query error (transient) | ✅ |
|
||||
| UPDATE error after Stop success (logs, continues) | ✅ |
|
||||
| Nil db.DB (defensive boot safety) | ✅ |
|
||||
| Nil reaper (disabled, no goroutine leak) | ✅ |
|
||||
| Boot sweep + tick cadence + ctx cancel | ✅ |
|
||||
| Registry prefix default / env override / empty env | ✅ |
|
||||
| Runtime image format for all known runtimes | ✅ |
|
||||
| Unknown runtime returns `""` | ✅ |
|
||||
| Registry override applies to ALL runtimes | ✅ |
|
||||
| Alphabetical order pin | ✅ |
|
||||
|
||||
**All tests pass:**
|
||||
```
|
||||
ok github.com/.../internal/registry 0.107s (9/9 CP sweeper tests)
|
||||
ok github.com/.../internal/provisioner 0.009s (7/7 registry tests)
|
||||
```
|
||||
|
||||
### ⚠️ BLOCKER: Semantic conflict with PR #3033
|
||||
|
||||
`registry.go` adds `"codex"` to `knownRuntimes`, making **9** production runtimes:
|
||||
```go
|
||||
knownRuntimes = []string{
|
||||
"autogen", "claude-code", "codex", "crewai", "deepagents",
|
||||
"gemini-cli", "hermes", "langgraph", "openclaw",
|
||||
}
|
||||
```
|
||||
|
||||
PR #3033 updates the README to claim **eight** production runtimes and explicitly lists:
|
||||
> Claude Code, Hermes, Gemini CLI, LangGraph, DeepAgents, CrewAI, AutoGen, OpenClaw
|
||||
|
||||
`codex` is absent from the README compatibility table, the "What Ships In main" section, and the architecture diagram list. After both PRs merge, the code will support 9 runtimes but the docs will claim 8 — a public-facing drift.
|
||||
|
||||
**Fix path:** Add `codex` to the README runtime list in PR #3033 (or a fast-follow) so the count and table stay accurate. `codex` already exists in `manifest.json` and has a template repo, so it is legitimate to list as "shipping on main."
|
||||
|
||||
---
|
||||
|
||||
## PR #3033 — Docs refresh (README + branding assets)
|
||||
|
||||
### Correctness ✅ (with 1 semantic drift pending)
|
||||
|
||||
- Terminology standardization ("adapters" → "runtimes") is correct and consistent with platform usage.
|
||||
- Deploy buttons updated from `molecule-monorepo` → `molecule-core`.
|
||||
- Canvas v4, Memory v2, SaaS surface, RFC #2967 mentions are all factually accurate.
|
||||
- **Missing:** `codex` runtime (see blocker above).
|
||||
|
||||
### Security ✅
|
||||
|
||||
- SVG assets are static branding; no scripts, no external references beyond the existing `<style>` media query.
|
||||
- No auth or credential surface touched.
|
||||
|
||||
### Performance N/A
|
||||
|
||||
- Docs-only; no runtime impact.
|
||||
|
||||
### Style / Readability ✅
|
||||
|
||||
- warm-paper theme description is concise and helpful.
|
||||
- Architecture diagram update (Docker → EC2 + SSM, KMS, SaaS CP) is accurate.
|
||||
- Quick Start clone URL fixed.
|
||||
|
||||
### Tests N/A
|
||||
|
||||
- No code changes; no test delta.
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| PR | Verdict | Action needed |
|
||||
|---|---|---|
|
||||
| #3029 | **Approve with nit** | Merge-ready after confirming #3033 (or follow-up) adds `codex` to README runtime list. |
|
||||
| #3033 | **Approve with blocker** | Add `codex` to the 8-runtimes list (making 9) and to the compatibility table before merge. |
|
||||
|
||||
**Risk if both merge as-is:** Public docs understate runtime count by 1; operators reading README may think `codex` is not supported when the provisioner already knows about it.
|
||||
|
||||
**Recommended merge order:** #3029 first (adds runtime support), then #3033 with `codex` line added (docs catch up).
|
||||
@@ -60,7 +60,8 @@ func refreshEnvFromCP() error {
|
||||
req.Header.Set("Authorization", "Bearer "+adminToken)
|
||||
req.Header.Set("X-Molecule-Org-Id", orgID)
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
client := &http.Client{Timeout: 10 * time.Second}
|
||||
resp, err := client.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("do request: %w", err)
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package bundle
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log"
|
||||
"strings"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
@@ -86,13 +87,20 @@ func Import(
|
||||
// PluginsPath set by caller if available
|
||||
}
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("bundle/importer: PANIC during provision start for %s: %v", wsID, r)
|
||||
}
|
||||
}()
|
||||
provCtx, cancel := context.WithTimeout(context.Background(), provisioner.ProvisionTimeout)
|
||||
defer cancel()
|
||||
url, err := prov.Start(provCtx, cfg)
|
||||
if err != nil {
|
||||
markFailed(provCtx, wsID, broadcaster, err)
|
||||
} else if url != "" {
|
||||
db.DB.ExecContext(provCtx, `UPDATE workspaces SET url = $1 WHERE id = $2`, url, wsID)
|
||||
if _, dbErr := db.DB.ExecContext(provCtx, `UPDATE workspaces SET url = $1 WHERE id = $2`, url, wsID); dbErr != nil {
|
||||
log.Printf("bundle import: failed to update workspace URL for %s: %v", wsID, dbErr)
|
||||
}
|
||||
}
|
||||
}()
|
||||
}
|
||||
@@ -139,12 +147,16 @@ func markFailed(ctx context.Context, wsID string, broadcaster *events.Broadcaste
|
||||
// markProvisionFailed in workspace-server/internal/handlers/
|
||||
// workspace_provision_shared.go.
|
||||
msg := err.Error()
|
||||
db.DB.ExecContext(ctx,
|
||||
if _, dbErr := db.DB.ExecContext(ctx,
|
||||
`UPDATE workspaces SET status = $1, last_sample_error = $2, updated_at = now() WHERE id = $3`,
|
||||
models.StatusFailed, msg, wsID)
|
||||
broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceProvisionFailed), wsID, map[string]interface{}{
|
||||
models.StatusFailed, msg, wsID); dbErr != nil {
|
||||
log.Printf("bundle import: failed to mark workspace %s failed: %v", wsID, dbErr)
|
||||
}
|
||||
if bcErr := broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceProvisionFailed), wsID, map[string]interface{}{
|
||||
"error": msg,
|
||||
})
|
||||
}); bcErr != nil {
|
||||
log.Printf("bundle import: failed to broadcast provision failed for %s: %v", wsID, bcErr)
|
||||
}
|
||||
}
|
||||
|
||||
func nilIfEmpty(s string) interface{} {
|
||||
|
||||
@@ -375,21 +375,25 @@ func (m *Manager) HandleInbound(ctx context.Context, ch ChannelRow, msg *Inbound
|
||||
|
||||
// Update stats in DB
|
||||
if db.DB != nil {
|
||||
db.DB.ExecContext(ctx, `
|
||||
if _, err := db.DB.ExecContext(ctx, `
|
||||
UPDATE workspace_channels
|
||||
SET last_message_at = now(), message_count = message_count + 1, updated_at = now()
|
||||
WHERE id = $1
|
||||
`, ch.ID)
|
||||
`, ch.ID); err != nil {
|
||||
log.Printf("Channels: failed to update inbound stats for channel %s: %v", ch.ID, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Broadcast event
|
||||
if m.broadcaster != nil {
|
||||
m.broadcaster.RecordAndBroadcast(ctx, string(events.EventChannelMessage), ch.WorkspaceID, map[string]interface{}{
|
||||
if err := m.broadcaster.RecordAndBroadcast(ctx, string(events.EventChannelMessage), ch.WorkspaceID, map[string]interface{}{
|
||||
"channel_id": ch.ID,
|
||||
"channel_type": ch.ChannelType,
|
||||
"username": msg.Username,
|
||||
"direction": "inbound",
|
||||
})
|
||||
}); err != nil {
|
||||
log.Printf("Channels: failed to broadcast inbound event: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -420,19 +424,23 @@ func (m *Manager) SendOutbound(ctx context.Context, channelID string, text strin
|
||||
}
|
||||
|
||||
if db.DB != nil {
|
||||
db.DB.ExecContext(ctx, `
|
||||
if _, err := db.DB.ExecContext(ctx, `
|
||||
UPDATE workspace_channels
|
||||
SET last_message_at = now(), message_count = message_count + 1, updated_at = now()
|
||||
WHERE id = $1
|
||||
`, channelID)
|
||||
`, channelID); err != nil {
|
||||
log.Printf("Channels: failed to update outbound stats for channel %s: %v", channelID, err)
|
||||
}
|
||||
}
|
||||
|
||||
if m.broadcaster != nil {
|
||||
m.broadcaster.RecordAndBroadcast(ctx, string(events.EventChannelMessage), ch.WorkspaceID, map[string]interface{}{
|
||||
if err := m.broadcaster.RecordAndBroadcast(ctx, string(events.EventChannelMessage), ch.WorkspaceID, map[string]interface{}{
|
||||
"channel_id": ch.ID,
|
||||
"channel_type": ch.ChannelType,
|
||||
"direction": "outbound",
|
||||
})
|
||||
}); err != nil {
|
||||
log.Printf("Channels: failed to broadcast outbound event: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -498,7 +506,10 @@ func (m *Manager) FetchWorkspaceChannelContext(ctx context.Context, workspaceID
|
||||
return ""
|
||||
}
|
||||
var config map[string]interface{}
|
||||
json.Unmarshal(configJSON, &config)
|
||||
if err := json.Unmarshal(configJSON, &config); err != nil {
|
||||
log.Printf("Channels: failed to unmarshal channel config: %v", err)
|
||||
return ""
|
||||
}
|
||||
if err := DecryptSensitiveFields(config); err != nil {
|
||||
return ""
|
||||
}
|
||||
@@ -555,8 +566,12 @@ func (m *Manager) loadChannel(ctx context.Context, channelID string) (ChannelRow
|
||||
if err != nil {
|
||||
return ch, fmt.Errorf("channel %s not found: %w", channelID, err)
|
||||
}
|
||||
json.Unmarshal(configJSON, &ch.Config)
|
||||
json.Unmarshal(allowedJSON, &ch.AllowedUsers)
|
||||
if err := json.Unmarshal(configJSON, &ch.Config); err != nil {
|
||||
return ch, fmt.Errorf("unmarshal channel %s config: %w", channelID, err)
|
||||
}
|
||||
if err := json.Unmarshal(allowedJSON, &ch.AllowedUsers); err != nil {
|
||||
return ch, fmt.Errorf("unmarshal channel %s allowed_users: %w", channelID, err)
|
||||
}
|
||||
// #319: decrypt bot_token / webhook_secret — SendOutbound and adapter
|
||||
// methods downstream read them as plaintext strings.
|
||||
if err := DecryptSensitiveFields(ch.Config); err != nil {
|
||||
|
||||
@@ -482,10 +482,12 @@ func (t *TelegramAdapter) StartPolling(ctx context.Context, config map[string]in
|
||||
if apiErr.Code == 429 {
|
||||
retryAfter := time.Duration(apiErr.RetryAfter) * time.Second
|
||||
log.Printf("Channels: Telegram poll rate-limited, sleeping %s", retryAfter)
|
||||
timer := time.NewTimer(retryAfter)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
timer.Stop()
|
||||
return nil
|
||||
case <-time.After(retryAfter):
|
||||
case <-timer.C:
|
||||
continue
|
||||
}
|
||||
}
|
||||
@@ -495,10 +497,12 @@ func (t *TelegramAdapter) StartPolling(ctx context.Context, config map[string]in
|
||||
}
|
||||
}
|
||||
log.Printf("Channels: Telegram poll error: %v", err)
|
||||
timer := time.NewTimer(telegramPollInterval)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
timer.Stop()
|
||||
return nil
|
||||
case <-time.After(telegramPollInterval):
|
||||
case <-timer.C:
|
||||
continue
|
||||
}
|
||||
}
|
||||
@@ -513,7 +517,9 @@ func (t *TelegramAdapter) StartPolling(ctx context.Context, config map[string]in
|
||||
|
||||
// Acknowledge the button press (removes loading spinner)
|
||||
ackCfg := tgbotapi.NewCallback(cb.ID, "Received")
|
||||
bot.Send(ackCfg)
|
||||
if _, err := bot.Send(ackCfg); err != nil {
|
||||
log.Printf("telegram: failed to send callback ack: %v", err)
|
||||
}
|
||||
|
||||
// Update the message to show what was clicked
|
||||
decision := "approved"
|
||||
@@ -525,7 +531,9 @@ func (t *TelegramAdapter) StartPolling(ctx context.Context, config map[string]in
|
||||
cb.Message.MessageID,
|
||||
cb.Message.Text+"\n\n✅ CEO "+decision,
|
||||
)
|
||||
bot.Send(editMsg)
|
||||
if _, err := bot.Send(editMsg); err != nil {
|
||||
log.Printf("telegram: failed to send edit message: %v", err)
|
||||
}
|
||||
|
||||
// Route the decision as an inbound message to the agent
|
||||
inbound := &InboundMessage{
|
||||
|
||||
@@ -41,8 +41,9 @@ type EventType string
|
||||
// scan-friendly as it grows.
|
||||
const (
|
||||
// Chat / agent messaging — surfaces in canvas chat panels.
|
||||
EventAgentMessage EventType = "AGENT_MESSAGE"
|
||||
EventA2AResponse EventType = "A2A_RESPONSE"
|
||||
EventAgentMessage EventType = "AGENT_MESSAGE"
|
||||
EventA2AResponse EventType = "A2A_RESPONSE"
|
||||
EventUserMessage EventType = "USER_MESSAGE"
|
||||
EventActivityLogged EventType = "ACTIVITY_LOGGED"
|
||||
EventChannelMessage EventType = "CHANNEL_MESSAGE"
|
||||
|
||||
@@ -95,6 +96,7 @@ const (
|
||||
var AllEventTypes = []EventType{
|
||||
EventA2AResponse,
|
||||
EventActivityLogged,
|
||||
EventUserMessage,
|
||||
EventAgentAssigned,
|
||||
EventAgentCardUpdated,
|
||||
EventAgentMessage,
|
||||
|
||||
@@ -41,6 +41,7 @@ func TestAllEventTypes_IsSnapshot(t *testing.T) {
|
||||
"DELEGATION_STATUS",
|
||||
"EXTERNAL_CREDENTIALS_ROTATED",
|
||||
"TASK_UPDATED",
|
||||
"USER_MESSAGE",
|
||||
"WORKSPACE_AWAITING_AGENT",
|
||||
"WORKSPACE_DEGRADED",
|
||||
"WORKSPACE_HEARTBEAT",
|
||||
|
||||
@@ -932,7 +932,12 @@ func applyIdleTimeout(parent context.Context, b *events.Broadcaster, workspaceID
|
||||
ctx, cancel := context.WithCancel(parent)
|
||||
sub, unsub := b.SubscribeSSE(workspaceID)
|
||||
go func() {
|
||||
defer unsub()
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("a2a_proxy: PANIC in SSE idle watcher for %s: %v", workspaceID, r)
|
||||
}
|
||||
unsub()
|
||||
}()
|
||||
timer := time.NewTimer(idle)
|
||||
defer timer.Stop()
|
||||
for {
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"log"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
@@ -344,6 +345,19 @@ func (h *WorkspaceHandler) logA2ASuccess(ctx context.Context, workspaceID, calle
|
||||
"duration_ms": durationMs,
|
||||
})
|
||||
}
|
||||
|
||||
// #228: fan user's own outbound message to all sessions of the workspace.
|
||||
// When a canvas user sends a message (callerID == "" and method == "message/send"),
|
||||
// the originating session already inserted it optimistically in useChatSend.
|
||||
// Other sessions see nothing until a manual refresh — this broadcast closes
|
||||
// that gap. The originating session collapses its optimistic copy via the
|
||||
// 3-second appendMessageDeduped window (same role + content = deduped).
|
||||
if callerID == "" && a2aMethod == "message/send" && statusCode < 400 {
|
||||
userPayload := extractCanvasUserMessage(body)
|
||||
if userPayload != nil {
|
||||
h.broadcaster.BroadcastOnly(workspaceID, string(events.EventUserMessage), userPayload)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func nilIfEmpty(s string) *string {
|
||||
@@ -393,6 +407,110 @@ func validateCallerToken(ctx context.Context, c *gin.Context, callerID string) e
|
||||
// matching (the wsauth errors are typed for the invalid case).
|
||||
var errInvalidCallerToken = errors.New("missing caller auth token")
|
||||
|
||||
// extractCanvasUserMessage parses an A2A JSON-RPC request body and extracts
|
||||
// the user-authored text and attachments from a canvas-initiated message/send.
|
||||
// Returns nil when the body is not a canvas user message (empty, malformed,
|
||||
// or not a message/send from canvas). The returned payload is safe to pass
|
||||
// directly to BroadcastOnly — nil fields are omitted from JSON.
|
||||
func extractCanvasUserMessage(body []byte) map[string]interface{} {
|
||||
if len(body) == 0 {
|
||||
return nil
|
||||
}
|
||||
var top map[string]json.RawMessage
|
||||
if err := json.Unmarshal(body, &top); err != nil {
|
||||
return nil
|
||||
}
|
||||
// Only handle message/send from canvas
|
||||
var method string
|
||||
if err := json.Unmarshal(top["method"], &method); err != nil || method != "message/send" {
|
||||
return nil
|
||||
}
|
||||
params, ok := top["params"]
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
var paramsMap map[string]json.RawMessage
|
||||
if err := json.Unmarshal(params, ¶msMap); err != nil {
|
||||
return nil
|
||||
}
|
||||
msgRaw, ok := paramsMap["message"]
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
var msg map[string]json.RawMessage
|
||||
if err := json.Unmarshal(msgRaw, &msg); err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
// role field: only broadcast user-role messages (canvas users)
|
||||
var role string
|
||||
if err := json.Unmarshal(msg["role"], &role); err != nil || role != "user" {
|
||||
return nil
|
||||
}
|
||||
|
||||
result := make(map[string]interface{})
|
||||
|
||||
// Extract messageId if present
|
||||
var mid string
|
||||
if err := json.Unmarshal(msg["messageId"], &mid); err == nil && mid != "" {
|
||||
result["messageId"] = mid
|
||||
}
|
||||
|
||||
// Extract text from parts — accumulate all text parts into a single string
|
||||
var parts []json.RawMessage
|
||||
if err := json.Unmarshal(msg["parts"], &parts); err == nil {
|
||||
var texts []string
|
||||
var fileAttachments []map[string]interface{}
|
||||
for _, pRaw := range parts {
|
||||
var p map[string]json.RawMessage
|
||||
if err := json.Unmarshal(pRaw, &p); err != nil {
|
||||
continue
|
||||
}
|
||||
var t string
|
||||
if err := json.Unmarshal(p["text"], &t); err == nil && t != "" {
|
||||
texts = append(texts, t)
|
||||
}
|
||||
var fileRaw json.RawMessage
|
||||
if err := json.Unmarshal(p["file"], &fileRaw); err == nil && fileRaw != nil {
|
||||
var f map[string]json.RawMessage
|
||||
if err := json.Unmarshal(fileRaw, &f); err == nil {
|
||||
att := make(map[string]interface{})
|
||||
var s string
|
||||
if err := json.Unmarshal(f["uri"], &s); err == nil {
|
||||
att["uri"] = s
|
||||
}
|
||||
if err := json.Unmarshal(f["name"], &s); err == nil {
|
||||
att["name"] = s
|
||||
}
|
||||
if err := json.Unmarshal(f["mimeType"], &s); err == nil {
|
||||
att["mimeType"] = s
|
||||
}
|
||||
var n float64
|
||||
if err := json.Unmarshal(f["size"], &n); err == nil {
|
||||
att["size"] = n
|
||||
}
|
||||
if len(att) > 0 {
|
||||
fileAttachments = append(fileAttachments, att)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(texts) > 0 {
|
||||
// Join with newlines — user may have sent multiple text parts
|
||||
result["message"] = strings.Join(texts, "\n")
|
||||
}
|
||||
if len(fileAttachments) > 0 {
|
||||
result["attachments"] = fileAttachments
|
||||
}
|
||||
}
|
||||
|
||||
// Drop empty payloads
|
||||
if len(result) == 0 {
|
||||
return nil
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// extractToolTrace pulls metadata.tool_trace from an A2A JSON-RPC response.
|
||||
// Returns nil when absent or malformed — callers can pass it straight through.
|
||||
func extractToolTrace(respBody []byte) json.RawMessage {
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestExtractCanvasUserMessage_TextOnly covers the primary path: a canvas user
|
||||
// sends a plain text message with no attachments.
|
||||
func TestExtractCanvasUserMessage_TextOnly(t *testing.T) {
|
||||
body := []byte(`{
|
||||
"method": "message/send",
|
||||
"params": {
|
||||
"message": {
|
||||
"role": "user",
|
||||
"messageId": "msg-abc-123",
|
||||
"parts": [
|
||||
{"kind": "text", "text": "Hello, agent!"}
|
||||
]
|
||||
}
|
||||
}
|
||||
}`)
|
||||
got := extractCanvasUserMessage(body)
|
||||
if got == nil {
|
||||
t.Fatal("expected non-nil payload for text message")
|
||||
}
|
||||
if got["message"] != "Hello, agent!" {
|
||||
t.Errorf("message = %v, want %q", got["message"], "Hello, agent!")
|
||||
}
|
||||
mid, ok := got["messageId"].(string)
|
||||
if !ok || mid != "msg-abc-123" {
|
||||
t.Errorf("messageId = %v, want %q", got["messageId"], "msg-abc-123")
|
||||
}
|
||||
_, hasAttachments := got["attachments"]
|
||||
if hasAttachments {
|
||||
t.Errorf("unexpected attachments: %v", got["attachments"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_FileOnly covers a user message with a file but no text.
|
||||
func TestExtractCanvasUserMessage_FileOnly(t *testing.T) {
|
||||
body := []byte(`{
|
||||
"method": "message/send",
|
||||
"params": {
|
||||
"message": {
|
||||
"role": "user",
|
||||
"messageId": "msg-file-456",
|
||||
"parts": [
|
||||
{
|
||||
"kind": "file",
|
||||
"file": {
|
||||
"name": "report.pdf",
|
||||
"uri": "workspace:/uploads/report.pdf",
|
||||
"mimeType": "application/pdf",
|
||||
"size": 4096
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}`)
|
||||
got := extractCanvasUserMessage(body)
|
||||
if got == nil {
|
||||
t.Fatal("expected non-nil payload for file-only message")
|
||||
}
|
||||
if got["message"] != nil {
|
||||
t.Errorf("unexpected message text: %v", got["message"])
|
||||
}
|
||||
attachments, ok := got["attachments"].([]map[string]interface{})
|
||||
if !ok || len(attachments) != 1 {
|
||||
t.Fatalf("attachments = %v, want 1-element array", got["attachments"])
|
||||
}
|
||||
att := attachments[0]
|
||||
if att["uri"] != "workspace:/uploads/report.pdf" {
|
||||
t.Errorf("uri = %v, want %q", att["uri"], "workspace:/uploads/report.pdf")
|
||||
}
|
||||
if att["name"] != "report.pdf" {
|
||||
t.Errorf("name = %v, want %q", att["name"], "report.pdf")
|
||||
}
|
||||
if att["mimeType"] != "application/pdf" {
|
||||
t.Errorf("mimeType = %v, want %q", att["mimeType"], "application/pdf")
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_TextAndFile covers a user message with both text and a file.
|
||||
func TestExtractCanvasUserMessage_TextAndFile(t *testing.T) {
|
||||
body := []byte(`{
|
||||
"method": "message/send",
|
||||
"params": {
|
||||
"message": {
|
||||
"role": "user",
|
||||
"parts": [
|
||||
{"kind": "text", "text": "Here is the file:"},
|
||||
{"kind": "text", "text": "see below"},
|
||||
{
|
||||
"kind": "file",
|
||||
"file": {
|
||||
"name": "data.csv",
|
||||
"uri": "workspace:/exports/data.csv",
|
||||
"mimeType": "text/csv",
|
||||
"size": 8192
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}`)
|
||||
got := extractCanvasUserMessage(body)
|
||||
if got == nil {
|
||||
t.Fatal("expected non-nil payload")
|
||||
}
|
||||
// Two text parts are joined with newline
|
||||
if got["message"] != "Here is the file:\nsee below" {
|
||||
t.Errorf("message = %v, want %q", got["message"], "Here is the file:\nsee below")
|
||||
}
|
||||
attachments, ok := got["attachments"].([]map[string]interface{})
|
||||
if !ok || len(attachments) != 1 {
|
||||
t.Fatalf("attachments = %v, want 1-element array", got["attachments"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_Malformed covers malformed JSON.
|
||||
func TestExtractCanvasUserMessage_Malformed(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
body []byte
|
||||
}{
|
||||
{"not JSON", []byte(`{not valid`)},
|
||||
{"wrong type top-level", []byte(`123`)},
|
||||
{"missing params", []byte(`{"method":"message/send"}`)},
|
||||
{"params not object", []byte(`{"method":"message/send","params":123}`)},
|
||||
{"missing message", []byte(`{"method":"message/send","params":{}}`)},
|
||||
{"message not object", []byte(`{"method":"message/send","params":{"message":123}}`)},
|
||||
{"role missing", []byte(`{"method":"message/send","params":{"message":{"parts":[]}}}`)},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := extractCanvasUserMessage(tc.body); got != nil {
|
||||
t.Errorf("expected nil for %s, got %v", tc.name, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_NotUserRole covers agent/workspace callers
|
||||
// whose role is not "user" — these should not be broadcast as USER_MESSAGE.
|
||||
func TestExtractCanvasUserMessage_NotUserRole(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
body []byte
|
||||
}{
|
||||
{
|
||||
"agent role",
|
||||
[]byte(`{"method":"message/send","params":{"message":{"role":"agent","parts":[{"kind":"text","text":"hello"}]}}}`),
|
||||
},
|
||||
{
|
||||
"assistant role",
|
||||
[]byte(`{"method":"message/send","params":{"message":{"role":"assistant","parts":[{"kind":"text","text":"hello"}]}}}`),
|
||||
},
|
||||
{
|
||||
"empty role",
|
||||
[]byte(`{"method":"message/send","params":{"message":{"role":"","parts":[{"kind":"text","text":"hello"}]}}}`),
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := extractCanvasUserMessage(tc.body); got != nil {
|
||||
t.Errorf("expected nil for role=%s, got %v", tc.name, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_NotMessageSend covers non-message/send methods.
|
||||
func TestExtractCanvasUserMessage_NotMessageSend(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
method string
|
||||
}{
|
||||
{"tasks/send", "tasks/send"},
|
||||
{"initialize", "initialize"},
|
||||
{"ping", "ping"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"method": tc.method,
|
||||
"params": map[string]interface{}{
|
||||
"message": map[string]interface{}{
|
||||
"role": "user",
|
||||
"parts": []map[string]interface{}{{"kind": "text", "text": "hello"}},
|
||||
},
|
||||
},
|
||||
})
|
||||
if got := extractCanvasUserMessage(body); got != nil {
|
||||
t.Errorf("expected nil for method=%q, got %v", tc.method, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_BlankOrEmpty covers text with only whitespace
|
||||
// and empty parts arrays.
|
||||
func TestExtractCanvasUserMessage_BlankOrEmpty(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
body []byte
|
||||
}{
|
||||
{
|
||||
"empty text part",
|
||||
[]byte(`{"method":"message/send","params":{"message":{"role":"user","parts":[{"kind":"text","text":""}]}}}`),
|
||||
},
|
||||
{
|
||||
"empty parts array",
|
||||
[]byte(`{"method":"message/send","params":{"message":{"role":"user","parts":[]}}}`),
|
||||
},
|
||||
{
|
||||
"whitespace-only text — still included as valid content",
|
||||
[]byte(`{"method":"message/send","params":{"message":{"role":"user","parts":[{"kind":"text","text":" "}]}}}`),
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := extractCanvasUserMessage(tc.body)
|
||||
if tc.name == "whitespace-only text — still included as valid content" {
|
||||
// Whitespace-only text is valid content — preserve it as-is.
|
||||
// Canvas dedup collapses identical copies; whitespace is not stripped.
|
||||
if got == nil {
|
||||
t.Error("expected non-nil for whitespace-only text")
|
||||
} else if got["message"] != " " {
|
||||
t.Errorf("message = %q, want %q", got["message"], " ")
|
||||
}
|
||||
return
|
||||
}
|
||||
if got != nil {
|
||||
t.Errorf("expected nil for %s, got %v", tc.name, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractCanvasUserMessage_Unicode covers non-ASCII text.
|
||||
func TestExtractCanvasUserMessage_Unicode(t *testing.T) {
|
||||
body := []byte(`{
|
||||
"method": "message/send",
|
||||
"params": {
|
||||
"message": {
|
||||
"role": "user",
|
||||
"parts": [
|
||||
{"kind": "text", "text": "こんにちは世界 🌍 日本語"}
|
||||
]
|
||||
}
|
||||
}
|
||||
}`)
|
||||
got := extractCanvasUserMessage(body)
|
||||
if got == nil {
|
||||
t.Fatal("expected non-nil payload for unicode message")
|
||||
}
|
||||
if got["message"] != "こんにちは世界 🌍 日本語" {
|
||||
t.Errorf("message = %v, want %q", got["message"], "こんにちは世界 🌍 日本語")
|
||||
}
|
||||
}
|
||||
@@ -51,23 +51,29 @@ func (h *ApprovalsHandler) Create(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
h.broadcaster.RecordAndBroadcast(ctx, string(events.EventApprovalRequested), workspaceID, map[string]interface{}{
|
||||
if err := h.broadcaster.RecordAndBroadcast(ctx, string(events.EventApprovalRequested), workspaceID, map[string]interface{}{
|
||||
"approval_id": approvalID,
|
||||
"action": body.Action,
|
||||
"reason": body.Reason,
|
||||
"task_id": body.TaskID,
|
||||
})
|
||||
}); err != nil {
|
||||
log.Printf("approvals: failed to broadcast approval requested: %v", err)
|
||||
}
|
||||
|
||||
// Auto-escalate to parent
|
||||
var parentID *string
|
||||
db.DB.QueryRowContext(ctx, `SELECT parent_id FROM workspaces WHERE id = $1`, workspaceID).Scan(&parentID)
|
||||
if err := db.DB.QueryRowContext(ctx, `SELECT parent_id FROM workspaces WHERE id = $1`, workspaceID).Scan(&parentID); err != nil {
|
||||
log.Printf("approvals: failed to lookup parent for escalation: %v", err)
|
||||
}
|
||||
if parentID != nil {
|
||||
h.broadcaster.RecordAndBroadcast(ctx, string(events.EventApprovalEscalated), *parentID, map[string]interface{}{
|
||||
if err := h.broadcaster.RecordAndBroadcast(ctx, string(events.EventApprovalEscalated), *parentID, map[string]interface{}{
|
||||
"approval_id": approvalID,
|
||||
"from_workspace_id": workspaceID,
|
||||
"action": body.Action,
|
||||
"reason": body.Reason,
|
||||
})
|
||||
}); err != nil {
|
||||
log.Printf("approvals: failed to broadcast approval escalated: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
c.JSON(http.StatusCreated, gin.H{"approval_id": approvalID, "status": "pending"})
|
||||
@@ -80,10 +86,12 @@ func (h *ApprovalsHandler) ListAll(c *gin.Context) {
|
||||
ctx := c.Request.Context()
|
||||
|
||||
// Auto-expire stale approvals (older than 10 min)
|
||||
db.DB.ExecContext(ctx, `
|
||||
if _, err := db.DB.ExecContext(ctx, `
|
||||
UPDATE approval_requests SET status = 'denied', decided_by = 'auto-expired', decided_at = now()
|
||||
WHERE status = 'pending' AND created_at < now() - interval '10 minutes'
|
||||
`)
|
||||
`); err != nil {
|
||||
log.Printf("approvals: failed to auto-expire stale approvals: %v", err)
|
||||
}
|
||||
|
||||
rows, err := db.DB.QueryContext(ctx, `
|
||||
SELECT a.id, a.workspace_id, w.name, a.action, a.reason, a.status, a.created_at
|
||||
@@ -211,11 +219,13 @@ func (h *ApprovalsHandler) Decide(c *gin.Context) {
|
||||
eventType = "APPROVAL_DENIED"
|
||||
}
|
||||
|
||||
h.broadcaster.RecordAndBroadcast(ctx, eventType, workspaceID, map[string]interface{}{
|
||||
if err := h.broadcaster.RecordAndBroadcast(ctx, eventType, workspaceID, map[string]interface{}{
|
||||
"approval_id": approvalID,
|
||||
"decision": body.Decision,
|
||||
"decided_by": decidedBy,
|
||||
})
|
||||
}); err != nil {
|
||||
log.Printf("approvals: failed to broadcast approval decision: %v", err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": body.Decision, "approval_id": approvalID})
|
||||
}
|
||||
|
||||
@@ -558,6 +558,11 @@ func (h *ChannelHandler) Webhook(c *gin.Context) {
|
||||
|
||||
// Process asynchronously — don't block the webhook response
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("Channels: PANIC in async HandleInbound for workspace %s: %v", ch.WorkspaceID[:12], r)
|
||||
}
|
||||
}()
|
||||
bgCtx := context.Background()
|
||||
if err := h.manager.HandleInbound(bgCtx, ch, msg); err != nil {
|
||||
log.Printf("Channels: async HandleInbound error for workspace %s: %v", ch.WorkspaceID[:12], err)
|
||||
|
||||
@@ -747,6 +747,14 @@ func (h *DelegationHandler) listDelegationsFromLedger(ctx context.Context, works
|
||||
entry["response_preview"] = textutil.TruncateBytes(resultPreview.String, 300)
|
||||
}
|
||||
if errorDetail.Valid && errorDetail.String != "" {
|
||||
// Emit both keys: `error_detail` is the canonical field the
|
||||
// Python poll-mode consumer (a2a_tools_delegation.py:184)
|
||||
// reads from /delegations rows — without it, poll-mode
|
||||
// silently loses the failure reason and falls through to
|
||||
// the generic "delegation failed" string. `error` is kept
|
||||
// for back-compat with existing UI surfaces that read the
|
||||
// shorter name.
|
||||
entry["error_detail"] = errorDetail.String
|
||||
entry["error"] = errorDetail.String
|
||||
}
|
||||
if lastHeartbeat != nil {
|
||||
@@ -808,6 +816,8 @@ func (h *DelegationHandler) listDelegationsFromActivityLogs(ctx context.Context,
|
||||
entry["delegation_id"] = delegationID
|
||||
}
|
||||
if errorDetail != "" {
|
||||
// Emit both keys per the rename: see listDelegationsFromLedger.
|
||||
entry["error_detail"] = errorDetail
|
||||
entry["error"] = errorDetail
|
||||
}
|
||||
if responseBody != "" {
|
||||
|
||||
@@ -1546,6 +1546,71 @@ func TestListDelegations_LedgerEmptyFallsBackToActivityLogs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- ListDelegations: activity_logs failed row emits BOTH error + error_detail ----------
|
||||
|
||||
// Field-rename pin (P1 #348 / RFC #2829 PR-2 follow-up): the legacy
|
||||
// activity_logs fallback path must also emit `error_detail` alongside
|
||||
// the historical `error` key. Without this, poll-mode (which reads
|
||||
// `error_detail`) silently loses the failure reason when the ledger
|
||||
// is empty and the handler falls back to activity_logs.
|
||||
func TestListDelegations_ActivityLogsFailedEmitsBothErrorKeys(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
dh := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
// Ledger empty → fall back to activity_logs.
|
||||
mock.ExpectQuery("SELECT d.delegation_id, d.caller_id, d.callee_id, d.task_preview").
|
||||
WithArgs("ws-source").
|
||||
WillReturnRows(sqlmock.NewRows([]string{
|
||||
"delegation_id", "caller_id", "callee_id", "task_preview",
|
||||
"status", "result_preview", "error_detail", "last_heartbeat",
|
||||
"deadline", "created_at", "updated_at",
|
||||
}))
|
||||
|
||||
now := time.Now()
|
||||
activityRows := sqlmock.NewRows([]string{
|
||||
"id", "activity_type", "source_id", "target_id",
|
||||
"summary", "status", "error_detail", "response_body",
|
||||
"delegation_id", "created_at",
|
||||
}).AddRow(
|
||||
"act-failed", "delegate_result", "ws-source", "ws-target",
|
||||
"Delegation failed", "error", "codex runtime timed out", "",
|
||||
"del-failed-002", now,
|
||||
)
|
||||
mock.ExpectQuery("SELECT id, activity_type").
|
||||
WithArgs("ws-source").
|
||||
WillReturnRows(activityRows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-source"}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/ws-source/delegations", nil)
|
||||
|
||||
dh.ListDelegations(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp []map[string]interface{}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("failed to parse response: %v", err)
|
||||
}
|
||||
if len(resp) != 1 {
|
||||
t.Fatalf("expected 1 row, got %d", len(resp))
|
||||
}
|
||||
if resp[0]["error"] != "codex runtime timed out" {
|
||||
t.Errorf("expected `error` field set, got %v", resp[0]["error"])
|
||||
}
|
||||
if resp[0]["error_detail"] != "codex runtime timed out" {
|
||||
t.Errorf("expected `error_detail` field set (poll-mode contract), got %v", resp[0]["error_detail"])
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- ListDelegations: both ledger and activity_logs empty → [] ----------
|
||||
|
||||
func TestListDelegations_BothEmptyReturnsEmptyArray(t *testing.T) {
|
||||
@@ -1744,7 +1809,15 @@ func TestListDelegations_LedgerFailedIncludesErrorDetail(t *testing.T) {
|
||||
t.Errorf("expected status 'failed', got %v", resp[0]["status"])
|
||||
}
|
||||
if resp[0]["error"] != "Callee workspace not reachable" {
|
||||
t.Errorf("expected error detail, got %v", resp[0]["error"])
|
||||
t.Errorf("expected error detail under `error`, got %v", resp[0]["error"])
|
||||
}
|
||||
// Field-rename pin (P1 #348 / RFC #2829 PR-2 follow-up): the
|
||||
// Python poll-mode consumer in a2a_tools_delegation.py:184 reads
|
||||
// `error_detail`, not `error`. Both keys MUST be present so polling
|
||||
// surfaces the real failure reason instead of falling through to
|
||||
// the generic "delegation failed" string.
|
||||
if resp[0]["error_detail"] != "Callee workspace not reachable" {
|
||||
t.Errorf("expected error detail under `error_detail`, got %v", resp[0]["error_detail"])
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
|
||||
@@ -239,7 +239,7 @@ func (h *DiscoveryHandler) Peers(c *gin.Context) {
|
||||
|
||||
// Siblings
|
||||
if parentID.Valid {
|
||||
siblings, _ := queryPeerMaps(`
|
||||
siblings, _ := queryPeerMaps(ctx, `
|
||||
SELECT w.id, w.name, COALESCE(w.role, ''), w.tier, w.status,
|
||||
COALESCE(w.agent_card, 'null'::jsonb), COALESCE(w.url, ''),
|
||||
w.parent_id, w.active_tasks
|
||||
@@ -247,7 +247,7 @@ func (h *DiscoveryHandler) Peers(c *gin.Context) {
|
||||
parentID.String, workspaceID)
|
||||
peers = append(peers, siblings...)
|
||||
} else {
|
||||
siblings, _ := queryPeerMaps(`
|
||||
siblings, _ := queryPeerMaps(ctx, `
|
||||
SELECT w.id, w.name, COALESCE(w.role, ''), w.tier, w.status,
|
||||
COALESCE(w.agent_card, 'null'::jsonb), COALESCE(w.url, ''),
|
||||
w.parent_id, w.active_tasks
|
||||
@@ -257,7 +257,7 @@ func (h *DiscoveryHandler) Peers(c *gin.Context) {
|
||||
}
|
||||
|
||||
// Children
|
||||
children, _ := queryPeerMaps(`
|
||||
children, _ := queryPeerMaps(ctx, `
|
||||
SELECT w.id, w.name, COALESCE(w.role, ''), w.tier, w.status,
|
||||
COALESCE(w.agent_card, 'null'::jsonb), COALESCE(w.url, ''),
|
||||
w.parent_id, w.active_tasks
|
||||
@@ -266,7 +266,7 @@ func (h *DiscoveryHandler) Peers(c *gin.Context) {
|
||||
|
||||
// Parent
|
||||
if parentID.Valid {
|
||||
parent, _ := queryPeerMaps(`
|
||||
parent, _ := queryPeerMaps(ctx, `
|
||||
SELECT w.id, w.name, COALESCE(w.role, ''), w.tier, w.status,
|
||||
COALESCE(w.agent_card, 'null'::jsonb), COALESCE(w.url, ''),
|
||||
w.parent_id, w.active_tasks
|
||||
@@ -303,8 +303,8 @@ func filterPeersByQuery(peers []map[string]interface{}, q string) []map[string]i
|
||||
}
|
||||
|
||||
// queryPeerMaps returns clean JSON-serializable maps instead of Workspace structs.
|
||||
func queryPeerMaps(query string, args ...interface{}) ([]map[string]interface{}, error) {
|
||||
rows, err := db.DB.Query(query, args...)
|
||||
func queryPeerMaps(ctx context.Context, query string, args ...interface{}) ([]map[string]interface{}, error) {
|
||||
rows, err := db.DB.QueryContext(ctx, query, args...)
|
||||
if err != nil {
|
||||
log.Printf("queryPeerMaps error: %v", err)
|
||||
return nil, err
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"strings"
|
||||
@@ -54,6 +55,22 @@ func updateMCPDelegationStatus(ctx context.Context, db *sql.DB, workspaceID, del
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// mcpHTTPClient is a dedicated client for MCP bridge A2A calls.
|
||||
// Per-request deadlines are enforced via context (30 s sync, 8 s async).
|
||||
// Transport-level timeouts ensure dead workspaces fail fast instead of
|
||||
// hanging on OS default TCP timeouts (~75 s Linux).
|
||||
var mcpHTTPClient = &http.Client{
|
||||
Transport: &http.Transport{
|
||||
DialContext: (&net.Dialer{
|
||||
Timeout: 5 * time.Second,
|
||||
KeepAlive: 30 * time.Second,
|
||||
}).DialContext,
|
||||
ResponseHeaderTimeout: 30 * time.Second,
|
||||
TLSHandshakeTimeout: 5 * time.Second,
|
||||
},
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Tool implementations
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
@@ -231,7 +248,7 @@ func (h *MCPHandler) toolDelegateTask(ctx context.Context, callerID string, args
|
||||
// so this header reflects a verified caller identity, not a spoofable value.
|
||||
httpReq.Header.Set("X-Workspace-ID", callerID)
|
||||
|
||||
resp, err := http.DefaultClient.Do(httpReq)
|
||||
resp, err := mcpHTTPClient.Do(httpReq)
|
||||
if err != nil {
|
||||
updateMCPDelegationStatus(ctx, h.database, callerID, delegationID, "failed", err.Error())
|
||||
return "", fmt.Errorf("A2A call failed: %w", err)
|
||||
@@ -279,6 +296,11 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
|
||||
// Fire and forget in a detached goroutine. Use a background context so
|
||||
// the call is not cancelled when the HTTP request completes.
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("MCPHandler.delegate_task_async: PANIC for %s → %s: %v", callerID, targetID, r)
|
||||
}
|
||||
}()
|
||||
bgCtx, cancel := context.WithTimeout(context.Background(), mcpAsyncCallTimeout)
|
||||
defer cancel()
|
||||
|
||||
@@ -314,7 +336,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
|
||||
httpReq.Header.Set("Content-Type", "application/json")
|
||||
httpReq.Header.Set("X-Workspace-ID", callerID)
|
||||
|
||||
resp, err := http.DefaultClient.Do(httpReq)
|
||||
resp, err := mcpHTTPClient.Do(httpReq)
|
||||
if err != nil {
|
||||
log.Printf("MCPHandler.delegate_task_async: A2A call to %s: %v", targetID, err)
|
||||
return
|
||||
|
||||
@@ -201,6 +201,11 @@ func (h *PluginsHandler) uninstallViaDocker(ctx context.Context, c *gin.Context,
|
||||
// Auto-restart (small delay to ensure fs writes are flushed)
|
||||
if h.restartFunc != nil {
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("plugins_install: PANIC in delayed restart for %s: %v", workspaceID, r)
|
||||
}
|
||||
}()
|
||||
time.Sleep(2 * time.Second)
|
||||
h.restartFunc(workspaceID)
|
||||
}()
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
func TestListRegistry_EmptyDir(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
h := NewPluginsHandler(dir, nil, nil)
|
||||
|
||||
got := h.listRegistryFiltered("")
|
||||
if len(got) != 0 {
|
||||
t.Errorf("expected empty list, got %d plugins", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListRegistry_IgnoresFiles(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "not-a-plugin.txt"), []byte("x"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
h := NewPluginsHandler(dir, nil, nil)
|
||||
|
||||
got := h.listRegistryFiltered("")
|
||||
if len(got) != 0 {
|
||||
t.Errorf("expected empty list (files ignored), got %d", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListRegistry_SinglePlugin(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
pluginDir := filepath.Join(dir, "my-plugin")
|
||||
if err := os.Mkdir(pluginDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte("name: my-plugin\nversion: 1.0.0\n"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
h := NewPluginsHandler(dir, nil, nil)
|
||||
|
||||
got := h.listRegistryFiltered("")
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected 1 plugin, got %d", len(got))
|
||||
}
|
||||
if got[0].Name != "my-plugin" {
|
||||
t.Errorf("expected name 'my-plugin', got %q", got[0].Name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListRegistry_FiltersByRuntime(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
for _, spec := range []struct{ name, yaml string }{
|
||||
{"runtime-a", "name: runtime-a\nruntimes:\n - claude-code\n"},
|
||||
{"runtime-b", "name: runtime-b\nruntimes:\n - hermes\n"},
|
||||
{"universal", "name: universal\nversion: 1.0.0\n"},
|
||||
} {
|
||||
pd := filepath.Join(dir, spec.name)
|
||||
if err := os.Mkdir(pd, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(pd, "plugin.yaml"), []byte(spec.yaml), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
h := NewPluginsHandler(dir, nil, nil)
|
||||
|
||||
// Filter to claude-code: runtime-a matches, universal (no runtimes field)
|
||||
// is always included per supportsRuntime semantics.
|
||||
got := h.listRegistryFiltered("claude-code")
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("expected 2 (runtime-a + universal), got %d: %v", len(got), func() []string {
|
||||
ns := make([]string, len(got))
|
||||
for i, p := range got { ns[i] = p.Name }
|
||||
return ns
|
||||
}())
|
||||
}
|
||||
}
|
||||
|
||||
func TestListRegistry_PluginWithNoRuntimeDeclarations_AlwaysIncluded(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
pd := filepath.Join(dir, "universal-plugin")
|
||||
if err := os.Mkdir(pd, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(pd, "plugin.yaml"), []byte("name: universal-plugin\nversion: 1.0.0\n"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
h := NewPluginsHandler(dir, nil, nil)
|
||||
|
||||
// When plugin declares no runtimes, it should always be included (try-it).
|
||||
got := h.listRegistryFiltered("any-runtime")
|
||||
if len(got) != 1 {
|
||||
t.Errorf("expected 1 plugin (unspecified runtime), got %d", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListRegistry_ReadDirError_ReturnsEmpty(t *testing.T) {
|
||||
h := NewPluginsHandler("/nonexistent/path/for/plugins", nil, nil)
|
||||
got := h.listRegistryFiltered("")
|
||||
if len(got) != 0 {
|
||||
t.Errorf("expected empty list on ReadDir error, got %d", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListRegistry_HTTPEndpoint(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
pd := filepath.Join(dir, "test-plugin")
|
||||
if err := os.Mkdir(pd, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(pd, "plugin.yaml"), []byte("name: test-plugin\nversion: 2.0.0\n"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
h := NewPluginsHandler(dir, nil, nil)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/plugins", nil)
|
||||
h.ListRegistry(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var plugins []pluginInfo
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &plugins); err != nil {
|
||||
t.Fatalf("failed to parse JSON: %v", err)
|
||||
}
|
||||
if len(plugins) != 1 {
|
||||
t.Errorf("expected 1 plugin, got %d", len(plugins))
|
||||
}
|
||||
if plugins[0].Name != "test-plugin" {
|
||||
t.Errorf("expected name 'test-plugin', got %q", plugins[0].Name)
|
||||
}
|
||||
}
|
||||
@@ -133,24 +133,24 @@ func loadRestartContextData(ctx context.Context, workspaceID string) restartCont
|
||||
// message bus.
|
||||
keySet := map[string]struct{}{}
|
||||
if rows, err := db.DB.QueryContext(ctx, `SELECT key FROM global_secrets`); err == nil {
|
||||
defer rows.Close()
|
||||
for rows.Next() {
|
||||
var k string
|
||||
if rows.Scan(&k) == nil {
|
||||
keySet[k] = struct{}{}
|
||||
}
|
||||
}
|
||||
rows.Close()
|
||||
}
|
||||
if rows, err := db.DB.QueryContext(ctx,
|
||||
`SELECT key FROM workspace_secrets WHERE workspace_id = $1`, workspaceID,
|
||||
); err == nil {
|
||||
defer rows.Close()
|
||||
for rows.Next() {
|
||||
var k string
|
||||
if rows.Scan(&k) == nil {
|
||||
keySet[k] = struct{}{}
|
||||
}
|
||||
}
|
||||
rows.Close()
|
||||
}
|
||||
for k := range keySet {
|
||||
d.EnvKeys = append(d.EnvKeys, k)
|
||||
@@ -163,6 +163,8 @@ func loadRestartContextData(ctx context.Context, workspaceID string) restartCont
|
||||
// workspace's status flips to 'online' or the deadline expires.
|
||||
// Returns true on success; callers log+drop on false.
|
||||
func waitForWorkspaceOnline(ctx context.Context, workspaceID string, timeout time.Duration) bool {
|
||||
ticker := time.NewTicker(restartContextOnlinePollInterval)
|
||||
defer ticker.Stop()
|
||||
deadline := time.Now().Add(timeout)
|
||||
for time.Now().Before(deadline) {
|
||||
var status string
|
||||
@@ -174,7 +176,7 @@ func waitForWorkspaceOnline(ctx context.Context, workspaceID string, timeout tim
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return false
|
||||
case <-time.After(restartContextOnlinePollInterval):
|
||||
case <-ticker.C:
|
||||
}
|
||||
}
|
||||
return false
|
||||
|
||||
@@ -86,6 +86,40 @@ var fallbackRuntimes = map[string]struct{}{
|
||||
"mock": {},
|
||||
}
|
||||
|
||||
// stripJSON5Comments removes // single-line comments from JSON5-formatted
|
||||
// data. The Integration Tester appends "// Triggered by <job>" to
|
||||
// manifest.json after cloning, which causes json.Unmarshal to fail with
|
||||
// "invalid character '/'". This strips trailing and mid-file comments
|
||||
// before parsing so Go's strict JSON parser accepts JSON5 files.
|
||||
//
|
||||
// Handles:
|
||||
// - Standalone comment lines: // comment
|
||||
// - Trailing comments: "key": "value", // comment
|
||||
// - Comments inside strings are NOT touched ("http://example.com")
|
||||
func stripJSON5Comments(data []byte) []byte {
|
||||
var result []byte
|
||||
inString := false
|
||||
i := 0
|
||||
for i < len(data) {
|
||||
if data[i] == '"' && (i == 0 || data[i-1] != '\\') {
|
||||
inString = !inString
|
||||
result = append(result, data[i])
|
||||
i++
|
||||
continue
|
||||
}
|
||||
if !inString && i+1 < len(data) && data[i] == '/' && data[i+1] == '/' {
|
||||
// Skip to end of line
|
||||
for i < len(data) && data[i] != '\n' {
|
||||
i++
|
||||
}
|
||||
continue
|
||||
}
|
||||
result = append(result, data[i])
|
||||
i++
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// loadRuntimesFromManifest builds the runtime allowlist from
|
||||
// manifest.json. Each workspace_templates[].name is normalized to its
|
||||
// base runtime identifier (strips the `-default` suffix templates
|
||||
@@ -101,6 +135,9 @@ func loadRuntimesFromManifest(path string) (map[string]struct{}, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Strip JSON5 // comments before parsing. The Integration Tester
|
||||
// appends "// Triggered by <job>" to manifest.json after cloning.
|
||||
data = stripJSON5Comments(data)
|
||||
var m manifestFile
|
||||
if err := json.Unmarshal(data, &m); err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -8,6 +8,7 @@ package handlers
|
||||
// fallback (tested at the initKnownRuntimes level via integration).
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
@@ -111,3 +112,133 @@ func keys(m map[string]struct{}) []string {
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// ── stripJSON5Comments tests ─────────────────────────────────────────────────
|
||||
|
||||
func TestStripJSON5Comments_Standalone(t *testing.T) {
|
||||
// Whitespace before the // is preserved; only the // and its text are removed.
|
||||
// The result is still valid JSON.
|
||||
input := "{\n\t// This is a comment\n\t\"workspace_templates\": []\n}"
|
||||
got := string(stripJSON5Comments([]byte(input)))
|
||||
// Stripping should produce valid JSON: try parsing it
|
||||
var m manifestFile
|
||||
if err := json.Unmarshal([]byte(got), &m); err != nil {
|
||||
t.Errorf("output is not valid JSON: %v\ngot: %q", err, got)
|
||||
}
|
||||
if m.WorkspaceTemplates == nil {
|
||||
t.Error("WorkspaceTemplates field should be present after parsing")
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripJSON5Comments_Trailing(t *testing.T) {
|
||||
input := `{"key": "value"} // trailing comment`
|
||||
got := string(stripJSON5Comments([]byte(input)))
|
||||
want := `{"key": "value"} `
|
||||
if got != want {
|
||||
t.Errorf("trailing comment:\ngot: %q\nwant: %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripJSON5Comments_URLsPreserved(t *testing.T) {
|
||||
// URLs with // in them must NOT be stripped
|
||||
input := `{"url": "https://example.com/path"}`
|
||||
got := string(stripJSON5Comments([]byte(input)))
|
||||
if got != input {
|
||||
t.Errorf("URL stripped: got %q, want %q", got, input)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripJSON5Comments_InlineComment(t *testing.T) {
|
||||
// Whitespace before // is preserved; only the comment text is removed.
|
||||
// Result must be valid JSON.
|
||||
input := "{\n\t\"workspace_templates\": [] // inline comment\n}"
|
||||
got := string(stripJSON5Comments([]byte(input)))
|
||||
var m manifestFile
|
||||
if err := json.Unmarshal([]byte(got), &m); err != nil {
|
||||
t.Errorf("output is not valid JSON: %v\ngot: %q", err, got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripJSON5Comments_IntegrationTesterAppend(t *testing.T) {
|
||||
// Simulates what the Integration Tester does: appends // Triggered by ...
|
||||
// to the end of a valid JSON file.
|
||||
input := `{
|
||||
"workspace_templates": [
|
||||
{"name": "hermes", "repo": "org/hermes"}
|
||||
]
|
||||
}
|
||||
// Triggered by e2e-test job 12345
|
||||
`
|
||||
got := string(stripJSON5Comments([]byte(input)))
|
||||
if got == input {
|
||||
t.Error("Integration Tester comment was NOT stripped")
|
||||
}
|
||||
// After stripping, it should be valid JSON
|
||||
var m manifestFile
|
||||
if err := json.Unmarshal([]byte(got), &m); err != nil {
|
||||
t.Errorf("stripped content is not valid JSON: %v\ngot: %q", err, got)
|
||||
}
|
||||
if len(m.WorkspaceTemplates) != 1 || m.WorkspaceTemplates[0].Name != "hermes" {
|
||||
t.Errorf("workspace_templates not parsed: %+v", m.WorkspaceTemplates)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripJSON5Comments_NoComments(t *testing.T) {
|
||||
input := `{"workspace_templates": [{"name": "hermes"}]}`
|
||||
got := string(stripJSON5Comments([]byte(input)))
|
||||
if got != input {
|
||||
t.Errorf("unmodified JSON changed: got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// ── loadRuntimesFromManifest with JSON5 comments ─────────────────────────────
|
||||
|
||||
func TestLoadRuntimesFromManifest_WithJSON5TrailingComment(t *testing.T) {
|
||||
// Regression: Integration Tester appends "// Triggered by ..." to manifest.json
|
||||
// after cloning. Before the fix, this caused json.Unmarshal to fail with
|
||||
// "invalid character '/'". After the fix, the comment is stripped first.
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "manifest.json")
|
||||
manifest := `{
|
||||
"workspace_templates": [
|
||||
{"name": "hermes", "repo": "org/hermes"},
|
||||
{"name": "claude-code-default", "repo": "org/cc"}
|
||||
]
|
||||
}
|
||||
// Triggered by e2e-test job
|
||||
`
|
||||
if err := os.WriteFile(path, []byte(manifest), 0600); err != nil {
|
||||
t.Fatalf("write: %v", err)
|
||||
}
|
||||
got, err := loadRuntimesFromManifest(path)
|
||||
if err != nil {
|
||||
t.Fatalf("loadRuntimesFromManifest with trailing comment: %v", err)
|
||||
}
|
||||
for _, must := range []string{"hermes", "claude-code"} {
|
||||
if _, ok := got[must]; !ok {
|
||||
t.Errorf("expected runtime %q in result: %v", must, keys(got))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadRuntimesFromManifest_WithInlineJSON5Comment(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "manifest.json")
|
||||
// JSON5 with inline comment
|
||||
manifest := `{
|
||||
// runtime templates
|
||||
"workspace_templates": [
|
||||
{"name": "langgraph", "repo": "org/lg"} // the default
|
||||
]
|
||||
}`
|
||||
if err := os.WriteFile(path, []byte(manifest), 0600); err != nil {
|
||||
t.Fatalf("write: %v", err)
|
||||
}
|
||||
got, err := loadRuntimesFromManifest(path)
|
||||
if err != nil {
|
||||
t.Fatalf("loadRuntimesFromManifest with inline comment: %v", err)
|
||||
}
|
||||
if _, ok := got["langgraph"]; !ok {
|
||||
t.Errorf("expected langgraph in result: %v", keys(got))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -213,7 +213,12 @@ func (h *TerminalHandler) handleLocalConnect(c *gin.Context, workspaceID string)
|
||||
// Bridge: container stdout → WebSocket
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("Terminal: PANIC in stdout bridge: %v", r)
|
||||
}
|
||||
close(done)
|
||||
}()
|
||||
buf := make([]byte, 4096)
|
||||
for {
|
||||
n, err := resp.Reader.Read(buf)
|
||||
@@ -434,7 +439,12 @@ func (h *TerminalHandler) handleRemoteConnect(c *gin.Context, workspaceID, insta
|
||||
|
||||
// PTY → WebSocket
|
||||
go func() {
|
||||
defer close(done)
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("Terminal: PANIC in PTY bridge: %v", r)
|
||||
}
|
||||
close(done)
|
||||
}()
|
||||
buf := make([]byte, 4096)
|
||||
for {
|
||||
n, err := ptmx.Read(buf)
|
||||
@@ -456,6 +466,11 @@ func (h *TerminalHandler) handleRemoteConnect(c *gin.Context, workspaceID, insta
|
||||
|
||||
// WebSocket → PTY (stdin)
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("Terminal: PANIC in stdin loop: %v", r)
|
||||
}
|
||||
}()
|
||||
for {
|
||||
_, msg, rErr := conn.ReadMessage()
|
||||
if rErr != nil {
|
||||
|
||||
@@ -296,6 +296,7 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create workspace"})
|
||||
return
|
||||
}
|
||||
defer func() { _ = tx.Rollback() }()
|
||||
|
||||
maxConcurrent := payload.MaxConcurrentTasks
|
||||
if maxConcurrent <= 0 {
|
||||
@@ -478,7 +479,9 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
|
||||
// from the external agent (with this token + its URL)
|
||||
// flips the row to online.
|
||||
// Preserve BYO-compute runtime label (kimi, kimi-cli, external).
|
||||
db.DB.ExecContext(ctx, `UPDATE workspaces SET status = $1, runtime = $2, updated_at = now() WHERE id = $3`, models.StatusAwaitingAgent, normalizeExternalRuntime(payload.Runtime), id)
|
||||
if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET status = $1, runtime = $2, updated_at = now() WHERE id = $3`, models.StatusAwaitingAgent, normalizeExternalRuntime(payload.Runtime), id); err != nil {
|
||||
log.Printf("External workspace %s: status update failed: %v", id, err)
|
||||
}
|
||||
tok, tokErr := wsauth.IssueToken(ctx, db.DB, id)
|
||||
if tokErr != nil {
|
||||
log.Printf("External workspace %s: token issuance failed: %v", id, tokErr)
|
||||
|
||||
@@ -261,7 +261,7 @@ func (h *WorkspaceHandler) buildProvisionerConfig(
|
||||
workspaceAccess := payload.WorkspaceAccess
|
||||
if (workspacePath == "" || workspaceAccess == "") && db.DB != nil {
|
||||
var dbDir, dbAccess string
|
||||
if err := db.DB.QueryRow(
|
||||
if err := db.DB.QueryRowContext(ctx,
|
||||
`SELECT COALESCE(workspace_dir, ''), COALESCE(workspace_access, 'none') FROM workspaces WHERE id = $1`,
|
||||
workspaceID,
|
||||
).Scan(&dbDir, &dbAccess); err == nil {
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -41,6 +42,11 @@ func NewMCPRateLimiter(rate int, interval time.Duration, ctx context.Context) *M
|
||||
interval: interval,
|
||||
}
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("mcp_ratelimit: PANIC in bucket cleanup: %v", r)
|
||||
}
|
||||
}()
|
||||
ticker := time.NewTicker(5 * time.Minute)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
|
||||
@@ -3,6 +3,7 @@ package middleware
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -35,6 +36,11 @@ func NewRateLimiter(rate int, interval time.Duration, ctx context.Context) *Rate
|
||||
interval: interval,
|
||||
}
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("ratelimit: PANIC in bucket cleanup: %v", r)
|
||||
}
|
||||
}()
|
||||
ticker := time.NewTicker(5 * time.Minute)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
|
||||
@@ -116,6 +116,11 @@ func sessionCachePut(key string, ok bool) {
|
||||
|
||||
func init() {
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("session_auth: PANIC in cache sweeper: %v", r)
|
||||
}
|
||||
}()
|
||||
// Jitter startup so restarts don't align sweeps.
|
||||
time.Sleep(time.Duration(rand.Int64N(int64(sessionCacheSweepEvery))))
|
||||
t := time.NewTicker(sessionCacheSweepEvery)
|
||||
|
||||
@@ -60,10 +60,12 @@ func RunWithRecover(ctx context.Context, name string, fn func(context.Context))
|
||||
}
|
||||
|
||||
// Panic → back off and restart.
|
||||
timer := time.NewTimer(backoff)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
timer.Stop()
|
||||
return
|
||||
case <-time.After(backoff):
|
||||
case <-timer.C:
|
||||
}
|
||||
if backoff < maxBackoff {
|
||||
backoff *= 2
|
||||
|
||||
Reference in New Issue
Block a user