fix(gateway): hot-reload agents.defaults.models changes#33820
fix(gateway): hot-reload agents.defaults.models changes#33820Mylszd wants to merge 3 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
gateway
Greptile SummaryThis PR fixes hot-reload handling for
Confidence Score: 3/5
Last reviewed commit: fbe2ee1 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (replyPayloads.length === 0) { | ||
| // Heartbeat runs can be configured to deliver via a messaging tool | ||
| // (for example when heartbeat.target routes to iMessage/WhatsApp). | ||
| // In that case buildReplyPayloads may suppress duplicate payloads because | ||
| // the text/media was already delivered externally, leaving the main session | ||
| // with "no response" and triggering followup chaining. | ||
| const deliveredViaMessagingTool = | ||
| (runResult.messagingToolSentTexts?.length ?? 0) > 0 || | ||
| (runResult.messagingToolSentMediaUrls?.length ?? 0) > 0; | ||
| if (isHeartbeat && deliveredViaMessagingTool) { | ||
| return finalizeWithFollowup({ text: SILENT_REPLY_TOKEN }, queueKey, runFollowupTurn); | ||
| } | ||
| return finalizeWithFollowup(undefined, queueKey, runFollowupTurn); |
There was a problem hiding this comment.
Early return skips SILENT_REPLY_TOKEN for zero-payload messaging-tool deliveries
The new check for isHeartbeat && deliveredViaMessagingTool is placed inside the replyPayloads.length === 0 block, which is only reached when payloadArray is non-empty (there is an earlier return at the payloadArray.length === 0 guard above).
If a heartbeat agent invokes a messaging tool to deliver content but emits no direct payloads (i.e., runResult.payloads is empty/undefined), the function returns undefined via the earlier early-return, completely bypassing the new logic. This would fail to return SILENT_REPLY_TOKEN even though the content was delivered via the messaging tool.
The deliveredViaMessagingTool guard should also be applied to the earlier early-return path:
// Around line 475-477
if (payloadArray.length === 0) {
const deliveredViaMessagingTool =
(runResult.messagingToolSentTexts?.length ?? 0) > 0 ||
(runResult.messagingToolSentMediaUrls?.length ?? 0) > 0;
if (isHeartbeat && deliveredViaMessagingTool) {
return finalizeWithFollowup({ text: SILENT_REPLY_TOKEN }, queueKey, runFollowupTurn);
}
return finalizeWithFollowup(undefined, queueKey, runFollowupTurn);
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/agent-runner.ts
Line: 503-515
Comment:
**Early return skips `SILENT_REPLY_TOKEN` for zero-payload messaging-tool deliveries**
The new check for `isHeartbeat && deliveredViaMessagingTool` is placed inside the `replyPayloads.length === 0` block, which is only reached when `payloadArray` is **non-empty** (there is an earlier return at the `payloadArray.length === 0` guard above).
If a heartbeat agent invokes a messaging tool to deliver content but emits **no direct payloads** (i.e., `runResult.payloads` is empty/undefined), the function returns `undefined` via the earlier early-return, completely bypassing the new logic. This would fail to return `SILENT_REPLY_TOKEN` even though the content was delivered via the messaging tool.
The `deliveredViaMessagingTool` guard should also be applied to the earlier early-return path:
```ts
// Around line 475-477
if (payloadArray.length === 0) {
const deliveredViaMessagingTool =
(runResult.messagingToolSentTexts?.length ?? 0) > 0 ||
(runResult.messagingToolSentMediaUrls?.length ?? 0) > 0;
if (isHeartbeat && deliveredViaMessagingTool) {
return finalizeWithFollowup({ text: SILENT_REPLY_TOKEN }, queueKey, runFollowupTurn);
}
return finalizeWithFollowup(undefined, queueKey, runFollowupTurn);
}
```
How can I resolve this? If you propose a fix, please make it concise.| "limit exhausted", | ||
| /weekly\/monthly limit/i, |
There was a problem hiding this comment.
"limit exhausted" substring match is overly broad
The plain string pattern "limit exhausted" is matched via value.includes(pattern) on the lowercased error message. This will also match phrases like "retry limit exhausted" or "reconnection limit exhausted" — which are transport/SDK-level errors and should not trigger the rate-limit failover path.
The accompanying regex /weekly\/monthly limit/i is precise enough to catch the specific zhipuai "Weekly/Monthly Limit Exhausted" error on its own. Consider removing the generic "limit exhausted" string pattern, or replacing it with a tighter regex (e.g., anchored to the usage/api/weekly/monthly context) to avoid false positives that could redirect retries through the rate-limit cooldown path when the real cause is a transient connectivity issue.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-helpers/failover-matches.ts
Line: 13-14
Comment:
**`"limit exhausted"` substring match is overly broad**
The plain string pattern `"limit exhausted"` is matched via `value.includes(pattern)` on the lowercased error message. This will also match phrases like `"retry limit exhausted"` or `"reconnection limit exhausted"` — which are transport/SDK-level errors and should not trigger the rate-limit failover path.
The accompanying regex `/weekly\/monthly limit/i` is precise enough to catch the specific zhipuai "Weekly/Monthly Limit Exhausted" error on its own. Consider removing the generic `"limit exhausted"` string pattern, or replacing it with a tighter regex (e.g., anchored to the `usage`/`api`/`weekly`/`monthly` context) to avoid false positives that could redirect retries through the rate-limit cooldown path when the real cause is a transient connectivity issue.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
agents.defaults.modelsas a hot-reloadable prefix in gateway reload planningagents.defaults.models.*pathsWhy
Issue #33600 reports that editing
agents.defaults.modelsis detected but not applied at runtime. The reload plan previously only handledagents.defaults.modelandmodels.*, so allowlist edits were skipped by hot actions.Testing
pnpm vitest src/gateway/config-reload.test.tsCloses #33600