Abort stalled LLM requests - #24
Conversation
|
This is my first contribution. I reproduced this against a stalled local llama.cpp backend. Happy to adjust the timeout behavior or tests based on your preferences. |
…ettings threading AgentOptions gains abortSignal + timeoutMs (UNDERSTORY_LLM_TIMEOUT_MS, default 120s); MCP HTTP requests abort in-flight LLM work on client disconnect; no-op mutations now report failure instead of silent success. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thecodacus
left a comment
There was a problem hiding this comment.
The disconnect-abort part of this is genuinely important, maybe more than you realize: on local inference a dead MCP client doesn't just waste one request, it leaves llama-server grinding while everything else queues behind it. The AbortSignal.any([caller, timeout]) plumbing is clean and the test coverage is real. I want this in.
Two things need to change first, both grounded in how this actually runs on local hardware.
1. The 120s default will abort healthy runs
On my 3060, a routine memory_add through a 35B takes one to two minutes, longer when llama-swap has to load the model first, and dream consolidation runs have gone past five minutes and succeeded. A 120 second total deadline as the default breaks the primary audience of this project on day one.
Suggestion: no deadline unless configured. The disconnect-abort stays always-on (that's the real win and costs nobody anything), and users who want a deadline opt in. Also please rename to LLM_TIMEOUT with duration strings (10m, 300s) via the existing parseDuration util — every other knob here works that way (DREAM_INTERVAL=6h, HOT_MEMORY_TTL=1h), and UNDERSTORY_LLM_TIMEOUT_MS is the only env with the project prefix.
2. Zero-files-changed is not a failure
The mutation prompt explicitly allows a legitimate no-write outcome: when the knowledge already exists verbatim, the agent is told to say so and write nothing. Dream runs are also instructed to leave orphans alone when nothing genuinely relates. Under this PR those correct outcomes return isError to MCP clients and record failed traces.
The stall you're actually hunting has a tighter signature, and I've hit it in the wild (gemma-4-12b flaking on tool calls): empty text AND zero writes. Flag that as failed — it's always wrong. Text with zero writes stays a success.
Fix those two and I'll merge. The abort infrastructure itself is exactly right.
What changed
UNDERSTORY_LLM_TIMEOUT_MS(default: 120 seconds)Why
When an LLM backend stalled, an MCP operation could wait indefinitely because Understory did not set a total model deadline. Canceling the client request also left the underlying model call running, which could keep a local inference server occupied and queue later requests behind it.
Separately, a mutation could report success when the model returned text without invoking a write tool, even though
filesChangedwas empty.Impact
Stalled inference now terminates after a bounded interval, disconnected clients stop consuming model capacity, and callers no longer receive false-success responses for mutations that made no changes.
Validation
pnpm testpnpm buildgit diff --check