diff --git a/CHANGELOG.md b/CHANGELOG.md index 21800a08..ec1416fb 100644 --- a/CHANGELOG.md +++ b/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 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 1c0516b4..0ccb1d4f 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -24,6 +24,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"; @@ -33,6 +34,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"; @@ -773,7 +775,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" }]); + }); +});