feat(continuity): harden adaptive reply threading and subagent delivery continuity#40377
feat(continuity): harden adaptive reply threading and subagent delivery continuity#40377KimGLee wants to merge 156 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
Bot
added
channel: telegram
Greptile SummaryThis PR hardens three coupled delivery layers: adaptive Telegram reply threading via burst detection, Key changes and observations:
Confidence Score: 3/5
Last reviewed commit: 239293c |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (wait?.status !== "ok" && wait?.status !== "error") { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Unexpected status causes permanent exit in perpetual poll loop
When callGateway returns null or an unexpected status string, the condition on line 1241 is true and the entire while(true) loop exits silently via return. For runs without a hard deadline (!hasHardTimeout), this permanently abandons monitoring — the subagent run will remain stuck in a pending state with no further completion signaling.
The previous single-shot implementation had the same early-exit behavior, but its impact was bounded. In the new perpetual polling paradigm, a single transient protocol anomaly (e.g., a partial gateway response) permanently stalls the run.
Consider using continue (with a brief backoff) instead of return for this defensive guard, so transient unexpected responses don't terminate long-running waits:
if (wait?.status !== "ok" && wait?.status !== "error") {
// Transient or unknown response — don't permanently abandon the run.
// Log and retry on next poll cycle instead of exiting.
defaultRuntime.log(`[warn] Unexpected agent.wait status run=${runId} status=${wait?.status}`);
await new Promise((resolve) => setTimeout(resolve, 5_000));
continue;
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/subagent-registry.ts
Line: 1241-1243
Comment:
**Unexpected status causes permanent exit in perpetual poll loop**
When `callGateway` returns `null` or an unexpected status string, the condition on line 1241 is `true` and the entire `while(true)` loop exits silently via `return`. For runs without a hard deadline (`!hasHardTimeout`), this permanently abandons monitoring — the subagent run will remain stuck in a pending state with no further completion signaling.
The previous single-shot implementation had the same early-exit behavior, but its impact was bounded. In the new perpetual polling paradigm, a single transient protocol anomaly (e.g., a partial gateway response) permanently stalls the run.
Consider using `continue` (with a brief backoff) instead of `return` for this defensive guard, so transient unexpected responses don't terminate long-running waits:
```ts
if (wait?.status !== "ok" && wait?.status !== "error") {
// Transient or unknown response — don't permanently abandon the run.
// Log and retry on next poll cycle instead of exiting.
defaultRuntime.log(`[warn] Unexpected agent.wait status run=${runId} status=${wait?.status}`);
await new Promise((resolve) => setTimeout(resolve, 5_000));
continue;
}
```
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 239293c93f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function buildReplyBurstKey(ctx: TelegramContext): string { | ||
| const msg = ctx.message; | ||
| const chatId = String(msg.chat.id); | ||
| const senderId = msg.from?.id != null ? String(msg.from.id) : "unknown"; |
There was a problem hiding this comment.
Including msg.from.id in the burst-state key means each participant in a group chat gets an independent streak, so alternating rapid messages from different users never reach streak >= 2 and adaptive threading stays off. In busy group conversations this removes reply bubbles exactly when disambiguation is most needed, even though the chat itself is in a burst.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 1b82a0f49.
I changed burst-key scoping so group/supergroup chats are tracked at chat scope (not per-sender), while private chats still keep sender-specific scope. This allows alternating rapid messages from different participants to build the same burst streak and enables adaptive reply threading as intended.
Also added a regression test in src/telegram/bot-message.test.ts that simulates two rapid group messages from different sender IDs and verifies reply mode flips from off to first on the second message.
|
Implemented in follow-up commit f507e2ecd: configurable adaptive burst windows + optional EMA learning. What changed
New config (high level)
EMA window logic (when learning enabled)Per burst-key state keeps
Default remains fixed-window mode unless |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f507e2ecdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (params.configuredMode === "off" || !params.adaptiveConfig.enabled) { | ||
| return "off"; | ||
| } |
There was a problem hiding this comment.
When replyAdaptive.enabled is set to false, this branch forces "off" and bypasses the configured replyToMode ("first"/"all"). That makes it impossible to disable burst adaptation without also disabling reply threading entirely, which is a behavior regression from the pre-adaptive path and contradicts the config’s role as an adaptive toggle.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Great catch — fixed in f3be474c6.
replyAdaptive.enabled=false now preserves configured replyToMode (first/all) instead of forcing off.
I also added a regression test (preserves configured reply mode when adaptive logic is disabled) in src/telegram/bot-message.test.ts to lock this behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3be474c66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sweepExpiredReplyBurstState({ | ||
| now, | ||
| burstState: params.burstState, | ||
| ttlMs: Math.max(veryDenseWindowMs, params.adaptiveConfig.veryDenseWindowMs), |
There was a problem hiding this comment.
When EMA learning is enabled, ttlMs is derived from the current message’s veryDenseWindowMs, so a fast-paced chat can trigger sweepExpiredReplyBurstState with a small TTL and evict state for other chats that previously learned a much larger window. In practice, a chat that should still be in-burst (e.g., learned 60–75s window) can lose its streak after ~25s as soon as another chat receives traffic, causing reply threading to flip back to off unexpectedly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes, this is a new actionable review — fixed in d3a9441ca.
I changed burst-state sweeping to use per-key TTL (state.ttlMs) instead of a single global TTL from the current message context. This prevents traffic in one chat from evicting learned state in another chat that has a larger learned window.
Also added a regression test (does not evict another chat using a larger learned ttl) in src/telegram/bot-message.test.ts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7f32ff1f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const recentShortMessageAt = (previous?.recentShortMessageAt ?? []).filter( | ||
| (ts) => now - ts <= params.adaptiveConfig.veryDenseWindowMs, | ||
| ); |
There was a problem hiding this comment.
When EMA learning is enabled, this pruning still uses the fixed adaptiveConfig.veryDenseWindowMs instead of the learned veryDenseWindowMs, so short-message history is truncated too aggressively once the learned window grows. In chats where learned windows expand (for example with higher baseMaxMs and multipliers), this prevents shortCount from reaching dense/very-dense thresholds and can flip adaptive reply threading back to off even though the learned burst window should still treat the conversation as active.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f68d7a7196
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -607,6 +607,7 @@ async function sendAnnounce(item: AnnounceQueueItem) { | |||
| threadId: requesterIsSubagent ? undefined : threadId, | |||
| deliver: !requesterIsSubagent, | |||
| internalEvents: item.internalEvents, | |||
| currentMessageId: item.requesterMessageId, | |||
There was a problem hiding this comment.
sendAnnounce now always forwards currentMessageId from the queued item, but in collect mode scheduleAnnounceDrain (in src/agents/subagent-announce-queue.ts) collapses multiple queued announces into one prompt by sending ...last, so mixed batches inherit only the last item’s requesterMessageId. When the same requester session accumulates queued completions from different turns before drain, the synthesized multi-item completion message is forced to reply to the wrong inbound message, which breaks the provenance this change is trying to preserve; this path should either partition by message id or clear currentMessageId for aggregated sends.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f79ef73fa6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| items | ||
| .map((item) => item.requesterMessageId?.trim()) | ||
| .filter((value): value is string => Boolean(value)), |
There was a problem hiding this comment.
In collect mode, this set is built only from truthy requesterMessageId values, so items without an ID are ignored. When a batch mixes a user-triggered announce (msg-...) with a cron/unbound announce (undefined), requesterMessageIds.size is still 1 and the merged prompt is sent with that message ID, incorrectly replying unrelated content to the user’s inbound message. The aggregate binding should only be kept when every queued item has the same non-empty ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f11505ba4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (wait?.status !== "ok" && wait?.status !== "error") { | ||
| defaultRuntime.log( | ||
| `[warn] Unexpected agent.wait status run=${runId} status=${wait?.status}`, | ||
| ); | ||
| await new Promise((resolve) => setTimeout(resolve, 5_000)); |
There was a problem hiding this comment.
When agent.wait returns a non-ok/error status, this branch always sleeps and retries without checking whether the run is still present in subagentRuns. If the run was already completed/removed by another path (for example the lifecycle listener) and runTimeoutSeconds=0 (no hard deadline), the loop can continue indefinitely and emit warning logs every 5s. Add the same subagentRuns.has(runId) guard here before retrying so orphaned wait loops terminate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6fc860b29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| timeoutMs: timeoutMs + 10_000, | ||
| }); | ||
|
|
||
| if (wait?.status === "timeout") { |
There was a problem hiding this comment.
This branch now unconditionally treats wait.status === "timeout" as a poll timeout and loops, but agent.wait also uses "timeout" as a terminal status for aborted/timed-out runs (see gateway lifecycle snapshot handling), so timed-out subagents no longer reach completeSubagentRun. In practice this can leave completion cleanup/announcements stuck, and with runTimeoutSeconds=0 (no hard deadline) the loop can persist indefinitely instead of finalizing the run.
Useful? React with 👍 / 👎.
| /** Whether remote gateway mode is enabled. */ | ||
| enabled?: boolean; |
There was a problem hiding this comment.
This adds gateway.remote.enabled to the TypeScript config type, but the strict runtime schema for gateway.remote still does not accept enabled, so using this newly-declared option will fail config validation as an unknown key. That makes the advertised field unusable at runtime and introduces a type/runtime contract mismatch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebbf5da2d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| types: [opened, edited, labeled] | ||
| issue_comment: | ||
| types: [created] |
There was a problem hiding this comment.
Removing the pull_request_target labeled trigger means this workflow now runs only on issues and issue_comment, so applying PR labels like dirty, invalid, or r: too-many-prs no longer invokes the moderation paths in this script (which are gated on labeled events). In practice, maintainers can add those labels and nothing happens, so the PR auto-close/comment behavior silently stops.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee4c22006e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const requesterMessageIds = items.map((item) => item.requesterMessageId?.trim()); | ||
| const hasUnboundItem = requesterMessageIds.some((value) => !value); | ||
| const nonEmptyRequesterMessageIds = new Set( | ||
| requesterMessageIds.filter((value): value is string => Boolean(value)), | ||
| ); | ||
| const aggregateRequesterMessageId = | ||
| !hasUnboundItem && nonEmptyRequesterMessageIds.size === 1 |
There was a problem hiding this comment.
In collect mode, aggregateRequesterMessageId is derived only from items, but the final prompt can also include overflowed content via summary (from droppedCount/summaryLines). When the queue cap is exceeded and dropped entries came from a different or unbound requester message, this code can still keep a single retained requesterMessageId and send a mixed-summary batch as a reply to the wrong inbound message. This misthreads unrelated completion content in high-backlog sessions; the binding should be cleared whenever summarized dropped entries are present unless their message IDs are also proven identical.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 414e00bf2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -607,6 +607,7 @@ async function sendAnnounce(item: AnnounceQueueItem) { | |||
| threadId: requesterIsSubagent ? undefined : threadId, | |||
| deliver: !requesterIsSubagent, | |||
| internalEvents: item.internalEvents, | |||
| currentMessageId: item.requesterMessageId, | |||
There was a problem hiding this comment.
sendAnnounce now always forwards item.requesterMessageId as currentMessageId, but non-collect queue modes still emit overflow summaries by reusing one queued item with a synthesized summary prompt. If the dropped items came from a different or unbound requester message, that synthesized summary inherits the surviving item's message ID and is posted as a reply to the wrong inbound message, which misthreads unrelated completion content under backlog conditions (for example mode: "followup" with dropPolicy: "summarize").
Useful? React with 👍 / 👎.
openclaw-barnacle
Bot
added
cli
|
Closing this PR because the proposed continuity hardening is no longer a necessary update in the current codebase direction, and it no longer aligns well with the project's current mainline evolution. If this area becomes a priority again, it should be revisited against the latest upstream baseline rather than continued from this branch. |
1 participant
Summary
Implements continuity hardening proposed in #37188 across three coupled layers:
The goal is to reduce conversational ambiguity in burst chats while keeping single-turn replies clean, and to preserve deterministic delivery context when worker/subagent completion messages are emitted.
What changed
1) Adaptive Telegram reply threading for burst conversations
src/telegram/bot-message.ts.channels.telegram.replyAdaptive:baseWindowMs(default 10s)denseWindowMs(default 20s)veryDenseWindowMs(default 25s)denseShortMinCount(default 2)veryDenseShortMinCount(default 4)shortMessageMaxChars(default 48)scope.private = sender(default)scope.group = chat(default)scope.supergroup = chat(default)replyAdaptive.learning.enabled, default false):emaGapMs+emaShortRatiobase = clamp(emaGapMs * (1 + shortMessageWeight * emaShortRatio), baseMinMs, baseMaxMs)dense = base * denseMultiplierveryDense = base * veryDenseMultiplieralphaGap=0.25,alphaShort=0.2,shortMessageWeight=0.8baseMinMs=6000,baseMaxMs=30000denseMultiplier=2.0,veryDenseMultiplier=2.5src/telegram/bot-message.test.ts:2) Subagent completion reply-target preservation
currentMessageIdsupport through agent gateway request path used by announce delivery.3) Long-run wait observability and queue-path hardening
Files touched (high-level)
src/telegram/bot-message.tssrc/telegram/bot-message.test.tssrc/config/types.telegram.tssrc/config/zod-schema.providers-core.tssrc/agents/subagent-announce.tssrc/agents/subagent-announce-queue.tssrc/agents/subagent-announce.timeout.test.tssrc/agents/subagent-registry.tssrc/agents/subagent-registry.types.tssrc/agents/subagent-spawn.tssrc/agents/tools/sessions-spawn-tool.tssrc/agents/openclaw-tools.tssrc/commands/agent.tssrc/commands/agent/types.tssrc/gateway/protocol/schema/agent.tssrc/gateway/server-methods/agent.tssrc/config/agent-limits.tssrc/process/command-queue.test.tsValidation
Executed:
Result:
Notes