fix: wire Agents page specialist chat buttons with direct session creation

Replace the fragile hash-based useEffect chain for specialist session
creation on the Agents page with a direct callback pattern. The previous
approach relied on setting window.location.hash then reacting via a
useEffect with a ref-based mutex, which was unreliable under React
StrictMode double-invocation — causing non-CMO specialist buttons to
silently fail at runtime.

The new handleSpecialistChat callback in App creates the session directly
via client.createSession({ specialistId }), sets activeSessionId and
currentSpecialistId, then navigates to #chat. This ensures the
ChatWorkspace immediately has an active session with no timing gap.

The hash-based auto-creation effect is preserved for Dashboard specialist
cards and handoff chip navigation which use window.location.hash directly.

Author: marian2js

apps/desktop/src/app/App.tsx

@@ -417,7 +417,36 @@ export function App() {
     [],
   );
 
+  // Directly create a specialist session and navigate to the chat view.
+  // This is called from the Agents page specialist buttons and avoids the
+  // fragile effect-based approach that was unreliable with React StrictMode
+  // double-invocation (the ref-based mutex could get stuck, causing non-CMO
+  // specialist buttons to silently fail at runtime).
+  const handleSpecialistChat = useCallback(
+    async (specialistId: string) => {
+      if (!client || !activeAgentId) {
+        return;
+      }
+      try {
+        const session = await client.createSession({
+          agentId: activeAgentId,
+          specialistId,
+        });
+        setSessions((prev) => [session, ...prev]);
+        setActiveSessionId(session.id);
+        setCurrentSpecialistId(specialistId);
+        window.location.hash = "#chat";
+      } catch (error) {
+        console.error("Failed to create specialist session", error);
+        toast.error("Failed to start specialist chat. Please try again.");
+      }
+    },
+    [client, activeAgentId],
+  );
+
   // Auto-create a specialist session when navigating to #chat?specialist=<id>
+  // via hash-based navigation (e.g. dashboard specialist cards, handoff chips).
+  // For the Agents page, handleSpecialistChat above is used instead.
   const specialistCreatingRef = useRef(false);
   useEffect(() => {
     if (!currentSpecialistId || !client || !activeAgentId || hashView !== "chat") {
@@ -652,6 +681,7 @@ export function App() {
             <SpecialistTeamBrowser
               client={client}
               agentId={activeAgentId}
+              onSpecialistChat={handleSpecialistChat}
             />
           )}
         </div>

apps/desktop/src/features/agents/components/SpecialistTeamBrowser.tsx

@@ -12,9 +12,10 @@ const MAX_OUTPUTS_PER_SPECIALIST = 3;
 interface SpecialistTeamBrowserProps {
   client: SidecarClient | null;
   agentId?: string | undefined;
+  onSpecialistChat?: ((specialistId: string) => void) | undefined;
 }
 
-export function SpecialistTeamBrowser({ client, agentId }: SpecialistTeamBrowserProps) {
+export function SpecialistTeamBrowser({ client, agentId, onSpecialistChat }: SpecialistTeamBrowserProps) {
   const [specialists, setSpecialists] = useState<SpecialistAgent[]>([]);
   const [isLoading, setIsLoading] = useState(true);
   const [error, setError] = useState<string | null>(null);
@@ -94,6 +95,13 @@ export function SpecialistTeamBrowser({ client, agentId }: SpecialistTeamBrowser
   }, [client, agentId]);
 
   function handleChat(specialistId: string): void {
+    if (onSpecialistChat) {
+      // Directly create session and navigate — avoids the fragile
+      // hash-based effect chain that could fail under StrictMode.
+      void onSpecialistChat(specialistId);
+      return;
+    }
+    // Fallback to hash-based navigation (used when rendered without callback)
     window.location.hash = `#chat?specialist=${encodeURIComponent(specialistId)}`;
   }
 

apps/desktop/src/features/agents/components/specialist-chat-navigation-runtime.test.ts

@@ -0,0 +1,205 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { readFileSync } from "node:fs";
+import { resolve } from "node:path";
+
+// ---------------------------------------------------------------------------
+// Source files under test
+// ---------------------------------------------------------------------------
+
+const specialistTeamBrowserSrc = readFileSync(
+  resolve(import.meta.dirname, "SpecialistTeamBrowser.tsx"),
+  "utf-8",
+);
+
+const specialistCardSrc = readFileSync(
+  resolve(import.meta.dirname, "SpecialistCard.tsx"),
+  "utf-8",
+);
+
+const appSrc = readFileSync(
+  resolve(import.meta.dirname, "../../../app/App.tsx"),
+  "utf-8",
+);
+
+// ---------------------------------------------------------------------------
+// AC: SpecialistTeamBrowser accepts onSpecialistChat callback prop
+// ---------------------------------------------------------------------------
+
+void test("SpecialistTeamBrowser interface includes onSpecialistChat prop", () => {
+  assert.ok(
+    specialistTeamBrowserSrc.includes("onSpecialistChat"),
+    "SpecialistTeamBrowserProps must include onSpecialistChat callback",
+  );
+});
+
+void test("SpecialistTeamBrowser destructures onSpecialistChat from props", () => {
+  assert.ok(
+    specialistTeamBrowserSrc.includes("onSpecialistChat }"),
+    "Component must destructure onSpecialistChat from props",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: handleChat uses direct callback when available (not hash navigation)
+// ---------------------------------------------------------------------------
+
+void test("handleChat calls onSpecialistChat when provided", () => {
+  assert.ok(
+    specialistTeamBrowserSrc.includes("onSpecialistChat(specialistId)") ||
+      specialistTeamBrowserSrc.includes("onSpecialistChat(specialistId);"),
+    "handleChat must call onSpecialistChat(specialistId) when the callback is provided",
+  );
+});
+
+void test("handleChat still has hash fallback for backward compatibility", () => {
+  assert.ok(
+    specialistTeamBrowserSrc.includes("#chat?specialist="),
+    "handleChat must retain hash-based navigation as fallback",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: App.tsx defines handleSpecialistChat that creates session directly
+// ---------------------------------------------------------------------------
+
+void test("App.tsx has handleSpecialistChat callback", () => {
+  assert.ok(
+    appSrc.includes("handleSpecialistChat"),
+    "App must define a handleSpecialistChat callback for direct session creation",
+  );
+});
+
+void test("handleSpecialistChat creates session with specialistId", () => {
+  // Find the handleSpecialistChat function body
+  const fnStart = appSrc.indexOf("handleSpecialistChat");
+  assert.ok(fnStart >= 0, "handleSpecialistChat must exist");
+
+  const fnBlock = appSrc.slice(fnStart, fnStart + 600);
+  assert.ok(
+    fnBlock.includes("createSession") && fnBlock.includes("specialistId"),
+    "handleSpecialistChat must call createSession with specialistId",
+  );
+});
+
+void test("handleSpecialistChat sets activeSessionId before navigating", () => {
+  const fnStart = appSrc.indexOf("handleSpecialistChat");
+  const fnBlock = appSrc.slice(fnStart, fnStart + 600);
+
+  const setActiveIdx = fnBlock.indexOf("setActiveSessionId");
+  const hashIdx = fnBlock.indexOf('window.location.hash = "#chat"');
+  assert.ok(setActiveIdx >= 0, "Must call setActiveSessionId");
+  assert.ok(hashIdx >= 0, "Must navigate to #chat");
+  assert.ok(
+    setActiveIdx < hashIdx,
+    "setActiveSessionId must be called before navigation to ensure session is ready",
+  );
+});
+
+void test("handleSpecialistChat sets currentSpecialistId", () => {
+  const fnStart = appSrc.indexOf("handleSpecialistChat");
+  const fnBlock = appSrc.slice(fnStart, fnStart + 600);
+
+  assert.ok(
+    fnBlock.includes("setCurrentSpecialistId"),
+    "handleSpecialistChat must set currentSpecialistId for chat specialist context",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: App.tsx passes onSpecialistChat to SpecialistTeamBrowser
+// ---------------------------------------------------------------------------
+
+void test("App.tsx passes onSpecialistChat to SpecialistTeamBrowser", () => {
+  assert.ok(
+    appSrc.includes("onSpecialistChat={handleSpecialistChat}"),
+    "App must pass onSpecialistChat={handleSpecialistChat} to SpecialistTeamBrowser",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: SpecialistCard button wiring unchanged — onChat callback for all cards
+// ---------------------------------------------------------------------------
+
+void test("SpecialistCard button onClick calls onChat(specialist.id)", () => {
+  assert.ok(
+    specialistCardSrc.includes("onClick={() => onChat(specialist.id)}"),
+    "Button must call onChat(specialist.id) on click — this is unchanged",
+  );
+});
+
+void test("SpecialistTeamBrowser passes onChat to all specialist cards", () => {
+  // Both manager and operational cards must receive onChat={handleChat}
+  const occurrences = specialistTeamBrowserSrc.match(/onChat=\{handleChat\}/g);
+  assert.ok(
+    occurrences !== null && occurrences.length >= 2,
+    "onChat={handleChat} must appear at least twice (manager + operational cards)",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: All 8 specialist IDs are valid for the navigation chain
+// ---------------------------------------------------------------------------
+
+const specialistMetaSrc = readFileSync(
+  resolve(import.meta.dirname, "../specialist-meta.ts"),
+  "utf-8",
+);
+
+const expectedSpecialists = [
+  "cmo",
+  "market-intel",
+  "positioning",
+  "website-conversion",
+  "seo-aeo",
+  "distribution",
+  "content",
+  "outbound",
+];
+
+for (const id of expectedSpecialists) {
+  void test(`specialist "${id}" exists in specialist-meta.ts`, () => {
+    assert.ok(
+      specialistMetaSrc.includes(`"${id}":`) || specialistMetaSrc.includes(`${id}:`),
+      `SPECIALIST_META must define specialist "${id}"`,
+    );
+  });
+}
+
+// ---------------------------------------------------------------------------
+// AC: handleSpecialistChat is a useCallback (avoids stale closure)
+// ---------------------------------------------------------------------------
+
+void test("handleSpecialistChat is wrapped in useCallback", () => {
+  // Check that handleSpecialistChat is defined via useCallback to prevent
+  // unnecessary re-renders and stale closures
+  const callbackIdx = appSrc.indexOf("handleSpecialistChat = useCallback");
+  assert.ok(
+    callbackIdx >= 0,
+    "handleSpecialistChat must be wrapped in useCallback",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: Error handling exists for session creation failure
+// ---------------------------------------------------------------------------
+
+void test("handleSpecialistChat has error handling with user toast", () => {
+  const fnStart = appSrc.indexOf("handleSpecialistChat");
+  const fnBlock = appSrc.slice(fnStart, fnStart + 600);
+  assert.ok(
+    fnBlock.includes("catch") && fnBlock.includes("toast.error"),
+    "handleSpecialistChat must catch errors and show toast to user",
+  );
+});
+
+// ---------------------------------------------------------------------------
+// AC: Hash-based effect still works for Dashboard and handoff chip navigation
+// ---------------------------------------------------------------------------
+
+void test("App.tsx retains the auto-creation effect for hash-based navigation", () => {
+  assert.ok(
+    appSrc.includes("specialistCreatingRef"),
+    "The hash-based auto-creation effect must still exist for Dashboard/handoff navigation",
+  );
+});