From 56700ca8804f596e436d5f3b1b1059c1ddc1493f Mon Sep 17 00:00:00 2001 From: Rahul Sethuram Date: Fri, 21 Aug 2026 14:05:31 +0400 Subject: [PATCH] A chat you can start again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of the same missing capability: the browser could never leave a thread behind. It remembered one thread id per Bot, forever, and never asked whether Intelligence still had that thread. Where it did not — and the platform does forget, or a deployment is renamed, or an id was minted against a run that never landed — 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. The reason existed only in the server log: the vendored runtime flattens the platform's 404 into a 500 before the browser sees it, so nothing on screen could tell a person their Bot had amnesia. So OpenBot now asks the question itself. GET /api/threads/:id answers {known:boolean} through a reader that duck-types the platform's 404 and rethrows everything else, which is the whole point: a thread the platform provably does not have can be replaced without losing a conversation, while a check that failed for any other reason must not cost somebody theirs. Absent the reader the route is not registered and the browser behaves exactly as before. And the other half, which the same minting path buys: a New chat button. The key on the packaged chat now carries the thread as well as the agent, because the vendored component's own startNewThread is a no-op while threadId is a controlled prop, and without that the id changes underneath a transcript that stays on screen. --- CHANGELOG.md | 12 ++ app/src/lib/copilot/bot-thread.ts | 172 ++++++++++++++++++++++++--- app/src/routes/_authed/_app/bot.tsx | 57 ++++++++- app/tests/bot-thread.test.ts | 51 ++++++++ server/src/app.ts | 18 ++- server/src/channels/thread-routes.ts | 68 +++++++++++ server/src/channels/thread-status.ts | 37 ++++++ server/src/intelligence-client.ts | 26 ++++ server/tests/thread-routes.test.ts | 74 +++++++++++- server/tests/thread-status.test.ts | 62 ++++++++++ 10 files changed, 550 insertions(+), 27 deletions(-) create mode 100644 app/tests/bot-thread.test.ts create mode 100644 server/src/channels/thread-status.ts create mode 100644 server/src/intelligence-client.ts create mode 100644 server/tests/thread-status.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 945ff421..bfeb01df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,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. ### Fixed - **A Bot could become root inside its container.** `sudo` was granted as `NOPASSWD: ALL`, and the @@ -116,6 +119,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. ### Changed diff --git a/app/src/lib/copilot/bot-thread.ts b/app/src/lib/copilot/bot-thread.ts index d1a1366d..271a3b10 100644 --- a/app/src/lib/copilot/bot-thread.ts +++ b/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 { } /** - * `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(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 }; } diff --git a/app/src/routes/_authed/_app/bot.tsx b/app/src/routes/_authed/_app/bot.tsx index a3dd9022..620b2e96 100644 --- a/app/src/routes/_authed/_app/bot.tsx +++ b/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 (
-

Browser Bot

+
+

Browser Bot

+ {/* + * 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. + */} + +

Ask it to open a page and watch it work.

+ {/* + * 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" ? ( +

+ Earlier messages in this conversation could not be loaded, and the Bot + is answering without them. +

+ ) : 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() {

) : null}
- {/* 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 ? ( - + ) : null}
diff --git a/app/tests/bot-thread.test.ts b/app/tests/bot-thread.test.ts new file mode 100644 index 00000000..74e4b24f --- /dev/null +++ b/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"); + }); +}); diff --git a/server/src/app.ts b/server/src/app.ts index 1f949d2b..f28b5ccc 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -22,6 +22,7 @@ import type { ChannelEventHub } from "./channels/events"; import { type ChannelStore, createChannelRoutes } from "./channels/routes"; import type { ThreadIdentity } from "./channels/thread-identity"; import { createThreadRoutes } from "./channels/thread-routes"; +import { createThreadReader } from "./channels/thread-status"; import { createComponentRoutes } from "./components/routes"; import type { SandboxedStore } from "./components/sandboxed"; import { createSandboxedRoutes } from "./components/sandboxed-routes"; @@ -31,6 +32,7 @@ import type { PolicyStore } from "./computer/policy-store"; import { createComputerRoutes } from "./computer/routes"; import { configuredAuthProviders, type DeploymentConfig } from "./config"; import type { ConnectorAdminService } from "./connectors"; +import { createIntelligenceClient } from "./intelligence-client"; import type { CredentialAdminService, CredentialInput } from "./credentials"; import type { PeopleStore } from "./people/store"; import { createPluginRoutes } from "./plugins/routes"; @@ -660,7 +662,21 @@ export function createApp( } if (threadIdentity) { - app.route("/api/threads", createThreadRoutes(threadIdentity, requireUser)); + app.route( + "/api/threads", + createThreadRoutes( + threadIdentity, + requireUser, + // config.ts refuses to boot without the full Intelligence contract (see copilot.ts's + // header comment), so `config.runtime.intelligence` is never missing here. Built from it + // rather than assumed, though: this is the one place besides the runtime mount itself that + // needs to reach Intelligence, and it should keep working unmodified if that guarantee ever + // loosens and a deployment can legitimately have no reader to build. + createThreadReader( + createIntelligenceClient(config.runtime.intelligence), + ), + ), + ); } /* diff --git a/server/src/channels/thread-routes.ts b/server/src/channels/thread-routes.ts index 7a53b1a7..889374c3 100644 --- a/server/src/channels/thread-routes.ts +++ b/server/src/channels/thread-routes.ts @@ -14,9 +14,42 @@ import type { ThreadIdentity } from "./thread-identity"; * Behind the session guard because a thread id is the name of somewhere a conversation will be * stored, and there is no reason for anybody signed out to be handed one. */ + +/** + * Answers whether Intelligence still has a given thread for a given person. + * + * Two outcomes only, and deliberately not a third: `"known"` means the thread is there, `"unknown"` + * means Intelligence has clearly said it is not. A check that failed to get either answer — a + * timeout, a 5xx, a client that was never configured right — is not a value this type can hold; the + * function that implements it throws instead, so `GET /:threadId` below can tell "this thread is + * gone" from "this thread could not be checked" and answer 502 for the second rather than quietly + * reporting it as gone. + */ +export type ThreadReader = ( + threadId: string, + userId: string, +) => Promise<"known" | "unknown">; + +/** + * A UUID-shaped string, nothing more. Not the format `thread-identity.ts` mints — this route also + * has to answer for a thread a *different* deployment minted, or one minted before this deployment + * had a name, and `identity.owns` is false for both of those without either meaning the thread is + * gone. The only question this route can honestly ask is Intelligence's own; the shape check exists + * only to keep a string that could not possibly be a thread id from reaching Intelligence at all. + */ +const PLAUSIBLE_THREAD_ID = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + export function createThreadRoutes( identity: ThreadIdentity, requireUser: MiddlewareHandler<{ Variables: AppVariables }>, + /** + * Absent when this deployment has no way to ask Intelligence about a thread — no client, or a + * caller that chose not to build one. `GET /:threadId` is then not registered at all, rather than + * registered and answering 502 to everyone: a route that cannot possibly succeed should not exist. + * `POST /mint` needs no reader and is unaffected either way. + */ + readThread?: ThreadReader, ) { const routes = new Hono<{ Variables: AppVariables }>(); @@ -24,5 +57,40 @@ export function createThreadRoutes( context.json({ threadId: identity.mint() }), ); + if (readThread) { + /* + * What this buys the browser: proof, before it sends a single message, that a thread id it + * remembers from last time is one Intelligence still has. If the answer is no, it can start a + * fresh thread knowing there is provably nothing left in the old one to lose. Without this route + * that is not what happens — the browser sends anyway, Intelligence silently starts a new, empty + * thread under the remembered id, and the person is answered as though the conversation were new + * with nothing on screen to say their history is gone. + */ + routes.get("/:threadId", requireUser, async (context) => { + const threadId = context.req.param("threadId"); + if (!PLAUSIBLE_THREAD_ID.test(threadId)) { + return context.json({ error: "Not a thread id." }, 400); + } + + try { + const status = await readThread(threadId, context.var.actor.id); + return context.json({ known: status === "known" }); + } catch { + // The reader throws for everything short of a clean known/unknown answer, and what it threw + // may name an upstream host or otherwise be unfit for a browser to see, so only its kind is + // logged, not its content. The browser gets "the check failed", which is exactly as much as + // it is owed: enough to know not to trust `known: false`, nothing that was not already true + // before this request. + console.error( + JSON.stringify({ + type: "thread-status-check-failed", + note: "Could not determine whether Intelligence still has this thread.", + }), + ); + return context.json({ error: "Could not check thread status." }, 502); + } + }); + } + return routes; } diff --git a/server/src/channels/thread-status.ts b/server/src/channels/thread-status.ts new file mode 100644 index 00000000..48c628dc --- /dev/null +++ b/server/src/channels/thread-status.ts @@ -0,0 +1,37 @@ +import type { ThreadReader } from "./thread-routes"; + +/** + * Build a {@link ThreadReader} from Intelligence's own thread lookup. + * + * `getThread` is typed as narrowly as this file needs it — a method that takes a thread and a + * person and either resolves or throws — rather than as the concrete `CopilotKitIntelligence` + * client. That keeps this file from importing a vendor class just to describe one call on it, and + * means a test can hand it a stub with no client to construct. + * + * The status check is duck-typed on `error?.status === 404` rather than an `instanceof` check + * against the client's own error class, because that class is not part of `@copilotkit/runtime/v2`'s + * public surface: importing it would mean reaching past the package's exports into its internals, + * which is exactly the kind of dependency a version bump breaks without warning. The shape of a 404 + * is a much smaller promise for the vendor to keep than the identity of an internal class. + * + * Anything that is not that specific shape is rethrown unchanged, never folded into `"unknown"`. A + * 404 is Intelligence telling this deployment, in terms it can act on, that the thread is gone; a + * timeout, a 500, or a malformed response is Intelligence (or the network) failing to tell it + * anything, and treating the two the same would let a browser discard history that is only + * temporarily unreachable, not actually missing. + */ +export function createThreadReader(intelligence: { + getThread: (params: { threadId: string; userId: string }) => Promise; +}): ThreadReader { + return async (threadId, userId) => { + try { + await intelligence.getThread({ threadId, userId }); + return "known"; + } catch (error) { + if ((error as { status?: unknown } | null)?.status === 404) { + return "unknown"; + } + throw error; + } + }; +} diff --git a/server/src/intelligence-client.ts b/server/src/intelligence-client.ts new file mode 100644 index 00000000..d6e57277 --- /dev/null +++ b/server/src/intelligence-client.ts @@ -0,0 +1,26 @@ +import { CopilotKitIntelligence } from "@copilotkit/runtime/v2"; +import type { IntelligenceSettings } from "./config"; + +/** + * A client through which this deployment can ask Intelligence a question of its own. + * + * The runtime already holds one of these, built where it is mounted, and everything a conversation + * does goes through that. This is for the questions OpenBot asks on its own account rather than on + * a run's: whether a thread the browser remembers is still there, and whatever else later needs an + * answer from the platform outside a turn. + * + * A second instance rather than a shared one, deliberately. The constructor stores its settings and + * opens nothing — no socket, no handshake, no pool — so the cost is an object, while reaching into + * the runtime for the client it built would tie this to the mounting order of a module that has no + * other reason to care. The settings both read come from one place, which is the part that has to + * agree. + */ +export function createIntelligenceClient( + settings: IntelligenceSettings, +): CopilotKitIntelligence { + return new CopilotKitIntelligence({ + apiUrl: settings.apiUrl, + wsUrl: settings.gatewayWsUrl, + apiKey: settings.apiKey, + }); +} diff --git a/server/tests/thread-routes.test.ts b/server/tests/thread-routes.test.ts index ba427d30..b44f3e8b 100644 --- a/server/tests/thread-routes.test.ts +++ b/server/tests/thread-routes.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { Hono } from "hono"; import type { MiddlewareHandler } from "hono"; +import type { ThreadReader } from "../src/channels/thread-routes"; import type { AppVariables } from "../src/auth/guards"; import { createThreadIdentity } from "../src/channels/thread-identity"; import { createThreadRoutes } from "../src/channels/thread-routes"; @@ -22,8 +23,11 @@ const asSignedIn: MiddlewareHandler<{ Variables: AppVariables }> = async ( return next(); }; -function app() { - return new Hono().route("/threads", createThreadRoutes(identity, asSignedIn)); +function app(reader?: ThreadReader) { + return new Hono().route( + "/threads", + createThreadRoutes(identity, asSignedIn, reader), + ); } async function mint() { @@ -55,3 +59,69 @@ describe("minting a thread", () => { expect(response.status).toBe(401); }); }); + +describe("checking whether a remembered thread is still known upstream", () => { + test("answers known when the reader can produce the thread", async () => { + const threadId = identity.mint(); + const response = await app(async () => "known").request( + `http://openbot.local/threads/${threadId}`, + ); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ known: true }); + }); + + test("answers unknown when the reader reports Intelligence has never heard of it", async () => { + const threadId = identity.mint(); + const response = await app(async () => "unknown").request( + `http://openbot.local/threads/${threadId}`, + ); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ known: false }); + }); + + test("answers 502, not the reader's own error, when the check itself fails", async () => { + // A message this specific would leak whatever the upstream call failed with; the caller only + // ever needs to know the check could not be completed. + const reader = async () => { + throw new Error( + "intelligence-platform: connection reset by peer at 10.0.4.7:443", + ); + }; + const response = await app(reader).request( + `http://openbot.local/threads/${identity.mint()}`, + ); + expect(response.status).toBe(502); + const body = (await response.json()) as { error?: unknown }; + expect(typeof body.error).toBe("string"); + expect(body.error).not.toContain("connection reset by peer"); + expect(body.error).not.toContain("10.0.4.7"); + }); + + test("is not registered at all when the deployment has no reader, and mint keeps working", async () => { + // No reader means no way to configure one, not an outage: `POST /mint` must be unaffected by a + // route that was never wired up. + const bare = app(); + const status = await bare.request( + `http://openbot.local/threads/${identity.mint()}`, + ); + expect(status.status).toBe(404); + + const minted = await bare.request("http://openbot.local/threads/mint", { + method: "POST", + }); + expect(minted.status).toBe(200); + }); + + test("asks about the user the session established, not one smuggled in the request", async () => { + const calls: Array<{ threadId: string; userId: string }> = []; + const threadId = identity.mint(); + const reader = async (id: string, userId: string) => { + calls.push({ threadId: id, userId }); + return "known" as const; + }; + await app(reader).request( + `http://openbot.local/threads/${threadId}?userId=someone-else`, + ); + expect(calls).toEqual([{ threadId, userId: "u1" }]); + }); +}); diff --git a/server/tests/thread-status.test.ts b/server/tests/thread-status.test.ts new file mode 100644 index 00000000..e1351ccb --- /dev/null +++ b/server/tests/thread-status.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test } from "bun:test"; +import { createThreadReader } from "../src/channels/thread-status"; + +/** + * Turning Intelligence's answer about a thread into the two states a caller can act on. + * + * Intelligence has exactly one way to say "I have never heard of this thread": a 404 from + * `getThread`. Everything else it can throw — a 500, a timeout, a network failure — means the + * question could not be answered at all, and that is not the same thing. Collapsing the two would + * make an outage look identical to a thread that genuinely does not exist, and a caller that + * believed it would discard a remembered thread it should have kept. + */ + +describe("reading whether Intelligence still has a thread", () => { + test("a thread it can produce is known", async () => { + const reader = createThreadReader({ + getThread: async () => ({ id: "irrelevant" }), + }); + await expect(reader("thread-1", "user-1")).resolves.toBe("known"); + }); + + test("a 404 means the thread is unknown, not a failure", async () => { + const reader = createThreadReader({ + getThread: async () => { + throw { status: 404 }; + }, + }); + await expect(reader("thread-1", "user-1")).resolves.toBe("unknown"); + }); + + test("a 500 is not swallowed as unknown — the check itself failed", async () => { + const failure = { status: 500 }; + const reader = createThreadReader({ + getThread: async () => { + throw failure; + }, + }); + await expect(reader("thread-1", "user-1")).rejects.toBe(failure); + }); + + test("a plain Error, with no status field to duck-type on, is not swallowed either", async () => { + const failure = new Error("network unreachable"); + const reader = createThreadReader({ + getThread: async () => { + throw failure; + }, + }); + await expect(reader("thread-1", "user-1")).rejects.toBe(failure); + }); + + test("asks Intelligence about the exact thread and user it was given", async () => { + const calls: Array<{ threadId: string; userId: string }> = []; + const reader = createThreadReader({ + getThread: async (params) => { + calls.push(params); + return { id: params.threadId }; + }, + }); + await reader("thread-77", "user-99"); + expect(calls).toEqual([{ threadId: "thread-77", userId: "user-99" }]); + }); +});