Skip to content

Keep the run alive when the route to a person is the thing that failed - #541

Open
Hotragn wants to merge 1 commit into
CopilotKit:mainfrom
Hotragn:ask-a-person-that-could-not-be-delivered
Open

Hotragn wants to merge 1 commit into
CopilotKit:mainfrom
Hotragn:ask-a-person-that-could-not-be-delivered

Conversation

@Hotragn

@Hotragn Hotragn commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What this changes

handoff.ts states the rule for both of these tools at the top of its module:

EVERY REFUSAL IS AN ANSWER, NOT AN ERROR. The asking Bot is mid-run with a person waiting, so a refusal comes back as a sentence it can say. A thrown error ends the run with nothing said, which reads to the person as the Bot ignoring them.

escalation.ts competes with that tool for the same decision and did not follow it.

EscalationRoute returns { reached } or { refusal }, and execute handles both. A route that throws was not handled at all — the error came straight back out of execute. 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:

/*
 * Recorded either way. An escalation that could not be delivered is the one worth finding
 * later: the Bot stopped, the person was never asked, and without a row nothing says so.
 */

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. askTheirOwnPerson is 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:

WHO "A PERSON" IS, IS A SEAM. In this template it is the person in the conversation… A company running this has a different one: an on-call rota, a duty desk, a queue somebody works through in the morning. That is a route this deployment hands in.

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

  • New state that outlives a request? None. A try/catch and a local string.
  • What happens on the second replica? Nothing changes; no state.
  • Anything serialised? The thrown text, into the audit payload, capped at 400 like store.ts does with a vendor's words.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No.

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.

  • A throw now records agent.escalation_failed, the type already declared for this, rather than nothing.
  • The route's 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 — while the tool's return value is paraphrased by the model to the person who asked. handoff-runner.ts keeps those apart for the same reason. The payload reuses store.ts's two names: reason is the sentence the Bot was given, failure is what actually went wrong.
  • The sentence tells the Bot not to claim a person was asked. This one took some thought. The Bot is used to receiving PUT_TO here, 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.
  • Non-Error throws are handled (String(error)), so the handler written to keep the run alive cannot itself be the thing that ends it.

Changelog

  • A line under 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. On main:

(fail) asking a person > a route that throws is an answer, not the end of the run
(fail) asking a person > what the route threw is on the row and not in the answer
(fail) asking a person > a route that throws something that is not an Error is still an answer
 12 pass, 3 fail

On this branch:

bun test server/tests/agent-escalation.test.ts    15 pass, 0 fail
bun test server/tests/agent-handoff.test.ts       24 pass, 0 fail   (the sibling tool, unchanged)

They assert the three separate properties, not one: that execute returns rather than throws, that the answer contains neither PUT_TO nor the internal hostname while the row carries the full thrown text, and that a thrown string behaves like a thrown Error.

cd server && bunx tsc --noEmit reports only three pre-existing errors on my machine — cron-parser, @ag-ui/mastra, @mastra/client-js are declared in package.json but missing from my node_modules. They are identical on unmodified main and none is in a file this touches. Biome check on the three changed files: clean.

What is not covered

  • A route that hangs rather than throws. No timeout is imposed here, so a rota that accepts the connection and never answers still stalls the turn. That wants a deadline rather than a catch, and the honest place for it is the route contract, not this handler — worth its own issue if anyone wants it.
  • auditStore is optional on this tool, so a caller that omits it still gets no row on any path. server/src/index.ts:934 passes it, so every route in the product is covered; making the parameter required is a wider change than this one.

🤖 Generated with Claude Code

`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>
@Hotragn

Hotragn commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Two things I want on the record here rather than found in review.

What I measured, and what I took from the repo

The 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: execute threw instead of returning, and no audit row was written. Both are in the diff and both fail on main.

Not verified: what the runtime then does with that throw. tools goes straight into the agent configuration at copilot.ts:50 ({ tools, maxSteps }) and execute is invoked by the runtime, not by any code in this repo. @copilotkit/runtime is not resolvable in my checkout, so I could not read it, and I am not going to assert behaviour I could not run.

What I relied on instead is this repo's own stated invariant — handoff.ts:14-16, in a comment a maintainer wrote as the reason for that module's design:

EVERY REFUSAL IS AN ANSWER, NOT AN ERROR. The asking Bot is mid-run with a person waiting, so a refusal comes back as a sentence it can say. A thrown error ends the run with nothing said, which reads to the person as the Bot ignoring them.

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

handoff-tool.ts:100 has the identical unguarded await:

const outcome = await desk.send({ ... });

with the same comment beneath it saying a refusal is a sentence and not an exception. And desk.send is more exposed than the escalation route was: it awaits actorFor, profiles.list, mayAddress, queue.offer and recordAuditEvent (handoff.ts:133-278), every one of them a database call, in every deployment. The escalation route that ships cannot fail; this one is on the database's critical path always.

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. handoffTool holds no auditStore, so a catch there buys a sentence and still no row — trading a loud failure for a quiet one, which is not obviously an improvement. The fix belongs inside send, where refuse() already exists and already writes agent.handoff_refused. But if the throw came from the database, the audit write inside refuse is likely to fail too, so that path needs a decision about what to do when the trail itself is unavailable — and that is a question about this product's failure model, not a bug fix I should answer by myself in a follow-up.

Happy to send it as its own PR if you tell me which way that should go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant