fix(gateway): exclude heartbeat sender ID from session display name#66544
fix(gateway): exclude heartbeat sender ID from session display name#66544suboss87 wants to merge 2 commits intoopenclaw:mainfrom
Conversation
|
@vincentkoc @mbelinky small display fix. The heartbeat fallback sender ID leaks into the webchat session dropdown label. 2-line filter in buildGatewaySessionRow. |
Greptile SummaryFilters the internal Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/session-utils.ts
Line: 1134
Comment:
**Magic string coupled to sentinel in `targets.ts`**
The literal `"heartbeat"` here is the fallback sentinel returned by `resolveHeartbeatSenderId` in `src/infra/outbound/targets.ts` (lines 339 and 353: `return candidates[0] ?? "heartbeat"`). These two sites are not linked by a constant, so a rename of the sentinel in `targets.ts` would silently re-introduce the bug. The comment's "e.g." framing also hints that other internal IDs (`"cron-event"` etc.) might need the same treatment eventually. Consider extracting a shared constant or a small predicate:
```ts
// In targets.ts (or a shared constants file):
export const HEARTBEAT_FALLBACK_SENDER_ID = "heartbeat";
// In session-utils.ts:
const originLabel =
origin?.label && origin.label !== HEARTBEAT_FALLBACK_SENDER_ID
? origin.label
: undefined;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(gateway): exclude heartbeat sender I..." | Re-trigger Greptile |
| const originLabel = origin?.label; | ||
| // Exclude internal system sender IDs (e.g. "heartbeat") from the display | ||
| // name fallback chain so the webchat session selector stays meaningful. | ||
| const originLabel = origin?.label && origin.label !== "heartbeat" ? origin.label : undefined; |
There was a problem hiding this comment.
targets.ts
The literal "heartbeat" here is the fallback sentinel returned by resolveHeartbeatSenderId in src/infra/outbound/targets.ts (lines 339 and 353: return candidates[0] ?? "heartbeat"). These two sites are not linked by a constant, so a rename of the sentinel in targets.ts would silently re-introduce the bug. The comment's "e.g." framing also hints that other internal IDs ("cron-event" etc.) might need the same treatment eventually. Consider extracting a shared constant or a small predicate:
// In targets.ts (or a shared constants file):
export const HEARTBEAT_FALLBACK_SENDER_ID = "heartbeat";
// In session-utils.ts:
const originLabel =
origin?.label && origin.label !== HEARTBEAT_FALLBACK_SENDER_ID
? origin.label
: undefined;Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/session-utils.ts
Line: 1134
Comment:
**Magic string coupled to sentinel in `targets.ts`**
The literal `"heartbeat"` here is the fallback sentinel returned by `resolveHeartbeatSenderId` in `src/infra/outbound/targets.ts` (lines 339 and 353: `return candidates[0] ?? "heartbeat"`). These two sites are not linked by a constant, so a rename of the sentinel in `targets.ts` would silently re-introduce the bug. The comment's "e.g." framing also hints that other internal IDs (`"cron-event"` etc.) might need the same treatment eventually. Consider extracting a shared constant or a small predicate:
```ts
// In targets.ts (or a shared constants file):
export const HEARTBEAT_FALLBACK_SENDER_ID = "heartbeat";
// In session-utils.ts:
const originLabel =
origin?.label && origin.label !== HEARTBEAT_FALLBACK_SENDER_ID
? origin.label
: undefined;
```
How can I resolve this? If you propose a fix, please make it concise.cb6a738 to
2d9bdc3
Compare
|
Addressed the Greptile P2 about the magic string. Replaced the inline |
The heartbeat delivery path uses "heartbeat" as a fallback sender ID (resolveHeartbeatSenderId). This flows into origin.label on the session entry, and buildGatewaySessionRow uses it as the last-resort display name. Over time this causes the webchat session selector to show "heartbeat" instead of the actual session label. Filter out the internal "heartbeat" placeholder from the originLabel fallback so the session dropdown stays meaningful. Closes openclaw#66533
cc6c53e to
bb3bc39
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb3bc39526
ℹ️ 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".
| // Matches the fallback sender ID in resolveHeartbeatSenderId (targets.ts). | ||
| const HEARTBEAT_FALLBACK_SENDER = "heartbeat"; | ||
| const originLabel = | ||
| origin?.label && origin.label !== HEARTBEAT_FALLBACK_SENDER ? origin.label : undefined; |
There was a problem hiding this comment.
Scope heartbeat-label suppression to synthetic events
This condition suppresses any origin.label === "heartbeat" regardless of where the label came from, but origin.label can also be a real conversation name (for direct chats it is derived from sender name/ID). In sessions that have no displayName or manual label, a legitimate contact/chat named "heartbeat" will now lose its human-readable name and fall back to a generic key-derived label. Filter by synthetic heartbeat provenance (for example system-event/provider context) instead of a global string match.
Useful? React with 👍 / 👎.
|
Related work from PRtags group Title: Open PR duplicate: [Bug]: WebChat session selector shows main session as “heartbeat” after a while, making /new fe…
|
|
@vincentkoc @mbelinky quick bump — 8 days since the last review, CI is green, Greptile P2 addressed (named constant). Two-line display fix, no logic change. Happy to split or adjust if anything looks off. |
|
Codex review: needs changes before merge. Summary Reproducibility: yes. as a source-level reproduction: a session entry with no Next step before merge Security Review findings
Review detailsBest possible solution: Land a scoped Gateway row-projection fix that ignores only synthetic or stale heartbeat origin metadata, preserves real labels named Do we have a high-confidence way to reproduce the issue? Yes, as a source-level reproduction: a session entry with no Is this the best way to solve the issue? No. The shared row projection is the right surface, but a global string filter is overbroad; the safer fix suppresses only known synthetic or stale heartbeat origin metadata while preserving explicit and human-derived labels. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against a4c1c28a1731. |
Summary
buildGatewaySessionRoworigin.label, which then becomes the session dropdown label in webchatRoot Cause
resolveHeartbeatSenderId(intargets.ts) returns "heartbeat" as a fallback when no real sender can be resolved. This ends up in the session entry'sorigin.label. WhenbuildGatewaySessionRowcomputesdisplayName, it falls throughentry.displayName -> groupName -> entry.label -> originLabel, and if all the earlier values are undefined, "heartbeat" becomes the visible session label in the webchat dropdown.Fix
2-line change: check if
origin.labelis "heartbeat" before using it asoriginLabelin the display name fallback chain.Closes #66533