test(ai-chat): cover role-pick autoStart logic + the rolePickedNoSend reset (#149 review)

Review of #156 (Request changes) flagged the new CLIENT logic as untested. Extract
the decision logic from chat-thread.tsx into pure, unit-testable helpers and cover
both branches the reviewer called out:

- `roleLaunchMessage(role, default)` — the three-way handleRolePick behavior:
  autoStart=false -> null (send nothing); autoStart=true + custom -> trimmed
  message; autoStart=true + empty/null/whitespace -> default fallback.
- `shouldResetRolePicked(chatId, roleId, flag)` — the #149 render-phase reset; the
  regression test asserts the stuck-flag case (New chat after an autoStart=false
  pick -> cards return) that the pre-fix code never handled, and that a still-bound
  role keeps the cards hidden.

chat-thread.tsx now calls these helpers (behavior unchanged). 9 new pure tests.

Also folded the review's cosmetic suggestion: `x ? x : null` -> `x || null` in
ai-agent-roles.repo.ts (identical for string|null|undefined).

Client tsc clean; role-launch + role-cards green; repo spec green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

apps/client/src/features/ai-chat/components/chat-thread.tsx

@@ -21,6 +21,10 @@ import {
   IAiChatMessageRow,
   IAiRole,
 } from "@/features/ai-chat/types/ai-chat.types.ts";
+import {
+  roleLaunchMessage,
+  shouldResetRolePicked,
+} from "@/features/ai-chat/utils/role-launch.ts";
 import { describeChatError } from "@/features/ai-chat/utils/error-message.ts";
 import { extractServerChatId } from "@/features/ai-chat/utils/adopt-chat-id.ts";
 import {
@@ -330,13 +334,14 @@ export default function ChatThread({
   const handleRolePick = (role: IAiRole): void => {
     roleIdRef.current = role.id;
     onRolePicked?.(role);
-    if (role.autoStart) {
-      // Custom launch message when set; otherwise the built-in default text.
-      sendMessage({
-        text: role.launchMessage?.trim() || t("Take a look at the current document"),
-      });
+    const launch = roleLaunchMessage(
+      role,
+      t("Take a look at the current document"),
+    );
+    if (launch !== null) {
+      sendMessage({ text: launch });
     } else {
-      // Bind only: hide the cards and show the composer, send nothing.
+      // autoStart=false -> bind only: hide the cards, show the composer.
       setRolePickedNoSend(true);
     }
   };
@@ -348,7 +353,7 @@ export default function ChatThread({
   // correctly stay hidden then. Render-phase reset (React "adjust state on prop
   // change"): one-shot — it re-renders with the flag false and the guard no longer
   // matches, so it cannot loop. (Review of #149.)
-  if (chatId === null && roleId == null && rolePickedNoSend) {
+  if (shouldResetRolePicked(chatId, roleId, rolePickedNoSend)) {
     setRolePickedNoSend(false);
   }
   const showRoleCards =

apps/client/src/features/ai-chat/utils/role-launch.test.ts

@@ -0,0 +1,72 @@
+import { describe, it, expect } from "vitest";
+import { roleLaunchMessage, shouldResetRolePicked } from "./role-launch.ts";
+
+const DEFAULT = "Take a look at the current document";
+
+// Covers the three-way handleRolePick behavior (issue #149) without mounting the
+// chat-thread component — the logic lives in these pure helpers.
+describe("roleLaunchMessage", () => {
+  it("autoStart=true + custom launchMessage -> the trimmed custom text", () => {
+    expect(
+      roleLaunchMessage(
+        { autoStart: true, launchMessage: "  Draft a plan  " },
+        DEFAULT,
+      ),
+    ).toBe("Draft a plan");
+  });
+
+  it("autoStart=true + empty launchMessage -> the default fallback", () => {
+    expect(
+      roleLaunchMessage({ autoStart: true, launchMessage: "" }, DEFAULT),
+    ).toBe(DEFAULT);
+  });
+
+  it("autoStart=true + whitespace-only launchMessage -> the default fallback", () => {
+    expect(
+      roleLaunchMessage({ autoStart: true, launchMessage: "   " }, DEFAULT),
+    ).toBe(DEFAULT);
+  });
+
+  it("autoStart=true + null launchMessage -> the default fallback", () => {
+    expect(
+      roleLaunchMessage({ autoStart: true, launchMessage: null }, DEFAULT),
+    ).toBe(DEFAULT);
+  });
+
+  it("autoStart=false -> null (bind only, send nothing) regardless of message", () => {
+    expect(
+      roleLaunchMessage(
+        { autoStart: false, launchMessage: "ignored" },
+        DEFAULT,
+      ),
+    ).toBeNull();
+    expect(
+      roleLaunchMessage({ autoStart: false, launchMessage: null }, DEFAULT),
+    ).toBeNull();
+  });
+});
+
+// Regression guard for #149: the "picked, not sent" flag must reset when the
+// user starts a fresh chat after an autoStart=false pick. On pre-fix code there
+// was no reset, so the flag stayed stuck and the role cards never returned —
+// this is exactly the `true` case below (which the old code never acted on).
+describe("shouldResetRolePicked", () => {
+  it("resets when the thread is empty and the bound role was cleared (New chat)", () => {
+    // chatId still null, roleId cleared by the parent, flag stuck -> reset.
+    expect(shouldResetRolePicked(null, null, true)).toBe(true);
+    expect(shouldResetRolePicked(null, undefined, true)).toBe(true);
+  });
+
+  it("does NOT reset while a role is still bound (cards stay hidden, composer shown)", () => {
+    // Right after the autoStart=false pick, roleId is the picked role -> keep hidden.
+    expect(shouldResetRolePicked(null, "role-1", true)).toBe(false);
+  });
+
+  it("does NOT reset once the chat exists (a message was sent / chat created)", () => {
+    expect(shouldResetRolePicked("chat-1", null, true)).toBe(false);
+  });
+
+  it("is a no-op when the flag is already false", () => {
+    expect(shouldResetRolePicked(null, null, false)).toBe(false);
+  });
+});

apps/client/src/features/ai-chat/utils/role-launch.ts

@@ -0,0 +1,34 @@
+import type { IAiRole } from "@/features/ai-chat/types/ai-chat.types.ts";
+
+/**
+ * Decide what (if anything) to auto-send when an agent role card is picked
+ * (issue #149). Extracted as a pure function so the three-way behavior is
+ * unit-testable without mounting the chat-thread component:
+ *   - autoStart=false              -> null  (bind the role only, send nothing)
+ *   - autoStart=true + message     -> the trimmed custom launchMessage
+ *   - autoStart=true + empty/null  -> the default fallback text
+ */
+export function roleLaunchMessage(
+  role: Pick<IAiRole, "autoStart" | "launchMessage">,
+  defaultText: string,
+): string | null {
+  if (!role.autoStart) return null;
+  return role.launchMessage?.trim() || defaultText;
+}
+
+/**
+ * Whether the "role picked but nothing sent yet" flag (`rolePickedNoSend`)
+ * should reset to false. After an autoStart=false pick the thread shows the
+ * composer with chatId still null; when the user then starts a fresh chat the
+ * parent clears the bound role (roleId -> null) but chatId stays null, so the
+ * thread never remounts and the flag would otherwise stay set — hiding the role
+ * cards forever. Reset exactly in that state; a still-bound role (roleId set)
+ * keeps the cards hidden. (Regression guard for #149.)
+ */
+export function shouldResetRolePicked(
+  chatId: string | null,
+  roleId: string | null | undefined,
+  rolePickedNoSend: boolean,
+): boolean {
+  return chatId === null && roleId == null && rolePickedNoSend;
+}

apps/server/src/database/repos/ai-agent-roles/ai-agent-roles.repo.ts

@@ -96,7 +96,7 @@ export class AiAgentRoleRepo {
         enabled: values.enabled ?? true,
         autoStart: values.autoStart ?? true,
         // Empty string is treated as "no custom text" => null.
-        launchMessage: values.launchMessage ? values.launchMessage : null,
+        launchMessage: values.launchMessage || null,
       })
       .returningAll()
       .executeTakeFirst();
@@ -133,7 +133,7 @@ export class AiAgentRoleRepo {
     if (patch.autoStart !== undefined) set.autoStart = patch.autoStart;
     if (patch.launchMessage !== undefined) {
       // Empty string clears to null (client default launch message).
-      set.launchMessage = patch.launchMessage ? patch.launchMessage : null;
+      set.launchMessage = patch.launchMessage || null;
     }
     await db
       .updateTable('aiAgentRoles')