Skip to content

fix: abort stalled LLM requests - #24

Merged
thecodacus merged 3 commits into
thecodacus:mainfrom
madmoneymike5:agent/abort-stalled-llm-requests
Oct 12, 2026
Merged

thecodacus merged 3 commits into
thecodacus:mainfrom
madmoneymike5:agent/abort-stalled-llm-requests

Conversation

@madmoneymike5

Copy link
Copy Markdown
Contributor

What changed

  • add a configurable total LLM deadline through UNDERSTORY_LLM_TIMEOUT_MS (default: 120 seconds)
  • propagate caller cancellation through query, mutation, chat, and hot-memory model calls
  • cancel in-flight MCP model work when the HTTP client disconnects
  • report a failed mutation when the model returns without changing any files
  • add regression coverage for timeout, caller cancellation, and zero-write mutations

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 filesChanged was 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 test
  • pnpm build
  • git diff --check

@madmoneymike5

Copy link
Copy Markdown
Contributor Author

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.

@madmoneymike5
madmoneymike5 marked this pull request as ready for review August 7, 2026 03:03
GitMakers1 pushed a commit to GitMakers1/understory that referenced this pull request Aug 22, 2026
…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 thecodacus left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Ringosystems

Ringosystems commented Sep 13, 2026 •

Copy link
Copy Markdown

First, thank you for Understory. We run it as the memory layer of a personal knowledge project, entirely on local models, and it has become the part of the stack we care most about. These measurements are offered in support of the two changes requested above, and I would be glad to test a revised version of this PR against our deployment before it merges.

We run against Qwen3.8-27B on llama.cpp (~26 tok/s generation, ~137 tok/s prefill) over a bundle of ~100 concepts. Over 450 timed memory_add calls in the last two weeks: median 425 s, 90th percentile 998 s, maximum 1,751 s. Dream passes take about 25 minutes, because the dream prompt reaches ~94k tokens on a bundle this size. With a 120 s default deadline, every one of our mutations and every dream would abort. So +1 to no deadline unless configured, and to LLM_TIMEOUT with duration strings.

One small thing while you are in there: log the effective deadline at startup (llm timeout: none / llm timeout: 2h) and warn on an unparseable value. As written, NaN, 0 and an empty string fall silently to the default and nothing tells the operator; with "no deadline unless configured", a typo should mean no deadline plus one warning line, not a silent 120 s.

On zero writes: agreed that text with zero writes is a success. We verify every memory_add against the bundle afterwards (is the sent text now present in the target concept?), and "already present, wrote nothing" is a real and common correct outcome for re-sent material. Empty text and zero writes is the failure signature we see too.

Happy to run any revision against our setup and report numbers back.


Transparency note. I am a fan of this project and want to help, so I want to be clear about how this was produced. I run Understory pinned by digest as the memory layer of a personal knowledge-capture project (a private repo) on my own hardware. The measurements come from that project's instrumentation: every memory_add is timed and then checked against the bundle by content. The review of the open pull-request queue that led to these posts, and the drafting of the text, were done with an AI assistant (Claude, via Claude Code) working against the actual diffs and my deployment. I read, corrected and approved every word before posting, and the numbers were re-checked against my records; two earlier claims were dropped as wrong before anything was posted. We also run a derived image (git plus safe.directory and committer identity, an undici preload with a 30-minute headers timeout, and a git init entrypoint), which is why #26 and #32 matter to us directly. No employer data or systems are involved. If any of this is unwelcome in your tracker, say so and I will adjust or withdraw it.

…tions

Applies the two changes requested in review.

- No deadline unless LLM_TIMEOUT is set. Healthy runs on local hardware
  take minutes (measured medians of 425s for memory_add were reported on
  this PR), so a 120s default would abort working setups. Cancelling
  in-flight model work when the client disconnects stays always-on.
- LLM_TIMEOUT takes duration strings (300s, 10m, 2h) through the same
  parser as DREAM_INTERVAL and HOT_MEMORY_TTL, replacing
  UNDERSTORY_LLM_TIMEOUT_MS. The effective deadline is logged at startup
  and an unparseable value warns and applies no deadline.
- A mutation that writes nothing is only a failure when the model also
  returned no text. Writing nothing while explaining why (knowledge
  already present, nothing related to link) is a designed outcome.
@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1f70d0db-5908-407d-b92a-5b3e5accdb73

📥 Commits

Reviewing files that changed from the base of the PR and between 282c37f and f13c562.


📒 Files selected for processing (9)
  • .env.example
  • packages/core/src/agent/agent.ts
  • packages/core/src/agent/hot-memory.ts
  • packages/core/src/agent/index.ts
  • packages/core/test/agent.test.ts
  • packages/server/src/index.ts
  • packages/server/src/mcp/http.ts
  • packages/server/src/mcp/server.ts
  • packages/server/src/mcp/stdio.ts

 _______________________________________________________
< Veni, Vidi, VCS: I came, I saw, I version-controlled. >
 -------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thecodacus thecodacus left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sat long enough that I pushed the two requested changes to the branch myself so it can land. Your abort plumbing is untouched.

What changed in the last commit:

  • No deadline unless LLM_TIMEOUT is set, and it takes duration strings (300s, 10m, 2h) like the other knobs. Cancelling the model call when the client disconnects stays always on.
  • The effective deadline is logged at startup, and an unparseable value warns and applies no deadline. Thanks @Ringosystems for the numbers and for that suggestion.
  • A mutation that writes nothing only fails when the model also returned no text.

Checked against a backend that accepts the request and never answers: a client that gives up after 3s closes the upstream connection in the same second, LLM_TIMEOUT=2s returns a failed mutation at 2s, and LLM_TIMEOUT=120000 prints the warning and runs with no deadline.

Thanks for the contribution @madmoneymike5.

@thecodacus thecodacus changed the title Abort stalled LLM requests fix: abort stalled LLM requests Oct 12, 2026
@thecodacus
thecodacus merged commit dfecee9 into thecodacus:main Oct 12, 2026
2 of 3 checks passed
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.

3 participants