Refuse a blank question put to a person, the way a blank task handed to a Bot is refused - #476
Merged
davidmckayv merged 2 commits intoSep 10, 2026
Conversation
…to a Bot is refused `ask_person` already answers a call with nothing in it by telling the model to say what it needs, but `z.string()` accepts "" and a run of spaces, so that only fired when the field was missing. Spelled the other way the turn ended on a question nobody was asked, and the trail took an `agent.escalated` row with nothing in its question. The tool this competes with for the same decision refuses a blank task and says so; this is the other half of that. The trimmed text is what travels, so what is recorded is what was asked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kevin9327
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
September 9, 2026 23:07
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
# Conflicts: # CHANGELOG.md
davidmckayv
approved these changes
Sep 10, 2026
davidmckayv
left a comment
Contributor
There was a problem hiding this comment.
Deep-reviewed against live code (correctness, governance, no vendor/secret/scale issues). Composed build+tests green. CHANGELOG/format rebase on CI-validated substance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A Bot calls
ask_personwith{"question": ""}. It is told:so it stops, and its turn ends. On the Audit screen there is now an
agent.escalatedrow — the rowan administrator counts escalations by — whose
questionis empty. It says a person was askedsomething that was never said.
On a deployment whose
EscalationRouteis a duty desk or an on-call rota rather than the personalready in the conversation, which is the seam the module is explicitly built around, that is a page
to somebody with no question on it.
Why
server/src/agents/escalation.tsalready has the sentence for this case:and
server/tests/agent-escalation.test.tsalready asserts it, undera call with nothing in it is refused as a sentence, by callingexecute({}).But
questionisz.string(), which accepts""and a run of spaces. So the refusal fired onlywhen the field was missing. Present and empty is the same call spelled the other way, and it went
straight through.
The tool this sits beside and competes with for the same decision does check.
handoff.ts:A blank task handed to a Bot is refused, recorded as
no_task, and answered with a sentence. A blankquestion put to a person was not.
The fix
Trim the question, and refuse it with the sentence that was already written for this. The trimmed
text is then what travels to the route and onto the row — for the same reason a handoff sends the
trimmed task: what is recorded should be what was asked, not the spacing around it.
Deliberately not changed:
whystays optional and is left as the model wrote it, and every otherpath through the tool is untouched.
Measured
bun test server/tests/agent-escalation.test.tsorigin/mainwith only the new tests applied: 8 pass, 4 fail.an empty question is refused as a sentence— returns the "That was put to…" sentence.a question of spaces is refused as a sentence— same.a blank question reaches nobody and leaves no row saying it did— the route is called and anagent.escalatedrow is written.a question with room around it is recorded as the question— the row carries" which account? ".The pins are the eight cases already in that file, which pass before and after: a real question still
reaches the route and comes back with the
PUT_TOmarker the transcript matches on, the row stillcarries the question and the
why, a routine's escalation is still filed under its initiator, aroute that reaches nobody is still recorded as
agent.escalation_failed, andexecute({})is stillrefused.
Also run, all green:
bun test server/tests/agent-handoff-tool.test.ts server/tests/agent-handoff.test.ts server/tests/control-refusal-end-to-end.test.ts server/tests/human-input-end-to-end.test.ts— 41 pass, 0 failbun run --filter server typecheck— exit 0bunx biome checkon both changed files — cleanNote on the CHANGELOG
A turn that used to end now does not, so there is an entry. It goes at the top of
## Unreleased,the one line every entry goes at, so it will conflict with any other PR open against that anchor.
Happy to rebase whenever it suits you.