fix: MCP tool refresh, final message delivery, and overflow retry#270
Merged
fix: MCP tool refresh, final message delivery, and overflow retry#270
Conversation
…ls in Redis Bug 1 - Final message not delivered: ChunkPollingService now reads the final snapshot from Redis before closing the channel on terminal Done chunks. Also handles task-state Completed fallback by reading lastContent from the hash. Bug 2 - Overflow messages stuck: SynchronizeTelegramMessagesInternalAsync now returns a SyncComplete flag. markdownActuallySynced is only set after a fully successful sync, ensuring the final retry fires when sends partially fail. Bug 3 - MCP tools not refreshed after runtime changes: Added McpToolHelper.RefreshAgentToolDefsInRedisAsync() and call it after every RegisterExternalMcpTools() in EditMcpConfService and McpInstallerToolService. Startup also uses the centralized method. Tests: Added 2 new tests verifying final snapshot delivery on Done terminal and task-state Completed fallback. All 417 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThe changes integrate Redis-backed refreshing of agent tool definitions across multiple services. A new method Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
TelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.cs (3)
259-263:⚠️ Potential issue | 🟠 MajorKeep failed deletes retryable.
A delete failure means the Telegram message is still live, but the returned state drops it and reports
SyncComplete=true. That can make overflow/stale messages untracked and unretryable.🐛 Proposed fix
try { await this.botClient.DeleteMessage(chatId, existingMsg.MessageId, cancellationToken: cancellationToken); this.logger.LogInformation($"SynchronizeMessages: Deleted message {existingMsg.MessageId} for empty chunk {i}."); nextHtmlMap.Remove(existingMsg.MessageId); - } catch (Exception ex) { this.logger.LogError(ex, $"SynchronizeMessages: Error deleting TG message {existingMsg.MessageId} for empty chunk {i}."); } + } catch (Exception ex) { + this.logger.LogError(ex, $"SynchronizeMessages: Error deleting TG message {existingMsg.MessageId} for empty chunk {i}."); + syncComplete = false; + nextTgMessagesState.Add(existingMsg); + } @@ try { await this.botClient.DeleteMessage(chatId, currentTgMessages[i].MessageId, cancellationToken: cancellationToken); this.logger.LogInformation($"SynchronizeMessages: Deleted superfluous message {currentTgMessages[i].MessageId}."); nextHtmlMap.Remove(currentTgMessages[i].MessageId); - } catch (Exception ex) { this.logger.LogError(ex, $"SynchronizeMessages: Error deleting superfluous TG message {currentTgMessages[i].MessageId}."); } + } catch (Exception ex) { + this.logger.LogError(ex, $"SynchronizeMessages: Error deleting superfluous TG message {currentTgMessages[i].MessageId}."); + syncComplete = false; + nextTgMessagesState.Add(currentTgMessages[i]); + }Also applies to: 286-292
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.cs` around lines 259 - 263, In SynchronizeMessages, do not remove message IDs from nextHtmlMap when DeleteMessage fails—only call nextHtmlMap.Remove(existingMsg.MessageId) after a successful await this.botClient.DeleteMessage(...) inside the try block; in the catch block log the error (using this.logger.LogError) but do NOT remove the ID so the message remains retryable. Apply the same change to the other delete block referenced (the one around lines 286-292) so both deletion paths only remove from nextHtmlMap on success.
272-281:⚠️ Potential issue | 🟠 MajorStop appending after a failed new chunk send.
If chunk
ifails but chunki + 1is sent,nextTgMessagesStatebecomes index-shifted;BuildResultForDbAsyncthen pairs the later Telegram message with the wrong markdown chunk. Mark invalid sends incomplete and stop so the final retry resumes from the missing chunk.🐛 Proposed fix
if (newMsg != null && newMsg.MessageId != 0) { nextTgMessagesState.Add(newMsg); nextHtmlMap[newMsg.MessageId] = htmlChunk; } else { this.logger.LogWarning($"SynchronizeMessages: Sending new TG message for chunk {i} returned null or invalid Message object."); + syncComplete = false; + break; } } catch (Exception ex) { this.logger.LogError(ex, $"SynchronizeMessages: Error sending new TG message for chunk {i}."); syncComplete = false; + break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.cs` around lines 272 - 281, In SynchronizeMessages, when a send for chunk i fails (either newMsg is null/invalid or an exception occurs), stop appending subsequent successful sends by marking syncComplete = false and breaking out of the loop instead of continuing; specifically, in the block handling newMsg (where nextTgMessagesState and nextHtmlMap are updated) only add entries when newMsg is valid, and in the else branch (invalid newMsg) set syncComplete = false and break, and likewise in the catch block set syncComplete = false and break so BuildResultForDbAsync won’t get an index-shifted nextTgMessagesState paired with the wrong markdown chunk.
238-256:⚠️ Potential issue | 🟠 MajorMark failed edits as incomplete before advancing the synced snapshot.
If
EditMessageTextfails or returns an invalid message, Telegram still contains the old chunk, but this method currently returnsSyncComplete=true; the caller can then skip the final retry and leave stale content visible.🐛 Proposed fix
if (editedMsg != null && editedMsg.MessageId != 0) { nextTgMessagesState.Add(editedMsg); nextHtmlMap[editedMsg.MessageId] = htmlChunk; } else { this.logger.LogWarning($"SynchronizeMessages: Editing TG message {existingMsg.MessageId} for chunk {i} returned null or invalid MessageId. Adding existing to list."); + syncComplete = false; nextTgMessagesState.Add(existingMsg); } @@ } catch (Exception ex) { this.logger.LogError(ex, $"SynchronizeMessages: Error editing TG message {existingMsg.MessageId} for chunk {i}. Adding existing to list."); + syncComplete = false; nextTgMessagesState.Add(existingMsg); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.cs` around lines 238 - 256, In SynchronizeMessages, when EditMessageText fails (catch Exception) or returns null/invalid (editedMsg == null || editedMsg.MessageId == 0), mark that chunk as not synchronized before progressing the snapshot by setting the sync-completion flag/indicator to false (e.g., set a local syncComplete = false or add the messageId to an incomplete set) and avoid updating nextHtmlMap for that message; keep the ApiRequestException branch for "message is not modified" as a successful case. Ensure the code paths that call this method (which inspect SyncComplete) will see the failure indicator so a final retry will run for that chunk.
🧹 Nitpick comments (4)
TelegramSearchBot.LLM/Service/Tools/McpInstallerToolService.cs (1)
27-38: Constructor DI addition is correct; consider null-guarding.Existing parameters use
??throw(seeEditMcpConfServicefor_mcpServerManager), while here none are null-guarded. Low risk since DI always provides singletons, but consider_redis = redis ?? throw new ArgumentNullException(nameof(redis));for consistency and earlier failure if mis-wired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot.LLM/Service/Tools/McpInstallerToolService.cs` around lines 27 - 38, The constructor for McpInstallerToolService doesn’t null-guard its injected dependencies; add null checks (e.g., _redis = redis ?? throw new ArgumentNullException(nameof(redis))) in the McpInstallerToolService constructor—apply the same pattern to other injected fields (_logger, _mcpServerManager) if they aren’t already guarded to match EditMcpConfService and fail fast on misconfiguration.TelegramSearchBot/Service/Manage/EditMcpConfService.cs (1)
246-246: Refresh calls are consistently placed afterRegisterExternalMcpTools— good.All 5 sites correctly pair
RegisterExternalMcpTools(_mcpServerManager)withawait McpToolHelper.RefreshAgentToolDefsInRedisAsync(connectionMultiplexer)inside the existingtry/catchblocks, so agent processes will see tool changes immediately after add/edit/env-update/remove/restart. This matches the invariant maintained inMcpInstallerToolService.cs.One nit: in
HandleEditingInputValueAsync(around line 393), the refresh still runs when theFieldIdTimeout/FieldIdEnabledbranches setchangeDescriptionto an error string (no config actually changed). It is harmless but produces an unnecessary Redis write; pre-existing logic, so not blocking.Also applies to: 393-393, 435-435, 486-486, 551-551
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/Manage/EditMcpConfService.cs` at line 246, HandleEditingInputValueAsync currently always calls McpToolHelper.RefreshAgentToolDefsInRedisAsync(connectionMultiplexer) after RegisterExternalMcpTools(_mcpServerManager), even when branches for FieldIdTimeout/FieldIdEnabled set changeDescription to an error (no config change). Modify HandleEditingInputValueAsync so that after the branch that sets changeDescription for FieldIdTimeout/FieldIdEnabled you only call RegisterExternalMcpTools(_mcpServerManager) and await McpToolHelper.RefreshAgentToolDefsInRedisAsync(connectionMultiplexer) when an actual change occurred (e.g., changeDescription does not indicate the error/no-op). Keep the existing try/catch structure and the pairing of RegisterExternalMcpTools with RefreshAgentToolDefsInRedisAsync, but gate the refresh behind a condition that checks that a config change was performed.TelegramSearchBot/Service/AI/LLM/ChunkPollingService.cs (2)
149-172: Optional: the innerDeliverFinalSnapshotAsynccall is largely redundant here.
TryCompleteFromTaskStateAsyncis only reached whenPollTaskAsyncalready observedAgentSnapshot(taskId)asNull(see lines 107-112). Re-reading the same key on line 151 will almost always be a no-op; it's a useful belt-and-suspenders read to close a tiny race between the twoStringGetAsynccalls, but a short comment noting that (or dropping it in favor of relying solely on thelastContentfallback) would make the intent clearer.Either option is fine — noting this for clarity only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/AI/LLM/ChunkPollingService.cs` around lines 149 - 172, The DeliverFinalSnapshotAsync call inside TryCompleteFromTaskStateAsync is effectively redundant because PollTaskAsync already observed AgentSnapshot(taskId) == Null; either remove the DeliverFinalSnapshotAsync(taskId, tracked, db, cancellationToken) call or add a short clarifying comment above it noting it's a belt‑and‑suspenders re-read to close a tiny race between the two StringGetAsync calls and that the subsequent lastContent fallback (the lastContentEntry/lastContent check and CompleteTrackedTaskAsync) is the actual safety net; update TryCompleteFromTaskStateAsync accordingly and ensure behavior remains covered by the lastContent/CompleteTrackedTaskAsync path.
186-199: Minor: thecatch (Exception)swallows cancellation as well.
tracked.Channel.Writer.WriteAsync(..., cancellationToken)can throwOperationCanceledExceptionwhen the polling loop is stopping, and the current catch silently discards it. That's consistent with the "best-effort" comment, but it makes shutdown observe a bit odd (the task still tries to complete afterwards even though cancellation was signaled). Consider either letting cancellation propagate or logging it.♻️ Optional refinement
- private async Task DeliverFinalSnapshotAsync(string taskId, TrackedTask tracked, IDatabase db, CancellationToken cancellationToken) { - try { - var snapshotJson = await db.StringGetAsync(LlmAgentRedisKeys.AgentSnapshot(taskId)); - if (!snapshotJson.HasValue) return; - - var snapshot = JsonConvert.DeserializeObject<AgentStreamChunk>(snapshotJson.ToString()); - if (snapshot != null && !string.IsNullOrEmpty(snapshot.Content) && snapshot.Content != tracked.LastContent) { - tracked.LastContent = snapshot.Content; - await tracked.Channel.Writer.WriteAsync(snapshot, cancellationToken); - } - } catch (Exception) { - // Best-effort: don't let snapshot read failure prevent task completion - } - } + private async Task DeliverFinalSnapshotAsync(string taskId, TrackedTask tracked, IDatabase db, CancellationToken cancellationToken) { + try { + var snapshotJson = await db.StringGetAsync(LlmAgentRedisKeys.AgentSnapshot(taskId)); + if (!snapshotJson.HasValue) return; + + var snapshot = JsonConvert.DeserializeObject<AgentStreamChunk>(snapshotJson.ToString()); + if (snapshot != null && !string.IsNullOrEmpty(snapshot.Content) && snapshot.Content != tracked.LastContent) { + tracked.LastContent = snapshot.Content; + await tracked.Channel.Writer.WriteAsync(snapshot, cancellationToken); + } + } catch (OperationCanceledException) { + throw; + } catch (Exception) { + // Best-effort: don't let snapshot read/deserialize failure prevent task completion + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TelegramSearchBot/Service/AI/LLM/ChunkPollingService.cs` around lines 186 - 199, In DeliverFinalSnapshotAsync, the broad catch is swallowing cancellation; change the error handling to let OperationCanceledException (or TaskCanceledException) propagate by catching it separately and rethrowing (or simply not catching it), while retaining the existing best-effort behavior for other exceptions—e.g., replace the single catch (Exception) with a catch (OperationCanceledException) { throw; } followed by a general catch (Exception) { /* best-effort log/ignore */ } so tracked.Channel.Writer.WriteAsync(...) cancellation is observed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TelegramSearchBot.LLM/Service/AI/LLM/McpToolHelper.cs`:
- Around line 1063-1073: RefreshAgentToolDefsInRedisAsync currently throws any
Redis exceptions and lacks a null-guard for the redis parameter, which can cause
upstream operations (e.g., EditMcpConfService.* handlers,
RestartAllServersAsync, McpInstallerToolService paths) to fail even though the
MCP change succeeded; modify RefreshAgentToolDefsInRedisAsync to validate redis
!= null and throw an ArgumentNullException if null, then wrap the Redis call in
a try/catch that logs the exception via the existing logging facility (or a new
logger) and suppresses the exception so the operation is a soft-fail (or
alternatively return a bool indicating success) — keep callers unchanged but
rely on the method to handle transient Redis outages and emit a clear warning
including exception details and context (method name and
LlmAgentRedisKeys.AgentToolDefs) so UX isn’t broken by stale Redis.
---
Outside diff comments:
In `@TelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.cs`:
- Around line 259-263: In SynchronizeMessages, do not remove message IDs from
nextHtmlMap when DeleteMessage fails—only call
nextHtmlMap.Remove(existingMsg.MessageId) after a successful await
this.botClient.DeleteMessage(...) inside the try block; in the catch block log
the error (using this.logger.LogError) but do NOT remove the ID so the message
remains retryable. Apply the same change to the other delete block referenced
(the one around lines 286-292) so both deletion paths only remove from
nextHtmlMap on success.
- Around line 272-281: In SynchronizeMessages, when a send for chunk i fails
(either newMsg is null/invalid or an exception occurs), stop appending
subsequent successful sends by marking syncComplete = false and breaking out of
the loop instead of continuing; specifically, in the block handling newMsg
(where nextTgMessagesState and nextHtmlMap are updated) only add entries when
newMsg is valid, and in the else branch (invalid newMsg) set syncComplete =
false and break, and likewise in the catch block set syncComplete = false and
break so BuildResultForDbAsync won’t get an index-shifted nextTgMessagesState
paired with the wrong markdown chunk.
- Around line 238-256: In SynchronizeMessages, when EditMessageText fails (catch
Exception) or returns null/invalid (editedMsg == null || editedMsg.MessageId ==
0), mark that chunk as not synchronized before progressing the snapshot by
setting the sync-completion flag/indicator to false (e.g., set a local
syncComplete = false or add the messageId to an incomplete set) and avoid
updating nextHtmlMap for that message; keep the ApiRequestException branch for
"message is not modified" as a successful case. Ensure the code paths that call
this method (which inspect SyncComplete) will see the failure indicator so a
final retry will run for that chunk.
---
Nitpick comments:
In `@TelegramSearchBot.LLM/Service/Tools/McpInstallerToolService.cs`:
- Around line 27-38: The constructor for McpInstallerToolService doesn’t
null-guard its injected dependencies; add null checks (e.g., _redis = redis ??
throw new ArgumentNullException(nameof(redis))) in the McpInstallerToolService
constructor—apply the same pattern to other injected fields (_logger,
_mcpServerManager) if they aren’t already guarded to match EditMcpConfService
and fail fast on misconfiguration.
In `@TelegramSearchBot/Service/AI/LLM/ChunkPollingService.cs`:
- Around line 149-172: The DeliverFinalSnapshotAsync call inside
TryCompleteFromTaskStateAsync is effectively redundant because PollTaskAsync
already observed AgentSnapshot(taskId) == Null; either remove the
DeliverFinalSnapshotAsync(taskId, tracked, db, cancellationToken) call or add a
short clarifying comment above it noting it's a belt‑and‑suspenders re-read to
close a tiny race between the two StringGetAsync calls and that the subsequent
lastContent fallback (the lastContentEntry/lastContent check and
CompleteTrackedTaskAsync) is the actual safety net; update
TryCompleteFromTaskStateAsync accordingly and ensure behavior remains covered by
the lastContent/CompleteTrackedTaskAsync path.
- Around line 186-199: In DeliverFinalSnapshotAsync, the broad catch is
swallowing cancellation; change the error handling to let
OperationCanceledException (or TaskCanceledException) propagate by catching it
separately and rethrowing (or simply not catching it), while retaining the
existing best-effort behavior for other exceptions—e.g., replace the single
catch (Exception) with a catch (OperationCanceledException) { throw; } followed
by a general catch (Exception) { /* best-effort log/ignore */ } so
tracked.Channel.Writer.WriteAsync(...) cancellation is observed.
In `@TelegramSearchBot/Service/Manage/EditMcpConfService.cs`:
- Line 246: HandleEditingInputValueAsync currently always calls
McpToolHelper.RefreshAgentToolDefsInRedisAsync(connectionMultiplexer) after
RegisterExternalMcpTools(_mcpServerManager), even when branches for
FieldIdTimeout/FieldIdEnabled set changeDescription to an error (no config
change). Modify HandleEditingInputValueAsync so that after the branch that sets
changeDescription for FieldIdTimeout/FieldIdEnabled you only call
RegisterExternalMcpTools(_mcpServerManager) and await
McpToolHelper.RefreshAgentToolDefsInRedisAsync(connectionMultiplexer) when an
actual change occurred (e.g., changeDescription does not indicate the
error/no-op). Keep the existing try/catch structure and the pairing of
RegisterExternalMcpTools with RefreshAgentToolDefsInRedisAsync, but gate the
refresh behind a condition that checks that a config change was performed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23d73e61-d495-4a1a-9e5a-d5ac5782cc9f
📒 Files selected for processing (8)
TelegramSearchBot.LLM.Test/Service/Tools/McpInstallerToolServiceTests.csTelegramSearchBot.LLM/Service/AI/LLM/McpToolHelper.csTelegramSearchBot.LLM/Service/Tools/McpInstallerToolService.csTelegramSearchBot.Test/Service/AI/LLM/ChunkPollingServiceTests.csTelegramSearchBot/AppBootstrap/GeneralBootstrap.csTelegramSearchBot/Service/AI/LLM/ChunkPollingService.csTelegramSearchBot/Service/BotAPI/SendMessageService.Streaming.csTelegramSearchBot/Service/Manage/EditMcpConfService.cs
Comment on lines
+1063
to
+1073
| /// <summary> | ||
| /// Re-exports tool definitions to Redis so that newly spawned agent processes | ||
| /// discover the latest available tools (including any MCP servers added/removed at runtime). | ||
| /// </summary> | ||
| public static async Task RefreshAgentToolDefsInRedisAsync(StackExchange.Redis.IConnectionMultiplexer redis) { | ||
| var toolDefs = ExportToolDefinitions(); | ||
| var json = JsonConvert.SerializeObject(toolDefs); | ||
| await redis.GetDatabase().StringSetAsync( | ||
| LlmAgentRedisKeys.AgentToolDefs, json, TimeSpan.FromHours(24)); | ||
| } | ||
|
|
There was a problem hiding this comment.
Refresh failures will surface to callers; consider the blast radius.
RefreshAgentToolDefsInRedisAsync does not log and does not swallow errors. In GeneralBootstrap.cs the call is wrapped in a try/catch that treats the failure as a soft warning, but in EditMcpConfService.HandleAddingTimeoutAsync/HandleEditingInputValueAsync/HandleEditingEnvValueAsync/HandleDeletingConfirmAsync/RestartAllServersAsync and in all McpInstallerToolService paths, the refresh is inside the outer operation's try/catch. A transient Redis outage will cause the add/remove/update/restart operation to report failure to the user even though the MCP server change already succeeded (server was added but refresh threw, which rolls nothing back). Consider either:
- Catching and logging within
RefreshAgentToolDefsInRedisAsyncso stale-Redis does not break the MCP lifecycle UX, or - Wrapping each call site in its own try/catch and only warning on failure.
Also worth noting: no null-guard on redis, which would produce an NRE rather than a meaningful error if a caller ever passes null.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TelegramSearchBot.LLM/Service/AI/LLM/McpToolHelper.cs` around lines 1063 -
1073, RefreshAgentToolDefsInRedisAsync currently throws any Redis exceptions and
lacks a null-guard for the redis parameter, which can cause upstream operations
(e.g., EditMcpConfService.* handlers, RestartAllServersAsync,
McpInstallerToolService paths) to fail even though the MCP change succeeded;
modify RefreshAgentToolDefsInRedisAsync to validate redis != null and throw an
ArgumentNullException if null, then wrap the Redis call in a try/catch that logs
the exception via the existing logging facility (or a new logger) and suppresses
the exception so the operation is a soft-fail (or alternatively return a bool
indicating success) — keep callers unchanged but rely on the method to handle
transient Redis outages and emit a clear warning including exception details and
context (method name and LlmAgentRedisKeys.AgentToolDefs) so UX isn’t broken by
stale Redis.
Contributor
🔍 PR检查报告📋 检查概览
🧪 测试结果
📊 代码质量
📁 测试产物
🔗 相关链接此报告由GitHub Actions自动生成 |
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
1 participant
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.
Summary
Fixes three bugs in the LLM agent process separation system:
Bug 1: Final message content not delivered to Telegram
Root cause: When the agent completes (Done terminal), \ChunkPollingService\ checked \ erminal.Content\ which was empty for Done chunks, then immediately closed the channel without reading the final snapshot from Redis.
Fix: \PollTaskAsync\ now calls \DeliverFinalSnapshotAsync()\ to read the \AGENT_SNAPSHOT:{taskId}\ key and deliver it to the channel before closing. The \TryCompleteFromTaskStateAsync\ fallback path also reads both the snapshot key and \lastContent\ from task state hash.
Bug 2: Overflow messages stuck when content spills to new TG message
Root cause: \markdownActuallySynced\ was set before calling \SynchronizeTelegramMessagesInternalAsync, so if the sync partially failed (new message send error), the final retry check \latestMarkdownSnapshot != markdownActuallySynced\ was false — no retry happened.
Fix: \SynchronizeTelegramMessagesInternalAsync\ now returns a \SyncComplete\ flag. \markdownActuallySynced\ is only set after a fully successful sync, and the final retry condition also checks !lastSyncComplete.
Bug 3: MCP tools not available after runtime changes
Root cause: Tool definitions were exported to Redis once at startup. When MCP servers were added/removed/restarted at runtime, \RegisterExternalMcpTools()\ updated the in-process registry, but the Redis key \AGENT_TOOL_DEFS\ was never refreshed.
Fix: Added \McpToolHelper.RefreshAgentToolDefsInRedisAsync()\ and call it after every \RegisterExternalMcpTools()\ in \EditMcpConfService\ (5 sites) and \McpInstallerToolService\ (4 sites). Startup also uses this centralized method.
Tests
Files Changed
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes