From 5a9ce5496a6836356c736f00978cd6a1fcbf03ee Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Mon, 31 Aug 2026 20:37:44 +0900 Subject: [PATCH] Name the Bot on a hop that was refused, retried or lost, not only on one that landed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Audit screen's Bot column reads `payload.bot` and nothing else, and renders a dash without it. `agent.handoff_offered` says so in its own comment, added when the accepted pair was fixed: without this the two handoff rows are the only Bot actions on a screen headed "Every action a Bot took" that name no Bot. The pair got the key. The four rows either side of it did not: `agent.handoff_refused`, both `agent.handoff_retried` rows, and `agent.handoff_failed` carry `from` and no `bot`, so every one of them draws a dash. Those are the rows the trail exists for. `server/src/audit.ts` says it outright of the first one — "a hop that happened is visible in the transcript anyway; a hop that was refused is invisible everywhere else" — and the same holds for one that ran out of attempts, which is where a person's unanswered question ends. So the screen named the Bot on the outcome you could already see, and drew a dash on the three you could not. `agent.escalated` and `agent.escalation_failed` one file over share a single payload and get this right on both outcomes, which is the shape being restored here. Pairs with #302, which stops that same screen calling `agent.handoff_refused` "Allowed". Together a refused hop reads as a refusal and says whose it was; either alone leaves half the row wrong. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 9 +++++ server/src/agents/handoff-runner.ts | 9 +++++ server/src/agents/handoff.ts | 6 +++ server/tests/agent-handoff-runner.test.ts | 34 ++++++++++++++++ server/tests/agent-handoff.test.ts | 49 +++++++++++++++++++++++ 5 files changed, 107 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c851fcd32..e57e807d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ 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 Bot's shell can no longer reach the embedded database without a password In the all-in-one image the cluster was `trust`-auth on loopback, and the Bot's shell runs in the diff --git a/server/src/agents/handoff-runner.ts b/server/src/agents/handoff-runner.ts index 1950e3223..8ec81631b 100644 --- a/server/src/agents/handoff-runner.ts +++ b/server/src/agents/handoff-runner.ts @@ -265,6 +265,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, @@ -339,6 +342,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, @@ -405,6 +410,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 0e23818ce..58c32848f 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 fa7c726e1..9d299ff71 100644 --- a/server/tests/agent-handoff-runner.test.ts +++ b/server/tests/agent-handoff-runner.test.ts @@ -196,6 +196,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 da3abae6c..748e232b7 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" }); + } + }); }); /*