diff --git a/CHANGELOG.md b/CHANGELOG.md index cd564de4..b4876146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A hop the boundary refused now names the Bot that was refused + +The audit page renders its Bot column from `payload.bot` and nothing else. `agent.handoff_offered` +and `agent.handoff_delivered` were given that key; the four rows either side of them — a hop +refused, a hop retried, and a hop that failed for good — were not, so they showed a dash where the +Bot belongs. Those are the rows somebody actually opens the trail for: a hop that happened is visible +in the transcript anyway, and a refused or lost one is visible nowhere else. All four now name the +asking Bot, exactly as the accepted pair and `agent.escalated` already did. ### A failed tool refresh no longer leaves a connector offering nothing Refreshing a connector's tools replaced the list with a delete and then an insert, as two separate diff --git a/server/src/agents/handoff-runner.ts b/server/src/agents/handoff-runner.ts index 5b7ab8db..c59a3bc7 100644 --- a/server/src/agents/handoff-runner.ts +++ b/server/src/agents/handoff-runner.ts @@ -300,6 +300,9 @@ export function createHandoffRunner(options: { targetId: work.toBotId, ...(work.actorId ? { actorUserId: work.actorId } : {}), payload: { + // See the same key on `agent.handoff_delivered` below: the Audit screen's Bot + // column reads `payload.bot`, so a row without it names no Bot. + bot: work.fromBotId, from: work.fromBotId, to: work.toBotId, run: work.runId, @@ -374,6 +377,8 @@ export function createHandoffRunner(options: { targetId: work.toBotId, ...(work.actorId ? { actorUserId: work.actorId } : {}), payload: { + // See the same key on `agent.handoff_delivered` below. + bot: work.fromBotId, from: work.fromBotId, to: work.toBotId, run: work.runId, @@ -456,6 +461,10 @@ export function createHandoffRunner(options: { targetId: work.toBotId, ...(work.actorId ? { actorUserId: work.actorId } : {}), payload: { + // See the same key on `agent.handoff_delivered` above. This row is the one a + // person's unanswered question ends on, so a Bot column showing a dash on it is + // the worst place in the set to have one. + bot: work.fromBotId, from: work.fromBotId, to: work.toBotId, run: work.runId, diff --git a/server/src/agents/handoff.ts b/server/src/agents/handoff.ts index 0e23818c..58c32848 100644 --- a/server/src/agents/handoff.ts +++ b/server/src/agents/handoff.ts @@ -110,6 +110,12 @@ export function createHandoffDesk(options: { targetId: from.botId, ...(from.actorId ? { actorUserId: from.actorId } : {}), payload: { + // The same key `agent.handoff_offered` sets below, and for the same reason: the Audit + // screen renders `payload.bot` and nothing else in its Bot column, so a row without it + // shows a dash. The accepted row was given this and its refusal was not, which left the + // refusal — the one the trail says matters more, because a hop that happened is visible in + // the transcript and a refused one is invisible everywhere else — naming no Bot at all. + bot: from.botId, from: from.botId, // As the model named it, capped: untrusted input, kept because "who did it reach for" is the // useful half of the question. diff --git a/server/tests/agent-handoff-runner.test.ts b/server/tests/agent-handoff-runner.test.ts index 61c5c56e..7f8a8f1f 100644 --- a/server/tests/agent-handoff-runner.test.ts +++ b/server/tests/agent-handoff-runner.test.ts @@ -199,6 +199,40 @@ describe("delivering a hop", () => { }); }); + /* + * And so do the rows either side of it, which is the half the assertion above did not reach. + * + * A delivery is the outcome that is also visible in the transcript. A hop that was retried or + * that failed is visible nowhere else at all, so those are the rows somebody actually comes to + * this screen for — and they were the ones rendering a dash where the Bot's name belongs. + */ + test("a hop that was retried or that failed names the Bot too", async () => { + const retried = runner({ + claimed: [ + { kind: "bot.message", key: "run-1:abc", payload: WORK, attempts: 2 }, + ], + }); + await retried.runner.sweep(); + + expect( + retried.written.find( + (event) => event.eventType === "agent.handoff_retried", + )?.payload, + ).toMatchObject({ bot: WORK.fromBotId, from: WORK.fromBotId }); + + const failed = runner({ + deliver: async () => { + throw new Error("the gateway was unreachable"); + }, + }); + await failed.runner.sweep(); + + expect( + failed.written.find((event) => event.eventType === "agent.handoff_failed") + ?.payload, + ).toMatchObject({ bot: WORK.fromBotId, from: WORK.fromBotId }); + }); + /* Releasing an unusable row would put it back on the queue for ever. */ test("a row that is not a hop is finished rather than released", async () => { const { runner: sweep, calls } = runner({ diff --git a/server/tests/agent-handoff.test.ts b/server/tests/agent-handoff.test.ts index da3abae6..748e232b 100644 --- a/server/tests/agent-handoff.test.ts +++ b/server/tests/agent-handoff.test.ts @@ -338,8 +338,57 @@ describe("handing work to another Bot", () => { expect(refused.events[0]?.payload).toMatchObject({ reason: "not_granted", run: "run-1", + /* + * And the refusal names the Bot too, which is the half this was missing. + * + * The accepted row above was given `bot` and its refusal was not, so the pair the trail calls + * "both outcomes" rendered one Bot and one dash. On the row the notes call the more important + * of the two: a hop that happened is visible in the transcript, and a refused one is + * invisible everywhere except here. + */ + bot: "assistant", }); }); + + /* + * Every way a hop can be refused, not only the one the pair above happens to use. + * + * `refuse` is one function and all five reasons go through it, so this could not drift per reason + * — but that is the argument for asserting it once across all of them rather than trusting it. + */ + test("every refusal names the Bot that was refused", async () => { + const cases: Array<[string, ReturnType]> = [ + ["no_task", desk()], + ["not_granted", desk({ granted: false })], + ["unknown_bot", desk()], + ["depth", desk({ caps: { maxDepth: 0, maxPerRun: 3 } })], + ["fan_out", desk({ caps: { maxDepth: 2, maxPerRun: 0 } })], + ]; + const envelopes: Record = { + no_task: { target: "researcher", task: "" }, + not_granted: { target: "researcher", task: "t" }, + unknown_bot: { target: "nobody-by-that-name", task: "t" }, + depth: { target: "researcher", task: "t" }, + fan_out: { target: "researcher", task: "t" }, + }; + + for (const [name, harness] of cases) { + const envelope = envelopes[name] as { target: string; task: string }; + const outcome = await harness.desk.send({ + from: FROM, + target: envelope.target, + envelope: { task: envelope.task }, + }); + + expect(outcome.ok).toBe(false); + expect(harness.events.map((event) => event.eventType)).toEqual([ + "agent.handoff_refused", + ]); + // The asking Bot, the same one `agent.handoff_offered` records, so the two rows of a pair + // read as one Bot's two possible outcomes rather than as one Bot and a dash. + expect(harness.events[0]?.payload).toMatchObject({ bot: "assistant" }); + } + }); }); /*