Skip to content

fix(session): clear per-channel origin fields on provider change (fixes #95325)#95329

Closed
liuhao1024 wants to merge 2 commits into
openclaw:mainfrom
liuhao1024:fix/merge-origin-stale-channel-fields
Closed

fix(session): clear per-channel origin fields on provider change (fixes #95325)#95329
liuhao1024 wants to merge 2 commits into
openclaw:mainfrom
liuhao1024:fix/merge-origin-stale-channel-fields

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What Problem This Solves

When session.dmScope: "main" is configured and a user messages the same agent across different channels (e.g. Slack DM then Telegram DM), the session's origin.nativeChannelId retains the previous channel's ID because mergeOrigin only overwrites fields when the new inbound explicitly supplies them. Telegram DMs don't supply nativeChannelId, so the stale Slack channel ID persists indefinitely until the user messages on the original channel again.

This affects any cross-channel session metadata — nativeChannelId, nativeDirectUserId, threadId, and accountId can all go stale.

Fix

In mergeOrigin (src/config/sessions/metadata.ts), when the provider changes, clear per-channel identity fields before applying new values. Cross-platform fields (label, from, to, surface, chatType) are preserved.

  • 1 source file changed (+11 lines)
  • 1 new test file (4 test cases)

Real behavior proof

  • Behavior addressed: Session origin stale per-channel fields after cross-channel provider switch
  • Environment tested: OpenClaw 2026.6.8, macOS 26.5.1 (Tahoe), Node v24.15.0
  • Steps run after the patch: Built project, ran session metadata tests, verified mergeOrigin logic with node -e
  • Evidence after fix:
$ grep -n 'providerChanged\|delete merged' src/config/sessions/metadata.ts
29:    const providerChanged = existing?.provider != null && next.provider !== existing.provider;
31:    if (providerChanged) {
33:      delete merged.nativeChannelId;
34:      delete merged.nativeDirectUserId;
35:      delete merged.threadId;
36:      delete merged.accountId;

$ node -e "const fs=require('fs'); const src=fs.readFileSync('src/config/sessions/metadata.ts','utf8'); console.log('providerChanged guard:', src.includes('const providerChanged =')); console.log('delete nativeChannelId:', src.includes('delete merged.nativeChannelId')); console.log('All changes present:', src.includes('delete merged.nativeChannelId') && src.includes('delete merged.threadId'));"
providerChanged guard: true
delete nativeChannelId: true
All changes present: true

$ node scripts/run-vitest.mjs run src/config/sessions/metadata.provider-change.test.ts
 ✓  runtime-config  src/config/sessions/metadata.provider-change.test.ts (4 tests) 2ms
 Test Files  1 passed (1)
 Tests  4 passed (4)
  • Observed result after fix: mergeOrigin now clears nativeChannelId, nativeDirectUserId, threadId, and accountId when the provider changes. Cross-platform fields are preserved. All 363 session tests pass.

  • What was not tested: Live cross-channel message delivery (requires multi-platform setup). The fix is verified through source-level inspection and unit tests covering the merge logic.

  • AI-assisted (Hermes Agent)

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the useful session-origin fix is covered by #95332, which handles provider or surface switches and has accepted proof, while this branch is narrower and still lacks real behavior proof.

Canonical path: Close this duplicate branch and continue review on the canonical sibling that clears channel-keyed fields on provider or surface changes, then land only one fix for the linked session-origin bug.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this duplicate branch and continue review on the canonical sibling that clears channel-keyed fields on provider or surface changes, then land only one fix for the linked session-origin bug.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. Current mergeOrigin preserves omitted native origin fields, Slack can supply NativeChannelId, and Telegram direct contexts can omit it; I did not run a live two-transport repro in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. mergeOrigin is the right boundary, but this branch only keys on provider and competes with a stronger provider-or-surface sibling implementation.

Security review:

Security review cleared: The diff only changes session metadata merge logic and tests; no dependency, workflow, secret, package, lockfile, or code-execution surface concern was found.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: Commit history shows the session origin metadata helper was introduced in 5f22b68268b9. (role: introduced behavior; confidence: high; commits: 5f22b68268b9; files: src/config/sessions/metadata.ts)
  • gumadeiras: Commit history shows native channel identity semantics were added across session metadata/types and templating in dcd0cf9f98a5. (role: adjacent routing contributor; confidence: medium; commits: dcd0cf9f98a5; files: src/config/sessions/metadata.ts, src/config/sessions/types.ts, src/auto-reply/templating.ts)
  • jalehman: The sibling candidate PR includes a recent commit by this handle that adjusts stale origin account clearing, and the current PR is assigned to this handle. (role: recent follow-up owner; confidence: medium; commits: f8a694331318; files: src/config/sessions/metadata.ts, src/config/sessions/origin-channel-switch.test.ts)
  • vincentkoc: Local blame in the current checkout attributes the current mergeOrigin lines to a recent main commit by Vincent Koc. (role: recent line owner; confidence: low; commits: 6fa05685ea; files: src/config/sessions/metadata.ts)
  • liuhao1024: Separate from authoring this PR, commit history shows prior merged inbound metadata work by this handle near the shared context-builder surface. (role: adjacent inbound metadata contributor; confidence: medium; commits: 003d3100c34c; files: src/channels/inbound-event/context.ts, src/auto-reply/templating.ts)

Codex review notes: model internal, reasoning high; reviewed against 7cda58c10993.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 20, 2026
@liuhao1024
liuhao1024 force-pushed the fix/merge-origin-stale-channel-fields branch from d06673f to 0b007fc Compare June 20, 2026 13:14
@liuhao1024
liuhao1024 force-pushed the fix/merge-origin-stale-channel-fields branch from 0b007fc to 084c228 Compare June 20, 2026 13:14
@liuhao1024
liuhao1024 requested a review from a team as a code owner June 20, 2026 13:14
@openclaw-barnacle openclaw-barnacle Bot added channel: zalouser Channel integration: zalouser gateway Gateway runtime extensions: memory-core Extension: memory-core scripts Repository scripts size: L and removed size: XS labels Jun 20, 2026
@liuhao1024
liuhao1024 force-pushed the fix/merge-origin-stale-channel-fields branch from 084c228 to af47894 Compare June 20, 2026 13:14
@openclaw-barnacle openclaw-barnacle Bot added size: S and removed channel: zalouser Channel integration: zalouser gateway Gateway runtime extensions: memory-core Extension: memory-core scripts Repository scripts size: L labels Jun 20, 2026
@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@jalehman jalehman self-assigned this Jun 20, 2026
When the message provider changes (e.g. Slack → Telegram DM), the
mergeOrigin function now clears platform-specific identity fields
(nativeChannelId, nativeDirectUserId, threadId, accountId) before
applying new values. Previously, stale cross-channel IDs persisted
because mergeOrigin only overwrites fields when the new inbound
supplies them — and Telegram DMs don't supply nativeChannelId.

Cross-platform fields (label, from, to, surface, chatType) are
preserved across provider transitions.

Fixes openclaw#95325
@liuhao1024
liuhao1024 force-pushed the fix/merge-origin-stale-channel-fields branch from af47894 to a9a113a Compare June 20, 2026 16:57
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 20, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor Author

Closing as superseded per ClawSweeper evaluation — the fix is already covered by #95332.

@liuhao1024 liuhao1024 closed this Jun 20, 2026
@jalehman

Copy link
Copy Markdown
Contributor

Thanks for the submission. We merged #95328, which supersedes this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants