fix(heartbeat): inherit delivery context from base session in isolated runs#65702
fix(heartbeat): inherit delivery context from base session in isolated runs#65702hclsys wants to merge 1 commit intoopenclaw:mainfrom
Conversation
…d runs When heartbeat creates an isolated session via resolveCronSession with forceNew: true, the new session entry has deliveryContext, lastThreadId, chatType, and other routing fields cleared (session.ts:80-86). For Telegram topic-scoped sessions, this causes heartbeat responses to route to the group's General topic instead of the correct topic. Fix: after resolveCronSession, copy deliveryContext, lastThreadId, lastChannel, lastTo, lastAccountId, and chatType from the base session entry into the isolated session entry (using ??= so explicit values on the isolated entry are preserved). This ensures the heartbeat response routes to the same channel/thread/topic as the base session. The fix is in heartbeat-runner.ts, NOT in session.ts (which is an AVOID zone with 5+ competing PRs). The approach works by patching the session entry AFTER the cron session resolver clears it, avoiding any contention with other session.ts changes. Fixes openclaw#65693
Greptile SummaryThis PR fixes a routing bug where heartbeat isolated sessions (created via The fix is correctly scoped and avoids the high-contention Confidence Score: 5/5Safe to merge; the fix is narrowly scoped and all remaining findings are style-level (P2). The core logic is correct — No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/infra/heartbeat-runner.ts
Line: 735-741
Comment:
**Variable name shadows outer `entry`**
`entry` here shadows the outer `entry` destructured from `preflight.session` at line 638 (`const { entry, sessionKey, storePath, ... } = preflight.session`). The outer `entry` is still used below (e.g. in `resolveHeartbeatDeliveryTarget`, `restoreHeartbeatUpdatedAt`). Renaming this to `isolatedEntry` eliminates any risk of reader confusion.
```suggestion
const isolatedEntry = cronSession.sessionEntry;
isolatedEntry.deliveryContext ??= baseEntry.deliveryContext;
isolatedEntry.lastThreadId ??= baseEntry.lastThreadId;
isolatedEntry.lastChannel ??= baseEntry.lastChannel;
isolatedEntry.lastTo ??= baseEntry.lastTo;
isolatedEntry.lastAccountId ??= baseEntry.lastAccountId;
isolatedEntry.chatType ??= baseEntry.chatType;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(heartbeat): inherit delivery context..." | Re-trigger Greptile |
| const entry = cronSession.sessionEntry; | ||
| entry.deliveryContext ??= baseEntry.deliveryContext; | ||
| entry.lastThreadId ??= baseEntry.lastThreadId; | ||
| entry.lastChannel ??= baseEntry.lastChannel; | ||
| entry.lastTo ??= baseEntry.lastTo; | ||
| entry.lastAccountId ??= baseEntry.lastAccountId; | ||
| entry.chatType ??= baseEntry.chatType; |
There was a problem hiding this comment.
entry
entry here shadows the outer entry destructured from preflight.session at line 638 (const { entry, sessionKey, storePath, ... } = preflight.session). The outer entry is still used below (e.g. in resolveHeartbeatDeliveryTarget, restoreHeartbeatUpdatedAt). Renaming this to isolatedEntry eliminates any risk of reader confusion.
| const entry = cronSession.sessionEntry; | |
| entry.deliveryContext ??= baseEntry.deliveryContext; | |
| entry.lastThreadId ??= baseEntry.lastThreadId; | |
| entry.lastChannel ??= baseEntry.lastChannel; | |
| entry.lastTo ??= baseEntry.lastTo; | |
| entry.lastAccountId ??= baseEntry.lastAccountId; | |
| entry.chatType ??= baseEntry.chatType; | |
| const isolatedEntry = cronSession.sessionEntry; | |
| isolatedEntry.deliveryContext ??= baseEntry.deliveryContext; | |
| isolatedEntry.lastThreadId ??= baseEntry.lastThreadId; | |
| isolatedEntry.lastChannel ??= baseEntry.lastChannel; | |
| isolatedEntry.lastTo ??= baseEntry.lastTo; | |
| isolatedEntry.lastAccountId ??= baseEntry.lastAccountId; | |
| isolatedEntry.chatType ??= baseEntry.chatType; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/infra/heartbeat-runner.ts
Line: 735-741
Comment:
**Variable name shadows outer `entry`**
`entry` here shadows the outer `entry` destructured from `preflight.session` at line 638 (`const { entry, sessionKey, storePath, ... } = preflight.session`). The outer `entry` is still used below (e.g. in `resolveHeartbeatDeliveryTarget`, `restoreHeartbeatUpdatedAt`). Renaming this to `isolatedEntry` eliminates any risk of reader confusion.
```suggestion
const isolatedEntry = cronSession.sessionEntry;
isolatedEntry.deliveryContext ??= baseEntry.deliveryContext;
isolatedEntry.lastThreadId ??= baseEntry.lastThreadId;
isolatedEntry.lastChannel ??= baseEntry.lastChannel;
isolatedEntry.lastTo ??= baseEntry.lastTo;
isolatedEntry.lastAccountId ??= baseEntry.lastAccountId;
isolatedEntry.chatType ??= baseEntry.chatType;
```
How can I resolve this? If you propose a fix, please make it concise.|
Superseded by #66035, which merged as b42c999. That landed fix addresses the same user-visible bug from the actual ownership seam: heartbeat target resolution in Issue #65693 is now closed by the merged fix. If you can still reproduce the thread/topic routing problem after the merge commit above, reopen or comment with a fresh repro and we can treat that as a remaining gap rather than this original defect family. |
Summary
When heartbeat creates an isolated session via `resolveCronSession(forceNew: true)`, the new session entry has `deliveryContext`, `lastThreadId`, `chatType`, and other routing fields cleared. For Telegram topic-scoped sessions, this causes heartbeat responses to route to the group's General topic instead of the correct topic.
Fixes #65693
Root cause
`resolveCronSession` with `forceNew: true` clears delivery routing fields on the new session entry (`session.ts:80-86`):
```ts
...(isNewSession && {
lastChannel: undefined,
lastTo: undefined,
lastAccountId: undefined,
lastThreadId: undefined,
deliveryContext: undefined,
}),
```
This is correct for most cron jobs (they should start fresh), but heartbeat isolated sessions need to inherit routing from the base session so responses go to the same channel/thread/topic.
Fix
After `resolveCronSession`, copy routing fields from the base session entry:
```ts
const baseEntry = cronSession.store[sessionKey];
if (baseEntry) {
const entry = cronSession.sessionEntry;
entry.deliveryContext ??= baseEntry.deliveryContext;
entry.lastThreadId ??= baseEntry.lastThreadId;
entry.lastChannel ??= baseEntry.lastChannel;
entry.lastTo ??= baseEntry.lastTo;
entry.lastAccountId ??= baseEntry.lastAccountId;
entry.chatType ??= baseEntry.chatType;
}
```
Using `??=` so any explicit values already on the isolated entry are preserved.
Why heartbeat-runner.ts, not session.ts
`session.ts` is a high-contention zone with 5+ competing PRs. Patching the session entry AFTER creation in `heartbeat-runner.ts` avoids contention while achieving the same result.
Scope
Credit to @richardmqq for the exact JSON diff showing the missing `threadId` in #65693.