Skip to content

fix(sessions): reset stale per-channel origin fields on channel switch (#95325)#95332

Closed
ruomuxydt wants to merge 2 commits into
openclaw:mainfrom
ruomuxydt:fix/session-origin-channel-switch
Closed

fix(sessions): reset stale per-channel origin fields on channel switch (#95325)#95332
ruomuxydt wants to merge 2 commits into
openclaw:mainfrom
ruomuxydt:fix/session-origin-channel-switch

Conversation

@ruomuxydt

@ruomuxydt ruomuxydt commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

With session.dmScope: "main", a user's DMs across channels (Slack, Telegram, webchat) share one session. When the channel switches (e.g. Slack → Telegram), the session origin's per-channel identity fields were not reset.

mergeOrigin only overwrites a field when the new inbound supplies it. Telegram DMs do not supply nativeChannelId (or threadId), so the previous Slack channel id persisted in the now-Telegram session and kept persisting on every subsequent Telegram turn until the user messaged on the original channel again.

Changes

  • src/config/sessions/metadata.ts: Detect a provider or surface change in mergeOrigin and reset the channel-specific fields (nativeChannelId, nativeDirectUserId, threadId) before applying the new channel's values. Same-channel turns that keep supplying nativeChannelId continue to update it as before.
  • src/config/sessions/metadata.test.ts: Regression tests covering the channel-switch reset and the preserved-same-channel case.
  • scripts/repro/issue-95325-channel-switch.mts: Real behavior proof harness driving the production deriveSessionMetaPatch.

Real behavior proof

Behavior addressed: Session origin no longer retains the previous channel's nativeChannelId/nativeDirectUserId/threadId after a provider or surface change.

Real environment tested: Windows 10 (10.0.17763), Node.js v22.19.2, source tree at commit 0a031e7.

Exact steps or command run after this patch:

  • node --import tsx scripts/repro/issue-95325-channel-switch.mts

The harness imports the real production deriveSessionMetaPatch and drives a three-turn sequence on a shared dm:shared session key (Slack DM → Telegram DM → Slack DM again), reading back the merged origin after each turn.

Evidence after fix:

PASS: Slack turn seeds origin.nativeChannelId = Dslackdm
Telegram-switch origin: {"label":"telegram-user-id","provider":"telegram","surface":"telegram","chatType":"direct","from":"telegram-user-id","accountId":"telegram-account"}
PASS: channel switch to telegram reset stale nativeChannelId / nativeDirectUserId
PASS: switch back to Slack re-adopts nativeChannelId = Dslackdm

ALL CHECKS PASSED — stale per-channel origin fields reset on channel switch.

Observed result after fix: After the Slack→Telegram switch, the merged origin carries provider: "telegram" and no nativeChannelId/nativeDirectUserId (the stale Slack Dslackdm/Uslackuser are gone). Switching back to Slack re-adopts nativeChannelId: "Dslackdm", proving the reset only fires on a real channel change and does not block later same-channel re-adoption.

What was not tested: Live Slack↔Telegram round-trip over real channels (requires live credentials). The harness drives the real runtime merge path (deriveSessionMetaPatchmergeOrigin), which is the single function the runtime uses to update session origin on each inbound turn.

Verification

  • node scripts/run-vitest.mjs run src/config/sessions/metadata.test.ts: 4/4 passing.
  • node --import tsx scripts/repro/issue-95325-channel-switch.mts: all checks pass (real production function).
  • tsgo -p tsconfig.core.json: no errors in the modified file.
  • oxlint on modified files: clean.
  • Pre-existing sessionFile path tests (2) fail identically on base origin/main without this change — unrelated to origin merging.

Fixes #95325

When session.dmScope:"main" shares one session across channels, switching
channels (e.g. Slack -> Telegram) left the prior channel's nativeChannelId,
nativeDirectUserId, and threadId in the session origin. Channels like Telegram
DM do not carry a nativeChannelId, so mergeOrigin never overwrote the stale
Slack value, which then persisted for the entire new-channel session and broke
native-channel-keyed operations (reactions, native threading).

Detect a provider or surface change in mergeOrigin and reset the channel-
specific fields before applying the new channel's values. Same-channel turns
that keep supplying nativeChannelId continue to update it as before.

Fixes openclaw#95325
@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 20, 2026, 3:01 PM ET / 19:01 UTC.

Summary
The PR changes mergeOrigin to clear nativeChannelId, nativeDirectUserId, and threadId when provider or surface changes, and adds regression tests plus a terminal repro harness.

PR surface: Source +13, Tests +116, Other +93. Total +222 across 3 files.

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

Review metrics: 2 noteworthy metrics.

  • Origin fields cleared: 3 cleared on provider/surface change. This is a persisted session-origin semantics change, so maintainers should notice the exact field set before merge.
  • Same-root candidate PRs: 2 open, 1 closed unmerged. Multiple branches target the linked stale-origin bug, so canonical selection matters before landing.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/metadata.test.ts, unknown-data-model-change: src/config/sessions/metadata.test.ts, vector/embedding metadata: scripts/repro/issue-95325-channel-switch.mts, vector/embedding metadata: src/config/sessions/metadata.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95325
Summary: This PR is a candidate fix for the canonical stale session-origin bug; one sibling candidate remains open and one earlier sibling is closed unmerged as superseded.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The PR intentionally changes persisted session-origin semantics by clearing three channel-keyed fields on provider or surface change; maintainers should confirm that exact field set before merge.
  • [P1] Another open candidate PR, fix(sessions): reset stale per-channel origin fields on channel switch #95328, targets the same root bug, so maintainers should land only one canonical branch to avoid duplicate tests or divergent session metadata behavior.
  • [P1] The contributor proof exercises the production metadata helper in a terminal harness, but no live Slack-to-Telegram transport run was performed.

Maintainer options:

  1. Accept This Canonical Contract (recommended)
    Maintainers can accept clearing exactly nativeChannelId, nativeDirectUserId, and threadId on provider or surface changes, merge this branch after normal CI, and close sibling candidate PRs.
  2. Choose The Sibling Instead
    If maintainers prefer the sibling implementation or its broader proof shape, pause or close this PR after linking the chosen canonical branch.
  3. Request Live Cross-Channel Proof
    Maintainers can ask for a redacted live or equivalent Slack-to-Telegram shared-session inspection before accepting the session-state risk.

Next step before merge

  • [P2] Maintainers need to choose the canonical same-root PR and confirm the session-origin clearing contract; there is no narrow automated repair blocker in this branch.

Security
Cleared: The diff changes session metadata logic, focused tests, and a local repro script; no dependency, workflow, credential, package, lockfile, install, or supply-chain concern was found.

Review details

Best possible solution:

Land one canonical session-origin fix that clears stale channel-keyed origin fields on provider or surface switches while preserving same-channel sparse metadata, then close the linked issue and supersede duplicate candidate PRs.

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-message 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?

Yes as a code shape: mergeOrigin is the shared metadata boundary and the PR preserves same-channel sparse updates. The remaining merge decision is the canonical PR and exact field-clearing contract, not a concrete patch defect.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3a14f247ad1b.

Label changes

Label justifications:

  • P2: The PR addresses a normal-priority session metadata bug with limited blast radius and no evidence that primary message delivery is broken.
  • merge-risk: 🚨 session-state: Merging changes how persisted session-origin metadata is cleared when a shared session moves between provider or surface identities.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a Windows Node run of a harness importing production deriveSessionMetaPatch and showing Slack-to-Telegram stale fields clearing and Slack re-adoption.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a Windows Node run of a harness importing production deriveSessionMetaPatch and showing Slack-to-Telegram stale fields clearing and Slack re-adoption.
Evidence reviewed

PR surface:

Source +13, Tests +116, Other +93. Total +222 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 13 0 +13
Tests 1 116 0 +116
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 93 0 +93
Total 3 222 0 +222

What I checked:

  • Repository policy applied: Root, scripts, channels, and extensions AGENTS.md guidance were read; session-state compatibility guidance and scripts wrapper guidance apply to this review. (AGENTS.md:14, 3a14f247ad1b)
  • Telegram maintainer note checked: The Telegram note says live Telegram proof is needed for transport, streaming, topics, callbacks, authorization, or reply context; this PR is a core metadata merge change, so terminal proof of the production helper is relevant. (.agents/maintainer-notes/telegram.md:1, 3a14f247ad1b)
  • Current main preserves sparse origin fields: Current mergeOrigin copies the existing origin and only overwrites native channel fields when the next origin supplies them, so stale omitted fields can persist. (src/config/sessions/metadata.ts:22, 3a14f247ad1b)
  • PR changes the shared merge boundary: The PR adds a provider/surface change guard in mergeOrigin, deletes the three channel-keyed fields, and then allows newly supplied values to be adopted in the same call. (src/config/sessions/metadata.ts:38, 0a031e71dc0d)
  • Sparse next origin is possible: deriveSessionOrigin only adds NativeChannelId, NativeDirectUserId, and MessageThreadId when the inbound MsgContext supplies those values. (src/config/sessions/metadata.ts:78, 3a14f247ad1b)
  • Reply caller uses metadata helper: The main reply session path calls deriveSessionMetaPatch with the existing session entry before updating the session entry. (src/auto-reply/reply/session.ts:710, 3a14f247ad1b)

Likely related people:

  • steipete: Commit 5f22b68268b9 introduced the session origin metadata helper that owns mergeOrigin. (role: introduced behavior; confidence: high; commits: 5f22b68268b9; files: src/config/sessions/metadata.ts)
  • gumadeiras: Commit dcd0cf9f98a5 added native channel identity semantics across session metadata, session types, and inbound templating. (role: adjacent routing contributor; confidence: medium; commits: dcd0cf9f98a5; files: src/config/sessions/metadata.ts, src/config/sessions/types.ts, src/auto-reply/templating.ts)
  • mbelinky: Merged PR Gateway/sessions: preserve shared session route on system events #66073 touched shared session-route preservation and metadata.ts, near the durable session behavior this PR changes. (role: adjacent session-route contributor; confidence: medium; commits: 527895f0362f; files: src/config/sessions/metadata.ts, src/auto-reply/reply/session.ts)
  • vincentkoc: Current checkout blame attributes the present mergeOrigin, caller, and context-builder lines to commit 7fc5a72433d1; this appears to be a recent main snapshot/carry-forward commit rather than the original feature introduction. (role: recent line owner; confidence: low; commits: 7fc5a72433d1; files: src/config/sessions/metadata.ts, src/auto-reply/reply/session.ts, src/channels/inbound-event/context.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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
@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: M and removed size: S labels Jun 20, 2026
@ruomuxydt

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

  • Added a real behavior proof harness (scripts/repro/issue-95325-channel-switch.mts) that drives the production deriveSessionMetaPatch through a Slack→Telegram→Slack sequence and reads back the merged origin.
  • Output confirms stale nativeChannelId/nativeDirectUserId are reset on the Telegram switch and re-adopted on switch back to Slack.

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session origin retains stale per-channel fields (nativeChannelId, threadId) across a channel switch in a dmScope:"main" session

3 participants