diff --git a/CHANGELOG.md b/CHANGELOG.md index d446e043..bcc6de1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A conversation started from the sidebar is recorded like one started from the home screen + +The trail had a `channel.routed` row for every conversation begun in the home composer — the +coworker it went to and why, whether inferred or named with `@` — and nothing at all for one begun +from the sidebar's +, a coworker's card or its profile, which read exactly like a row that failed +to write. Picking a coworker in that To: field is now recorded the same way an `@` is. + ### Coworkers are made in a wizard and managed in a dialog Creating a coworker is now a three-step wizard — who it is, who may see it, then where it runs, diff --git a/app/src/lib/channels/start.ts b/app/src/lib/channels/start.ts index de2c2551..4d3a6d23 100644 --- a/app/src/lib/channels/start.ts +++ b/app/src/lib/channels/start.ts @@ -3,6 +3,31 @@ import { useNavigate } from "@tanstack/react-router"; import { stashFirstMessage } from "@/components/channels/transcript-messages"; import { createChannelMutationOptions } from "./mutations"; import { channelKeys } from "./queries"; +import { routeMessage } from "./route"; + +/** + * Start a channel with a coworker the person chose themselves, and say so first. + * + * A conversation that was routed has a `channel.routed` row saying where it went and why; one whose + * coworker the person picked must have one too, or the trail reads as if the row failed to write. + * The home composer told the server about an `@` choice; a coworker picked in the To: field of + * `/channel/new` — the sidebar's +, a coworker's card, its profile — was never told to anybody. The + * two screens now share this one sequence: record, then start. + * + * The record is told before the channel exists, the way the routed path records before a channel is + * pinned, and its answer is thrown away: the person already decided and nothing here may change + * that. Failing to write the row must not stop the conversation, so a rejection is swallowed whole. + * Pure so the sequence can be tested; the hook below binds it to the real calls. + */ +export async function startWithChosen(input: { + agentId: string; + text: string; + record: (text: string, agentId: string) => Promise; + start: (agentId: string, text: string) => Promise; +}): Promise { + await input.record(input.text, input.agentId).catch(() => undefined); + await input.start(input.agentId, input.text); +} /** * Start a channel from a just-submitted first message, then navigate there. @@ -15,17 +40,22 @@ export function useStartChannel() { const navigate = useNavigate(); const createChannel = useMutation(createChannelMutationOptions(queryClient)); + const start = async (agentId: string, text: string) => { + const channel = await createChannel.mutateAsync([agentId]); + queryClient.setQueryData(channelKeys.detail(channel.id), channel); + stashFirstMessage(channel.id, text); + await navigate({ + params: { channelId: channel.id }, + replace: true, + to: "/channel/$channelId", + }); + }; + return { pending: createChannel.isPending, - start: async (agentId: string, text: string) => { - const channel = await createChannel.mutateAsync([agentId]); - queryClient.setQueryData(channelKeys.detail(channel.id), channel); - stashFirstMessage(channel.id, text); - await navigate({ - params: { channelId: channel.id }, - replace: true, - to: "/channel/$channelId", - }); - }, + start, + /** `start`, for a coworker the person chose: the choice is recorded first. */ + startChosen: (agentId: string, text: string) => + startWithChosen({ agentId, text, record: routeMessage, start }), }; } diff --git a/app/src/routes/_authed/_app/channel/new.tsx b/app/src/routes/_authed/_app/channel/new.tsx index c4ab4346..6299e51e 100644 --- a/app/src/routes/_authed/_app/channel/new.tsx +++ b/app/src/routes/_authed/_app/channel/new.tsx @@ -38,7 +38,7 @@ export const Route = createFileRoute("/_authed/_app/channel/new")({ function RouteComponent() { const { agent } = Route.useSearch(); const navigate = Route.useNavigate(); - const { start, pending } = useStartChannel(); + const { startChosen, pending } = useStartChannel(); const { data: profiles } = useQuery(agentListQueryOptions()); const [error, setError] = useState(null); @@ -135,7 +135,9 @@ function RouteComponent() { setSent(seedMessage(draft.text, newId())); try { - await start(recipient.id, draft.text); + // Recorded, then started: a coworker picked here is as much a choice as an `@` on the + // home screen, and the trail has to say so for both. + await startChosen(recipient.id, draft.text); } catch (caught) { // Preserve the unsent draft when channel creation fails. setSent(null); diff --git a/app/src/routes/_authed/_app/index.tsx b/app/src/routes/_authed/_app/index.tsx index 623d3423..bf3952a6 100644 --- a/app/src/routes/_authed/_app/index.tsx +++ b/app/src/routes/_authed/_app/index.tsx @@ -16,7 +16,7 @@ export const Route = createFileRoute("/_authed/_app/")({ function RouteComponent() { const { data: agents } = useQuery(agentListQueryOptions()); const explore = agents?.filter((a) => !a.mine && a.visibility === "public"); - const { start, pending } = useStartChannel(); + const { start, startChosen, pending } = useStartChannel(); const [error, setError] = useState(null); /** Default recipient when the composer draft has no mention. */ @@ -46,22 +46,17 @@ function RouteComponent() { // run, it falls back to the same default the composer used to always use. setError(null); try { - let agentId: string | undefined = draft.agentId ?? undefined; - if (agentId) { - /* - * Told to the server so the choice is recorded, and its answer thrown away: the - * person already decided and nothing here may change that. Failing to write the - * audit row must not stop the conversation, so a rejection is swallowed whole. - */ - await routeMessage(draft.text, agentId).catch( - () => undefined, - ); - } else { - try { - agentId = (await routeMessage(draft.text)).agentId; - } catch { - agentId = fallback?.id; - } + if (draft.agentId) { + // Recorded and started as one sequence, shared with `/channel/new`: the person + // already decided, and the trail has to say so wherever they decided it. + await startChosen(draft.agentId, draft.text); + return; + } + let agentId: string | undefined; + try { + agentId = (await routeMessage(draft.text)).agentId; + } catch { + agentId = fallback?.id; } if (!agentId) return; await start(agentId, draft.text); diff --git a/app/tests/start-chosen.test.ts b/app/tests/start-chosen.test.ts new file mode 100644 index 00000000..a7304399 --- /dev/null +++ b/app/tests/start-chosen.test.ts @@ -0,0 +1,85 @@ +import { expect, test } from "bun:test"; +import { startWithChosen } from "../src/lib/channels/start"; + +/** + * A conversation whose coworker the person picked themselves has to reach the trail the same way a + * routed one does. `/channel/new` — the sidebar's +, a coworker's card, its profile — started the + * channel and told nobody; the home composer told the server about an `@`. Both now run this one + * sequence, so what it does is what the trail sees. + */ + +function harness(record: (text: string, agentId: string) => Promise) { + const calls: string[] = []; + return { + calls, + record: (text: string, agentId: string) => { + calls.push(`record ${agentId} ${text}`); + return record(text, agentId); + }, + start: async (agentId: string, text: string) => { + calls.push(`start ${agentId} ${text}`); + }, + }; +} + +test("tells the server the choice before the channel is made", async () => { + const { calls, record, start } = harness(async () => ({ + agentId: "risk-analyst", + })); + + await startWithChosen({ + agentId: "risk-analyst", + text: "hello", + record, + start, + }); + + expect(calls).toEqual([ + "record risk-analyst hello", + "start risk-analyst hello", + ]); +}); + +test("a record that fails to write does not stop the conversation", async () => { + const { calls, record, start } = harness(async () => { + throw new Error("Could not choose a coworker."); + }); + + await startWithChosen({ + agentId: "risk-analyst", + text: "hello", + record, + start, + }); + + expect(calls).toEqual([ + "record risk-analyst hello", + "start risk-analyst hello", + ]); +}); + +test("the server's answer cannot change who the person chose", async () => { + const { calls, record, start } = harness(async () => ({ + agentId: "somebody-else", + })); + + await startWithChosen({ + agentId: "risk-analyst", + text: "hello", + record, + start, + }); + + expect(calls[1]).toBe("start risk-analyst hello"); +}); + +test("a channel that cannot be started still fails the send", async () => { + const { record } = harness(async () => undefined); + const start = async () => { + throw new Error("Could not start a channel"); + }; + + await expect( + startWithChosen({ agentId: "risk-analyst", text: "hello", record, start }), + ).rejects.toThrow("Could not start a channel"); +});