Skip to content

Commit a31a49c

Browse files
committed
fix(core): LLM client resilience + tool-call extraction hardening
Audit wave 2026-07-11, findings 2/3/7/9/10/11/15: - ParseResponse degrades only the malformed tool call, keeps text + other calls - index-less streaming delta attributed to the sole open call; only open calls can be poisoned, completed ones never - rate-limit classification requires explicit signals (no more 'generate' hits) - circuit breaker: single half-open probe; abandoned/cancelled streams release the probe without counting as backend failure - ToolExecutionPolicy: top-level isError:true counts as failure (MCP contract) - text extractor: exact call shape required, cited/backticked/fenced examples skipped, balanced-paren scanning preserves parens inside arguments - assistant history is stripped of think-blocks before persisting - InGameLlmChatService serializes overlapping requests
1 parent 6025f8a commit a31a49c

16 files changed

Lines changed: 1122 additions & 86 deletions

Assets/CoreAI/CHANGELOG.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,41 @@
22

33
## [Unreleased]
44

5+
### Fixed (2026-07-11 audit wave)
6+
7+
- **Lua sandbox: nested guarded calls can no longer disarm the outer guard.** `LuaCsExecutionGuard` keeps a
8+
per-`LuaState` stack of installed hooks; leaving a nested `mods_call` restores the caller's hook instead of
9+
removing it, so step/time/alloc budgets stay armed across direct and indirect (`A→B→A`) cross-mod calls.
10+
- **Lua mods: transaction scope is reset after every handler/timer/load.** `LuaCsModRuntime` now accepts the
11+
shared `ILuaTransactionScope` and resets it in `finally`, so a handler dying between `coreai_world_begin`
12+
and commit no longer leaks an open transaction that silently swallows later world commands.
13+
- **Mod headers: tolerant capability parsing.** Unknown capability tokens are skipped (`Enum.TryParse`,
14+
fail-closed to `None` when nothing parses) and `ResourcesBundledModSource` isolates per-mod load failures,
15+
so one bad header no longer breaks seeding of all bundled mods.
16+
- **Non-streaming responses survive one bad tool call.** `ParseResponse` degrades only the malformed call to
17+
the parse-error marker contract; the text and remaining calls are preserved (previously the whole message
18+
was silently replaced with an empty one).
19+
- **Streaming: an index-less tool-call delta no longer poisons every pending call.** The fragment is
20+
attributed to the sole open call when unambiguous; only genuinely ambiguous open calls are failed, and
21+
completed calls are never touched.
22+
- **Error classification: `rate` substring false positives removed.** Rate-limit detection now requires
23+
explicit signals (`rate limit`, `429` status, `too many requests`, `quota`) instead of matching
24+
"gene**rate**"/"mode**rate**".
25+
- **Circuit breaker: half-open admits exactly one probe** (concurrent calls short-circuit) and an abandoned
26+
or cancelled stream releases the probe slot without being misclassified as a backend failure; a stream
27+
abandoned after a terminal error chunk still counts as a failure.
28+
- **Tool result classification: top-level `isError: true` (MCP contract) counts as failure**; nested `error`
29+
fields in legitimate tool payloads never did and still don't (regression-pinned by test).
30+
- **Text-extracted tool calls: quoted examples are no longer executed.** The extractor now requires exact
31+
call shape (top-level `name` string + `arguments` object), skips backtick/quote-cited spans and fenced
32+
code blocks, and preserves parentheses inside quoted arguments via balanced scanning (previously a lazy
33+
regex truncated them and dropped the call).
34+
- **Reasoning no longer persists into conversation history.** Assistant messages are run through the
35+
think-block filter before `AppendChatMessage`, so multi-kilobyte reasoning blobs stop inflating every
36+
subsequent turn's context.
37+
- **`InGameLlmChatService`: overlapping requests are serialized** (snapshot → LLM → append under one gate),
38+
so a second response always sees the first turn and history order cannot interleave.
39+
540
## 5.6.1 - Build-time policy registration + code-style pass (2026-07-11)
641

742
- **`AgentBuilder.Build()` auto-applies to the global policy.** When `CoreAIAgent.Policy` exists, `Build()`

Assets/CoreAI/Runtime/Core/Features/Llm/CircuitBreakerLlmClientDecorator.cs

Lines changed: 88 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ private enum State
4444
private State _state = State.Closed;
4545
private int _consecutiveFailures;
4646
private long _openedAtMs;
47+
private bool _halfOpenProbeInFlight;
4748

4849
/// <param name="inner">The client to protect.</param>
4950
/// <param name="failureThreshold">Consecutive transient failures that trip the breaker (min 1).</param>
@@ -89,25 +90,40 @@ public async Task<LlmCompletionResult> CompleteAsync(
8990
};
9091
}
9192

92-
LlmCompletionResult result;
93+
bool classified = false;
9394
try
9495
{
95-
result = await _inner.CompleteAsync(request, cancellationToken).ConfigureAwait(false);
96-
}
97-
catch (OperationCanceledException)
98-
{
99-
// WHY: Cancellation is caller intent, not a backend fault — do not count it, do not swallow it.
100-
throw;
96+
LlmCompletionResult result;
97+
try
98+
{
99+
result = await _inner.CompleteAsync(request, cancellationToken).ConfigureAwait(false);
100+
}
101+
catch (OperationCanceledException)
102+
{
103+
// WHY: Cancellation is caller intent, not a backend fault — do not count it, do not swallow it.
104+
throw;
105+
}
106+
catch (Exception ex)
107+
{
108+
// WHY: An unexpected throw from the inner client is treated as a transient backend fault.
109+
RecordFailure();
110+
classified = true;
111+
throw new LlmClientException(ex.Message, LlmErrorCode.ProviderError);
112+
}
113+
114+
RecordResult(result?.Ok ?? false, result?.ErrorCode ?? LlmErrorCode.ProviderError);
115+
classified = true;
116+
return result;
101117
}
102-
catch (Exception ex)
118+
finally
103119
{
104-
// WHY: An unexpected throw from the inner client is treated as a transient backend fault.
105-
RecordFailure();
106-
throw new LlmClientException(ex.Message, LlmErrorCode.ProviderError);
120+
if (!classified)
121+
{
122+
// WHY: The call ended without a health verdict (cancellation): release the half-open
123+
// probe slot so the breaker is not stuck waiting for a result that never comes.
124+
ReleaseHalfOpenProbe();
125+
}
107126
}
108-
109-
RecordResult(result?.Ok ?? false, result?.ErrorCode ?? LlmErrorCode.ProviderError);
110-
return result;
111127
}
112128

113129
public async IAsyncEnumerable<LlmStreamChunk> CompleteStreamingAsync(
@@ -129,6 +145,8 @@ public async IAsyncEnumerable<LlmStreamChunk> CompleteStreamingAsync(
129145
bool sawTerminalFailure = false;
130146
LlmErrorCode terminalCode = LlmErrorCode.None;
131147
bool sawAnyChunk = false;
148+
bool streamEnded = false;
149+
bool classified = false;
132150

133151
IAsyncEnumerator<LlmStreamChunk> e =
134152
_inner.CompleteStreamingAsync(request, cancellationToken).GetAsyncEnumerator(cancellationToken);
@@ -156,6 +174,7 @@ public async IAsyncEnumerable<LlmStreamChunk> CompleteStreamingAsync(
156174
catch (Exception ex)
157175
{
158176
RecordFailure();
177+
classified = true;
159178
moveError = ex.Message;
160179
chunk = default;
161180
}
@@ -180,16 +199,32 @@ public async IAsyncEnumerable<LlmStreamChunk> CompleteStreamingAsync(
180199

181200
yield return chunk;
182201
}
202+
203+
streamEnded = true;
183204
}
184205
finally
185206
{
186207
await e.DisposeAsync().ConfigureAwait(false);
187-
}
188208

189-
// WHY: Classify the whole stream: a stream that produced an error chunk (or nothing at all) is a
190-
// failure; a stream that ended cleanly is a success that closes/keeps-closed the breaker.
191-
bool ok = sawAnyChunk && !sawTerminalFailure;
192-
RecordResult(ok, ok ? LlmErrorCode.None : terminalCode);
209+
// WHY: Classify in the finally so the breaker state also updates when the consumer
210+
// abandons the await-foreach early (user stop): a stream that already produced a
211+
// terminal error chunk is a failure even then; a completed stream is classified as
212+
// before (an error chunk or nothing at all = failure, a clean end = success); an
213+
// abandoned/cancelled healthy stream carries no verdict at all — it only releases
214+
// the half-open probe slot and is never misclassified as a backend failure.
215+
if (!classified)
216+
{
217+
if (streamEnded || sawTerminalFailure)
218+
{
219+
bool ok = streamEnded && sawAnyChunk && !sawTerminalFailure;
220+
RecordResult(ok, ok ? LlmErrorCode.None : terminalCode);
221+
}
222+
else
223+
{
224+
ReleaseHalfOpenProbe();
225+
}
226+
}
227+
}
193228
}
194229

195230
/// <summary>Current state name for diagnostics/tests: "Closed", "Open", or "HalfOpen".</summary>
@@ -217,6 +252,7 @@ private bool TryEnter(out string rejectReason)
217252
if (_nowMs() - _openedAtMs >= _openDurationMs)
218253
{
219254
_state = State.HalfOpen;
255+
_halfOpenProbeInFlight = true;
220256
_log?.Invoke("[CircuitBreaker] half-open: admitting one probe request.");
221257
rejectReason = null;
222258
return true;
@@ -228,12 +264,42 @@ private bool TryEnter(out string rejectReason)
228264
return false;
229265
}
230266

231-
// WHY: Closed or HalfOpen: allow through (HalfOpen admits the single probe already in flight).
267+
if (_state == State.HalfOpen)
268+
{
269+
// WHY: Exactly ONE probe may be in flight while half-open. Admitting every concurrent
270+
// caller here used to burst the whole backlog onto a backend that is still likely down.
271+
if (_halfOpenProbeInFlight)
272+
{
273+
rejectReason =
274+
"Circuit breaker half-open: a single probe request is already in flight; " +
275+
"short-circuited until the probe result is known.";
276+
return false;
277+
}
278+
279+
_halfOpenProbeInFlight = true;
280+
_log?.Invoke("[CircuitBreaker] half-open: admitting one probe request.");
281+
rejectReason = null;
282+
return true;
283+
}
284+
232285
rejectReason = null;
233286
return true;
234287
}
235288
}
236289

290+
/// <summary>
291+
/// Releases the half-open probe slot when a call ends without any success/failure verdict
292+
/// (consumer cancelled the call or abandoned the stream). The breaker stays half-open so the
293+
/// next call becomes the new probe; abandonment is never counted as a backend failure.
294+
/// </summary>
295+
private void ReleaseHalfOpenProbe()
296+
{
297+
lock (_gate)
298+
{
299+
_halfOpenProbeInFlight = false;
300+
}
301+
}
302+
237303
private void RecordResult(bool ok, LlmErrorCode code)
238304
{
239305
if (ok)
@@ -260,6 +326,7 @@ private void RecordSuccess()
260326
lock (_gate)
261327
{
262328
_consecutiveFailures = 0;
329+
_halfOpenProbeInFlight = false;
263330
if (_state != State.Closed)
264331
{
265332
_state = State.Closed;
@@ -272,6 +339,7 @@ private void RecordFailure()
272339
{
273340
lock (_gate)
274341
{
342+
_halfOpenProbeInFlight = false;
275343
if (_state == State.HalfOpen)
276344
{
277345
// WHY: The probe failed — re-open for another cooldown.

0 commit comments

Comments
 (0)