A chat you can start again

CHANGELOG.md

@@ -82,6 +82,9 @@ Sessions survive and nobody signs in again.
   path, rather than reporting an element it was never about.
 - **`COMPUTER_SANDBOX=on`** turns on Chromium's own sandbox where the host permits user namespaces.
   Which way it went is printed at start-up either way.
+- **New chat.** The direct Bot chat has a button that starts a fresh conversation, which it had no way
+  to do before: the thread was minted once and remembered for that Bot forever, so the only way out
+  of a conversation was to clear the browser's storage by hand.
 - **You can watch what a Bot is doing, not only what it is looking at.** The screen answered half the
   question: a Bot spending two minutes in a terminal showed a blank browser and one grey line per
   command, with the output nowhere. A command line in the transcript now opens to show what it
@@ -221,6 +224,15 @@ Sessions survive and nobody signs in again.
   laptop `http://localhost` counts as one, so this never showed up in development; on a real
   address it does not, and the surface did nothing at all when you pressed send. No message, no
   error. Ids now come from an API with no such restriction.
+- **A chat could quietly forget everything and carry on.** The browser remembers a thread id for each
+  Bot, and nothing ever asked whether Intelligence still had that thread. Where it did not, the
+  transcript loaded empty, every later message silently recreated an empty thread under the same id,
+  and the Bot answered as though the conversation were new — with the reason nowhere but the server
+  log, as a 404 flattened into a 500 by the time it reached the browser. A remembered thread is now
+  checked before it is used: one the platform provably does not have is replaced, because there is no
+  conversation left to lose, and a check that fails for any other reason keeps the thread and says on
+  screen that earlier messages could not be loaded. A person reading a confident answer can now tell
+  whether the Bot has read what came before it.
 - **The first browser action a Bot was ever asked for failed.** Creating a computer and starting it
   are two calls to Docker, and a name the daemon has not published yet answers the second with a 404.
   The supervisor treats that as a lost race and rebuilds, which is right, but it went straight back

app/src/lib/copilot/bot-thread.ts

@@ -1,4 +1,4 @@
-import { useEffect, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { tryClient } from "@/lib/client";
 import { newId } from "../new-id";
 
@@ -11,13 +11,26 @@ import { newId } from "../new-id";
  *
  * Kept per Bot: the chat is bound to one Bot at a time, and two Bots sharing a thread would read
  * each other's conversation.
+ *
+ * "Tomorrow" is a promise this module cannot keep on its own, though. The id survives in
+ * `localStorage` forever, but Intelligence is free to forget the thread behind it — expiry,
+ * environment reset, a wiped dev database. Remembering an id nobody upstream recognises does not
+ * fail loudly: every send just opens a new, empty thread under the old id, and the chat answers as
+ * if nothing had ever been said, with no error on screen to explain why. So before this hook hands
+ * out a remembered id, it asks Intelligence whether that id still means something, and only keeps
+ * pretending the history is there when the answer says it is.
  */
 
 const KEY = "openbot.bot-thread";
 
+/** The one place the storage key is built, so the getter and the setter can never drift apart. */
+export function botThreadKey(agentId: string): string {
+  return `${KEY}.${agentId}`;
+}
+
 function remembered(agentId: string): string | null {
   try {
-    return window.localStorage.getItem(`${KEY}.${agentId}`);
+    return window.localStorage.getItem(botThreadKey(agentId));
   } catch {
     // Storage can be unavailable or full. A thread for this visit is better than no chat at all.
     return null;
@@ -26,7 +39,7 @@ function remembered(agentId: string): string | null {
 
 function remember(agentId: string, threadId: string): void {
   try {
-    window.localStorage.setItem(`${KEY}.${agentId}`, threadId);
+    window.localStorage.setItem(botThreadKey(agentId), threadId);
   } catch {
     // As above: the conversation still works, it just will not be here next time.
   }
@@ -44,35 +57,156 @@ async function mint(): Promise<string | null> {
 }
 
 /**
- * `undefined` until it is known, which is not the same as absent: rendering the chat before then
- * would let it mint an id of its own, and that is the one this deployment would then be stuck with.
+ * Whether Intelligence still has a remembered thread, and whether the question could even be put
+ * to it. Those are separate facts: a 404 on this route means no reader is configured on this
+ * deployment at all, which says nothing about the thread and must not be read as bad news, while a
+ * 400 or a 502 (or the request simply throwing) means the question was asked and failed to get a
+ * clean answer.
+ */
+async function checkKnown(
+  threadId: string,
+): Promise<{ known: boolean | undefined; unavailable: boolean }> {
+  try {
+    const response = await tryClient(
+      `/api/threads/${encodeURIComponent(threadId)}`,
+    );
+    if (response.status === 404) {
+      // The route itself is absent, which is what an unconfigured reader looks like server-side.
+      // Behave exactly as this hook did before the check existed: nothing was learned, nothing
+      // changes.
+      return { known: undefined, unavailable: false };
+    }
+    if (!response.ok) {
+      // A 400 means the remembered id was not even a plausible thread id; a 502 means the check
+      // itself failed to reach Intelligence. Either way there is no clean answer, and the safer
+      // read is "unknown" rather than "gone" — see threadToUse for why.
+      return { known: undefined, unavailable: true };
+    }
+    const body = (await response.json()) as { known?: unknown };
+    if (typeof body.known !== "boolean") {
+      // A 200 that does not carry the shape this hook asked for is not an answer either.
+      return { known: undefined, unavailable: true };
+    }
+    return { known: body.known, unavailable: false };
+  } catch {
+    return { known: undefined, unavailable: true };
+  }
+}
+
+/**
+ * Whether to keep the remembered thread id or start over, given what the check above learned.
+ *
+ * The two ways of not keeping it are not the same mistake. `known: false` is Intelligence saying,
+ * plainly, that it has never heard of this id — replacing it loses nothing, because there was
+ * never anything there to lose, and keeping it would only mean sending every message into a thread
+ * that quietly reopens empty. `known: undefined` is the opposite kind of ignorance: the check
+ * failed to get an answer at all, and discarding somebody's conversation on the strength of a
+ * network blip is the worse mistake of the two. So only a provable "no" moves this to `"fresh"`;
+ * an inconclusive check keeps what was remembered. No remembered id to keep is its own case: with
+ * nothing to protect, there is nothing the check could have told us that would change the outcome.
+ */
+export function threadToUse(input: {
+  remembered: string | null;
+  known: boolean | undefined;
+}): "remembered" | "fresh" {
+  if (input.remembered === null) return "fresh";
+  return input.known === false ? "fresh" : "remembered";
+}
+
+export type BotThread = {
+  /** `undefined` until it is known, which is not the same as absent: rendering the chat before
+   * then would let it mint an id of its own, and that is the one this deployment would then be
+   * stuck with. */
+  threadId: string | undefined;
+  /**
+   * `"unavailable"` means the check that guards a remembered thread failed to get a clean answer,
+   * so this render is going ahead on a thread id that history may or may not still exist for and
+   * nobody can currently say which. `"ready"` covers every case where that question was either
+   * answered or never needed asking.
+   */
+  history: "ready" | "unavailable";
+  /** Mints a fresh thread and starts using it from now on, independent of whatever was
+   * remembered. */
+  startNew: () => void;
+};
+
+/**
+ * The thread the direct Bot chat talks in, resolved once per `agentId` and re-verified on every
+ * mount rather than trusted forever — see the module doc for why a remembered id can go stale.
  */
-export function useBotThread(agentId: string): string | undefined {
+export function useBotThread(agentId: string): BotThread {
   const [threadId, setThreadId] = useState<string | undefined>(undefined);
+  const [history, setHistory] = useState<"ready" | "unavailable">("ready");
+  // Mirrors the effect's own `current` flag so `startNew` — which runs from an event handler, not
+  // from the effect — can tell whether it is still safe to touch state: the agent may have changed
+  // or the component may have unmounted while its mint was in flight.
+  const mountedRef = useRef(true);
+  // Shared between the effect's own resolution mint and `startNew`'s mint so the two can never
+  // race each other into two POST /mint calls fighting over the same localStorage slot.
+  const mintingRef = useRef(false);
 
   useEffect(() => {
     let current = true;
+    mountedRef.current = true;
     setThreadId(undefined);
+    setHistory("ready");
+
+    const adoptFresh = () => {
+      mintingRef.current = true;
+      void mint().then((minted) => {
+        mintingRef.current = false;
+        if (!current) return;
+        // Falling back to one made here keeps the chat working when the deployment cannot be
+        // asked; it is simply a thread nothing can later attribute.
+        const next = minted ?? newId();
+        if (minted) remember(agentId, minted);
+        setThreadId(next);
+        setHistory("ready");
+      });
+    };
 
     const existing = remembered(agentId);
-    if (existing) {
-      setThreadId(existing);
-      return;
+    if (!existing) {
+      // Nothing remembered means nothing the check could protect — minting straight away is both
+      // faster and exactly as safe as asking Intelligence about an id that was never assigned.
+      adoptFresh();
+    } else {
+      void checkKnown(existing).then((outcome) => {
+        if (!current) return;
+        const decision = threadToUse({
+          remembered: existing,
+          known: outcome.known,
+        });
+        if (decision === "remembered") {
+          setThreadId(existing);
+          setHistory(outcome.unavailable ? "unavailable" : "ready");
+        } else {
+          adoptFresh();
+        }
+      });
     }
 
-    void mint().then((minted) => {
-      if (!current) return;
-      // Falling back to one made here keeps the chat working when the deployment cannot be asked;
-      // it is simply a thread nothing can later attribute.
-      const next = minted ?? newId();
-      if (minted) remember(agentId, minted);
-      setThreadId(next);
-    });
-
     return () => {
       current = false;
+      mountedRef.current = false;
     };
   }, [agentId]);
 
-  return threadId;
+  const startNew = useCallback(() => {
+    if (mintingRef.current) return;
+    mintingRef.current = true;
+    void mint().then((minted) => {
+      mintingRef.current = false;
+      if (!mountedRef.current) return;
+      // Leaving the current thread alone here matters: a person pressing "New chat" should never
+      // end up worse off — mid-conversation and suddenly unable to send — than before they pressed
+      // it.
+      if (!minted) return;
+      remember(agentId, minted);
+      setThreadId(minted);
+      setHistory("ready");
+    });
+  }, [agentId]);
+
+  return { threadId, history, startNew };
 }

app/src/routes/_authed/_app/bot.tsx

@@ -1,5 +1,7 @@
 import { CopilotChat } from "@copilotkit/react-core/v2";
+import { IconPlus } from "@tabler/icons-react";
 import { createFileRoute } from "@tanstack/react-router";
+import { Button } from "@/components/ui/button";
 import { useActiveBot } from "@/lib/copilot/active-bot";
 import { useBotThread } from "@/lib/copilot/bot-thread";
 import { useStoppedTurn } from "@/lib/copilot/stopped-turn";
@@ -17,8 +19,14 @@ function RouteComponent() {
 
   // Tool calls here act on this Bot's own computer.
   useActiveBot(agentId);
-  // Minted by this deployment rather than by the chat, and the same one on the next visit.
-  const threadId = useBotThread(agentId);
+  /*
+   * Minted by this deployment rather than by the chat. `history` reports whether Intelligence
+   * still recognised the thread this browser remembered from a previous visit; when it did not,
+   * `useBotThread` has already swapped in a fresh id on its own; this flag exists only so the
+   * page can say so instead of letting the Bot answer as if nothing were missing. `startNew`
+   * mints another fresh thread on demand for the New chat control below.
+   */
+  const { threadId, history, startNew } = useBotThread(agentId);
   /*
    * A turn that ends without an answer has to be said out loud here, because the packaged chat says
    * nothing. It reports a failed run to an `onError` prop and otherwise carries on as though the
@@ -31,11 +39,38 @@ function RouteComponent() {
   return (
     <div className="flex h-screen flex-col">
       <header className="border-b px-6 py-3">
-        <h1 className="text-lg font-semibold">Browser Bot</h1>
+        <div className="flex items-baseline justify-between">
+          <h1 className="text-lg font-semibold">Browser Bot</h1>
+          {/*
+           * Labelled rather than the bare icon button the sidebar uses for its own "start
+           * something new" control: that one opens an empty screen, but this one throws away
+           * whatever conversation is currently on screen, and a click with that consequence
+           * deserves a word, not just a glyph.
+           */}
+          <Button onClick={startNew} size="sm" variant="ghost">
+            <IconPlus />
+            New chat
+          </Button>
+        </div>
         <p className="text-sm text-muted-foreground">
           Ask it to open a page and watch it work.
         </p>
       </header>
+      {/*
+       * Both banners render as plain siblings in this fixed order, never one nested inside the
+       * other, so either can appear alone or both together without the layout jumping around
+       * depending on which conditions are true.
+       */}
+      {history === "unavailable" ? (
+        <p
+          className="border-b bg-destructive/10 px-6 py-2 text-destructive text-sm"
+          data-testid="bot-chat-history-unavailable"
+          role="alert"
+        >
+          Earlier messages in this conversation could not be loaded, and the Bot
+          is answering without them.
+        </p>
+      ) : null}
       {/*
        * Under the header rather than at the end of the transcript, which is where the missing answer
        * was going to be and where the channel draws its own version of this. The packaged chat owns
@@ -54,9 +89,21 @@ function RouteComponent() {
         </p>
       ) : null}
       <div className="min-h-0 flex-1">
-        {/* Remount when switching Bots so chat state stays bound to the selected agent. */}
+        {/*
+         * Keyed on the thread as well as the agent. Switching agents was already handled by
+         * `agentId`, but `startNew` changes only the thread while the agent stays put, and the
+         * packaged chat's own `startNewThread`/`setActiveThreadId` are proven no-ops once
+         * `threadId` is a controlled prop (node_modules/@copilotkit/react-core/dist/copilotkit-
+         * C4RqjAba.mjs:226-254): asking it to start over does nothing while it still holds the
+         * old id. A key that omits the thread would leave the previous conversation on screen
+         * under a composer that silently posts to the new one.
+         */}
         {threadId ? (
-          <CopilotChat agentId={agentId} key={agentId} threadId={threadId} />
+          <CopilotChat
+            agentId={agentId}
+            key={`${agentId}:${threadId}`}
+            threadId={threadId}
+          />
         ) : null}
       </div>
     </div>

app/tests/bot-thread.test.ts

@@ -0,0 +1,51 @@
+import { describe, expect, test } from "bun:test";
+import { botThreadKey, threadToUse } from "../src/lib/copilot/bot-thread";
+
+/**
+ * The two decisions `useBotThread` makes that do not need a browser to test: which localStorage
+ * key belongs to which Bot, and whether a remembered thread id is still safe to use once
+ * Intelligence has said whether it knows about it.
+ */
+
+describe("botThreadKey", () => {
+  test("is the same key for the same agent every time", () => {
+    expect(botThreadKey("bot-a")).toBe(botThreadKey("bot-a"));
+  });
+
+  test("is namespaced rather than the bare agent id", () => {
+    // A raw agent id used directly as a localStorage key would collide with anything else in the
+    // app that happens to key storage by agent id.
+    const key = botThreadKey("bot-a");
+    expect(key).not.toBe("bot-a");
+    expect(key).toContain("bot-a");
+  });
+
+  test("two agents never collide", () => {
+    expect(botThreadKey("bot-a")).not.toBe(botThreadKey("bot-b"));
+  });
+});
+
+describe("threadToUse", () => {
+  test("a remembered thread Intelligence confirms it has is kept", () => {
+    expect(threadToUse({ remembered: "t1", known: true })).toBe("remembered");
+  });
+
+  test("a remembered thread Intelligence says it does not have is replaced", () => {
+    expect(threadToUse({ remembered: "t1", known: false })).toBe("fresh");
+  });
+
+  test("a remembered thread is kept when the check itself could not be completed", () => {
+    // known: undefined means the lookup failed, not that the thread is gone. Discarding a
+    // perfectly good thread id because Intelligence was briefly unreachable would be worse than
+    // the failure it was reacting to.
+    expect(threadToUse({ remembered: "t1", known: undefined })).toBe(
+      "remembered",
+    );
+  });
+
+  test("with nothing remembered, every outcome of the check starts fresh", () => {
+    expect(threadToUse({ remembered: null, known: true })).toBe("fresh");
+    expect(threadToUse({ remembered: null, known: false })).toBe("fresh");
+    expect(threadToUse({ remembered: null, known: undefined })).toBe("fresh");
+  });
+});