Conversation
`handoff.ts` states the rule for both of these tools at the top of its module: every refusal is an
answer, not an error, because the asking Bot is mid-run with a person waiting and a thrown error
ends the run with nothing said. `escalation.ts` competes with it for the same decision and did not
follow it.
`EscalationRoute` returns `{ reached }` or `{ refusal }`, and both are handled. A route that throws
was not. The error came straight back out of `execute`, so the run ended with nothing said to the
person waiting — the exact failure the sibling module names, on the one tool whose whole job is to
stop a Bot falling silent — and the audit row never went down, despite the comment beside it
promising that an escalation which could not be delivered is recorded because it is the one worth
finding later.
It looks unreachable and is not. `askTheirOwnPerson` is a pure function and cannot throw, so nothing
in this repo or its tests has ever taken this path. But the module comment says WHO "A PERSON" IS,
IS A SEAM, and names what a company puts there: an on-call rota, a duty desk, a queue somebody works
through in the morning. Every one of those is a network call that can time out, 502, or resolve to
nothing. The only route that cannot fail is the one that ships, so the guard was missing precisely
where the documentation invites a deployment to go.
The route's own words go on the row and not into the answer. What a rota throws is written for
whoever operates it — a connection reset, a status line, an internal hostname — and the tool's return
value is paraphrased by the model to the person who asked. That is the split `handoff-runner.ts`
already makes, and the split `store.ts` makes between `reason` and `failure` in an audit payload, so
the payload uses those same two names.
The sentence the Bot gets tells it not to claim a person has been asked. Without that it has just
been handed a refusal where it is used to being handed `PUT_TO`, and the likely next thing it says
is that somebody is looking into it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Two things I want on the record here rather than found in review. What I measured, and what I took from the repoThe PR body says a throw "ended the run with nothing said to the person waiting". I should be precise about which half of that I verified. Verified, and what the tests assert: Not verified: what the runtime then does with that throw. What I relied on instead is this repo's own stated invariant —
So the argument is "this tool does not follow a rule the codebase states for tools of exactly this kind", which stands on its own. If the runtime does in fact catch and surface tool throws, the missing audit row is still a bug and the fix is still right, but the user-visible half of the motivation is weaker than I wrote it — worth knowing before you weigh it. If somebody with the dependency installed can confirm either way I will amend the changelog wording. The sibling has the same shape, and I left it alone deliberately
const outcome = await desk.send({ ... });with the same comment beneath it saying a refusal is a sentence and not an exception. And I did not fix it in this PR, for a reason I would rather state than have you assume I missed it: The clean fix is not in the tool. Happy to send it as its own PR if you tell me which way that should go. |
What this changes
handoff.tsstates the rule for both of these tools at the top of its module:escalation.tscompetes with that tool for the same decision and did not follow it.EscalationRoutereturns{ reached }or{ refusal }, andexecutehandles both. A route that throws was not handled at all — the error came straight back out ofexecute. So the run ended with nothing said to the person waiting, on the one tool whose entire job is to stop a Bot falling silent, and no audit row went down, despite the comment immediately below promising the opposite:That comment described the two returned shapes. The delivery failure it names as the one worth finding is the one that got no row.
Why this is reachable, not theoretical
It looks unreachable, and that is the interesting part.
askTheirOwnPersonis a pure function that cannot throw, so nothing in this repo or its tests has ever taken this path.But the module's own comment says where deployments go instead:
Every one of those is a network call: a timeout, a 502, a hostname that stops resolving. The only route that cannot fail is the one that ships, so the guard was missing exactly where the documentation invites a deployment to go, and no local run or test could have shown it.
Where it runs
try/catchand a local string.store.tsdoes with a vendor's words.Boundary and audit
This tool is deliberately not gated on a grant, and that is unchanged — nothing here widens what a Bot may do. What changes is what the trail says when the route fails.
agent.escalation_failed, the type already declared for this, rather than nothing.handoff-runner.tskeeps those apart for the same reason. The payload reusesstore.ts's two names:reasonis the sentence the Bot was given,failureis what actually went wrong.PUT_TOhere, and handing it a bare refusal invites it to say somebody is looking into it — which would be a false statement to a person, produced by the fix. So the sentence says plainly that nobody could be asked and that it must not say otherwise.Errorthrows are handled (String(error)), so the handler written to keep the run alive cannot itself be the thing that ends it.Changelog
Unreleased, noting that deployments on the shipped in-conversation route are unaffected because it cannot fail.Proof
Three tests in
server/tests/agent-escalation.test.ts. Onmain:On this branch:
They assert the three separate properties, not one: that
executereturns rather than throws, that the answer contains neitherPUT_TOnor the internal hostname while the row carries the full thrown text, and that a thrown string behaves like a thrownError.cd server && bunx tsc --noEmitreports only three pre-existing errors on my machine —cron-parser,@ag-ui/mastra,@mastra/client-jsare declared inpackage.jsonbut missing from mynode_modules. They are identical on unmodifiedmainand none is in a file this touches. Biome check on the three changed files: clean.What is not covered
catch, and the honest place for it is the route contract, not this handler — worth its own issue if anyone wants it.auditStoreis optional on this tool, so a caller that omits it still gets no row on any path.server/src/index.ts:934passes it, so every route in the product is covered; making the parameter required is a wider change than this one.🤖 Generated with Claude Code