fix: wire deferred review items end-to-end#39
fix: wire deferred review items end-to-end#39Telli merged 3 commits intofeat/channel-expansion-tools-featuresfrom
Conversation
Addresses remaining PR review comments that required deeper integration: Multi-agent routing: - Wire AgentRouteResolver into GatewayWorkers inbound processing - Apply route model overrides to sessions after creation Reasoning effort (/think): - Plumb session.ReasoningEffort into ChatOptions.AdditionalProperties for both RunAsync and RunStreamingAsync LLM call paths History compaction (/compact): - Make CompactHistoryAsync public on AgentRuntime - Wire SetCompactCallback from gateway runtime initialization so /compact triggers LLM-powered summarization instead of simple trim Verbose mode (/verbose): - Append tool call count and token delta footer to responses when session.VerboseMode is enabled DM policy enforcement: - Extend DmPolicy resolution in GatewayWorkers to cover Slack, Discord, and Signal channels (was only SMS/Telegram/WhatsApp/Teams) - Add DmPolicy validation for new channels in ConfigValidator Documentation: - Update ROADMAP.md to move completed channel/tool/preset/command work from future plans to "Recently Completed" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 Augment PR SummarySummary: Follow-up PR to wire previously deferred review items end-to-end across the gateway runtime. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| { | ||
| services.CommandProcessor.SetCompactCallback(async (session, ct) => | ||
| { | ||
| var before = session.History.Count; |
There was a problem hiding this comment.
before is computed but never used, and the callback returns a derived kept value that may not reflect what was actually preserved when CompactHistoryAsync falls back to TrimHistory. This can make the /compact response message report misleading “recent turns preserved” counts.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| if (resolvedRoute is not null) | ||
| { | ||
| if (resolvedRoute.ModelOverride is not null) | ||
| session.ModelOverride ??= resolvedRoute.ModelOverride; |
There was a problem hiding this comment.
Using session.ModelOverride ??= resolvedRoute.ModelOverride means routing won’t update the model for sessions that already have ModelOverride set (including persisted/stale overrides), so routing config changes may not take effect for existing sessions.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| if (session.VerboseMode) | ||
| { | ||
| var lastTurn = session.History.Count > 0 ? session.History[^1] : null; | ||
| var toolCallCount = lastTurn?.ToolCalls?.Count ?? 0; |
There was a problem hiding this comment.
toolCallCount is derived from session.History[^1].ToolCalls, but tool calls are stored on the separate [tool_use] history turn (the final assistant text turn usually has ToolCalls=null). This will often show 0 tool calls even when tools were invoked.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| heartbeatService.RecordResult(session, responseText, suppressHeartbeatDelivery, inputTokenDelta, outputTokenDelta); | ||
|
|
||
| // Append verbose mode footer (tool calls and token delta) | ||
| if (session.VerboseMode) |
There was a problem hiding this comment.
| : null | ||
| }; | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(session.ReasoningEffort)) |
There was a problem hiding this comment.
This wires reasoning_effort for the native AgentRuntime, but MafAgentRuntime constructs its own ChatOptions and doesn’t apply session.ReasoningEffort, so /think may be a no-op when Runtime.Orchestrator is MAF.
Severity: medium
Other Locations
src/OpenClaw.Agent/AgentRuntime.cs:401src/OpenClaw.MicrosoftAgentFrameworkAdapter/MafAgentRuntime.cs:402
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Pull request overview
This PR completes end-to-end wiring for previously added gateway commands and routing/policy behavior, integrating them into the inbound processing pipeline and agent runtime execution paths.
Changes:
- Enforces
DmPolicyfor Slack/Discord/Signal inGatewayWorkersand validates these config fields at startup. - Wires multi-agent route resolution into inbound processing and applies route model overrides onto sessions.
- Implements
/think,/compact, and/verbosefunctionality by plumbingReasoningEffort, wiring an LLM-backed compaction callback, and appending a verbose footer.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/OpenClaw.Gateway/Extensions/GatewayWorkers.cs | Enforces DM policy for additional channels, resolves routes on inbound, applies model override, and appends verbose footer. |
| src/OpenClaw.Gateway/Composition/RuntimeInitializationExtensions.cs | Wires /compact callback to invoke LLM-backed history compaction. |
| src/OpenClaw.Core/Validation/ConfigValidator.cs | Adds validation for Slack/Discord/Signal DmPolicy fields. |
| src/OpenClaw.Agent/AgentRuntime.cs | Plumbs reasoning_effort into LLM options and exposes CompactHistoryAsync publicly. |
| docs/ROADMAP.md | Updates roadmap to move completed work into “Recently Completed”. |
Comments suppressed due to low confidence (1)
src/OpenClaw.Gateway/Extensions/GatewayWorkers.cs:381
DmPolicyis validated case-insensitively (seeValidateDmPolicy), but the enforcement here uses case-sensitive pattern matching (policy is "closed"/policy is "pairing"). This can cause "Closed"/"PAIRING" configs to bypass the policy. Normalizepolicy(e.g.,ToLowerInvariant()) or compare withStringComparison.OrdinalIgnoreCasebefore enforcing.
// ── DM Pairing Check ─────────────────────────────────
var policy = "open";
if (msg.ChannelId == "sms") policy = config.Channels.Sms.DmPolicy;
if (msg.ChannelId == "telegram") policy = config.Channels.Telegram.DmPolicy;
if (msg.ChannelId == "whatsapp") policy = config.Channels.WhatsApp.DmPolicy;
if (msg.ChannelId == "teams") policy = config.Channels.Teams.DmPolicy;
if (msg.ChannelId == "slack") policy = config.Channels.Slack.DmPolicy;
if (msg.ChannelId == "discord") policy = config.Channels.Discord.DmPolicy;
if (msg.ChannelId == "signal") policy = config.Channels.Signal.DmPolicy;
if (policy is "closed")
continue; // Silently drop all inbound messages
if (!msg.IsSystem && policy is "pairing" && !pairingManager.IsApproved(msg.ChannelId, msg.SenderId))
{
| if (config.Routing.Enabled) | ||
| { | ||
| var routeResolver = new OpenClaw.Gateway.Integrations.AgentRouteResolver(config.Routing); |
There was a problem hiding this comment.
AgentRouteResolver is instantiated per inbound message. Since it only depends on config, this adds avoidable allocations on a hot path. Consider constructing it once (outside the loop) or registering/injecting a singleton resolver and reusing it for all messages.
| if (config.Routing.Enabled) | |
| { | |
| var routeResolver = new OpenClaw.Gateway.Integrations.AgentRouteResolver(config.Routing); | |
| static OpenClaw.Gateway.Integrations.AgentRouteResolver? routeResolver; | |
| if (config.Routing.Enabled) | |
| { | |
| routeResolver ??= new OpenClaw.Gateway.Integrations.AgentRouteResolver(config.Routing); |
| // Append verbose mode footer (tool calls and token delta) | ||
| if (session.VerboseMode) | ||
| { | ||
| var lastTurn = session.History.Count > 0 ? session.History[^1] : null; | ||
| var toolCallCount = lastTurn?.ToolCalls?.Count ?? 0; | ||
| responseText += $"\n\n---\n🔧 {toolCallCount} tool call(s) | ↑ {inputTokenDelta} in / {outputTokenDelta} out tokens (this turn)"; | ||
| } |
There was a problem hiding this comment.
The verbose footer’s tool call count is likely incorrect: RunAsync appends the final assistant text as the last history turn (without ToolCalls), while tool invocations are stored on earlier [tool_use] turns. As a result, session.History[^1].ToolCalls will typically be null and the footer will report 0 even when tools were used. Track tool calls for the current turn explicitly (e.g., capture history/tool-call counts before the run and diff after, or expose per-turn tool-call count from the runtime).
| { | ||
| var before = session.History.Count; | ||
| await concreteRuntime.CompactHistoryAsync(session, ct); | ||
| return Math.Min(config.Memory.CompactionKeepRecent, session.History.Count); |
There was a problem hiding this comment.
The /compact callback returns Math.Min(config.Memory.CompactionKeepRecent, session.History.Count) after compaction, but session.History.Count includes the inserted summary turn, so this can over-report “recent turns preserved” by 1 when compaction runs (and more generally won’t match the runtime’s keepCount). Also, before is assigned but never used. Compute and return the actual preserved turn count (e.g., based on the pre-compaction history size and keep-recent rules), and remove the unused local.
| return Math.Min(config.Memory.CompactionKeepRecent, session.History.Count); | |
| var preserved = Math.Min(config.Memory.CompactionKeepRecent, before); | |
| return preserved; |
| /// <summary> | ||
| /// Compacts session history by summarizing older turns via the LLM. | ||
| /// Keeps the most recent turns verbatim and replaces older ones with a summary. | ||
| /// </summary> | ||
| internal async Task CompactHistoryAsync(Session session, CancellationToken ct) | ||
| public async Task CompactHistoryAsync(Session session, CancellationToken ct) | ||
| { | ||
| if (session.History.Count <= _compactionThreshold) |
There was a problem hiding this comment.
CompactHistoryAsync was made public so the gateway can call it, but it’s still not exposed via IAgentRuntime, forcing callers to downcast (agentRuntime is AgentRuntime). This couples the gateway to a concrete orchestrator and makes it easy to miss compaction support in other runtimes. Consider introducing an interface (e.g., IHistoryCompactor) or adding a capability method on IAgentRuntime so the gateway can wire /compact without concrete casts.
…tegrations Complete README overhaul covering the current feature set: - 48 native tools organized by category with full table - 9 channel adapters with transport details and feature highlights - Tool presets (full/coding/messaging/minimal) and groups (group:*) - Chat commands (/think, /compact, /verbose) - Plugin installer (openclaw plugins install) - Multi-agent routing, Tailscale, Gmail Pub/Sub, mDNS - Updated architecture diagram showing message pipeline, session manager, route resolution, and tool backends - Simplified runtime flow diagram - Badges for tool count and channel count Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move AgentRouteResolver construction outside worker loop (was per-message) - Route model override now always applies (was ??= which skipped existing) - Fix verbose footer tool call count: scan all turns since last user turn instead of only checking the final assistant turn - Add verbose footer to streaming branch (was non-streaming only) - Fix /compact callback: return actual remaining turn count, remove unused var - Wire reasoning_effort into MafAgentRuntime.CreateChatOptions for MAF parity Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Follow-up to #38 addressing review comments that required deeper integration across the runtime.
AgentRouteResolvernow runs inGatewayWorkersinbound processing, applying model overrides from matched routes to sessionssession.ReasoningEffortplumbed intoChatOptions.AdditionalProperties["reasoning_effort"]for bothRunAsyncandRunStreamingAsyncpathsCompactHistoryAsyncmade public, callback wired from gateway initialization — LLM-powered summarization instead of simple trimGatewayWorkersand validation inConfigValidatorTest plan
/think highsets reasoning_effort in LLM call/compacttriggers LLM summarization (vs simple trim fallback)/verbose onshows tool call footer in responses🤖 Generated with Claude Code