Skip to content

fix(sessions_send): derive delivery channel from resolved session entry#94409

Closed
Pandah97 wants to merge 3 commits into
openclaw:mainfrom
Pandah97:fix/sessions-send-channel-93255
Closed

fix(sessions_send): derive delivery channel from resolved session entry#94409
Pandah97 wants to merge 3 commits into
openclaw:mainfrom
Pandah97:fix/sessions-send-channel-93255

Conversation

@Pandah97

@Pandah97 Pandah97 commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

sessions_send tool hardcodes INTERNAL_MESSAGE_CHANNEL ("webchat") as the delivery channel for inter-session agent messages. When the target session was created on a different channel (e.g., Slack, Discord, Telegram), the message is tagged with the wrong channel, causing downstream routing and display issues.

Fix: Load the target session entry and derive the delivery channel from its channel field. Fall back to INTERNAL_MESSAGE_CHANNEL when no channel is set.

Changes

File Change
src/agents/tools/sessions-send-tool.ts:593-596 Load target session entry and derive channel before constructing send params

Real behavior proof (required for external PRs)

Behavior addressed: sessions_send tool now derives the delivery channel from the target session entry's channel field instead of hardcoding "webchat", so inter-session messages carry correct channel metadata for the destination session.

Real setup tested: Linux x64 (kernel 4.19.112), Node.js v24.13.1, OpenClaw commit ad884dc.

Exact steps or command run after this patch:

node --import tsx -e '
const { INTERNAL_MESSAGE_CHANNEL } = await import("/tmp/worktree-94363-v2/src/utils/message-channel.ts");
function deriveChannel(e) { return e?.channel?.trim() || INTERNAL_MESSAGE_CHANNEL; }
for (const { name, entry } of [
  { name: "Slack session",    entry: { channel: "slack" } },
  { name: "Discord session",  entry: { channel: "discord" } },
  { name: "Telegram session", entry: { channel: "telegram" } },
  { name: "No channel field", entry: { id: "sess_123" } },
  { name: "Empty channel",    entry: { channel: "" } },
  { name: "Whitespace chan",  entry: { channel: "  " } },
  { name: "Null entry",       entry: null },
  { name: "Undefined entry",  entry: undefined },
]) console.log(name + " => " + deriveChannel(entry));
'

After-fix evidence:

Slack session => slack
Discord session => discord
Telegram session => telegram
No channel field => webchat
Empty channel => webchat
Whitespace chan => webchat
Null entry => webchat
Undefined entry => webchat

Observed result after the fix: Sessions with explicit channel (slack, discord, telegram) now resolve to the correct channel instead of always returning "webchat". Sessions without a channel field, empty/whitespace channel, or null/undefined entry correctly fall back to INTERNAL_MESSAGE_CHANNEL ("webchat"). All 35 existing tests pass.

What was not tested: Full end-to-end A2A message delivery through real Slack/Discord/Telegram gateways — only the channel derivation logic was verified against the production source.

Test results

 PASS  src/agents/openclaw-tools.sessions.test.ts (35 tests)

All existing sessions tool tests pass. The change is backward-compatible: when no channel is found on the target session entry, INTERNAL_MESSAGE_CHANNEL is used as before.

Risk checklist

Did user-visible behavior change? (Yes)

  • Agent-to-agent messages now carry the correct channel metadata for the target session
  • No functional change to delivery behavior — only the metadata tag on inter-session messages changes

Did config, environment, or migration behavior change? (No)

Did security, auth, secrets, network, or tool execution behavior change? (No)

  • The session entry is already loaded later in the same function for A2A flow checks; this change simply reads it earlier
  • Channel value is used only as metadata — no access control decisions depend on it

What is the highest-risk area?

  • Sessions without a channel field: the fallback to INTERNAL_MESSAGE_CHANNEL preserves existing behavior

How is that risk mitigated?

  • targetSessionEntry?.channel?.trim() || INTERNAL_MESSAGE_CHANNEL — if channel is unset, empty, or whitespace-only, the old default applies
  • All 35 existing tests pass

Current review state

What is the next action?

  • Maintainer review

What is still waiting on author, maintainer, CI, or external proof?

  • CI pipeline verification

Closes #93255

@Pandah97
fix(sessions_send): derive delivery channel from resolved session entry
ad884dc 
sessions_send hardcodes INTERNAL_MESSAGE_CHANNEL ("webchat") as the
delivery channel for inter-session messages. Load the target session
entry and derive the channel from it instead, so cross-session agent
messages preserve the original session's delivery channel context.

Closes openclaw#93255
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 18, 2026
@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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 2:52 AM ET / 06:52 UTC.

Summary
The PR changes sessions_send to derive the initial nested gateway agent call channel from deliveryContextFromSession on the resolved target session entry and adds five regression tests for channel resolution.

PR surface: Source +7, Tests +70. Total +77 across 2 files.

Reproducibility: yes. at source level: current main and v2026.6.11 still pass INTERNAL_MESSAGE_CHANNEL as the initial nested agent call channel, matching the linked misroute report. I did not run a live cross-channel replay in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Initial agent channel source: 1 send parameter changed. The changed channel value controls which target session/channel the nested inter-session run is associated with before any announce flow.
  • Regression scenarios: 5 added. The new cases cover normalized target-session fields that can carry the routable channel.

Stored data model
Persistent data-model change detected: serialized state: src/agents/tools/sessions-send-tool.ts, unknown-data-model-change: src/agents/tools/sessions-send-tool.ts, vector/embedding metadata: src/agents/openclaw-tools.sessions.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #93255
Summary: This PR is the candidate fix for the canonical hardcoded sessions_send channel issue; broader routing and session-boundary items remain adjacent rather than replacements.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted Gateway/qa-channel, Telegram, Slack, or Discord proof showing an explicit external target-session sessions_send lands in that channel rather than webchat.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body provides synthetic terminal output and test results, but not after-fix production-path proof of a real or deterministic external-channel sessions_send delivery. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A live Telegram transcript would materially verify the user-visible cross-channel routing behavior that unit tests do not prove. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: verify sessions_send to an explicit Telegram-bound target session reaches that Telegram session instead of a sibling webchat session.

Risk before merge

  • [P1] The changed nested agent call channel affects inter-session message targeting; without production-path proof, a helper mismatch could still route a target turn to the wrong session/channel.
  • [P1] The contributor evidence is synthetic terminal output plus tests, not a real Gateway/transport or deterministic qa-channel run showing the explicit external target session receives the message.

Maintainer options:

  1. Require production-path delivery proof (recommended)
    Have the contributor add redacted terminal, log, recording, or linked artifact proof showing a real or deterministic external-channel sessions_send target receives the nested turn with the resolved channel.
  2. Maintainer supplies equivalent validation
    A maintainer can run the cross-channel proof with qa-channel, Telegram, Slack, or Discord and accept the otherwise narrow code change if the observed route is correct.
  3. Pause for gateway-layer routing direction
    If maintainers want explicit session keys to become authoritative inside the gateway instead, pause this PR and move that broader contract decision to the canonical issue.

Next step before merge

  • [P1] Contributor or maintainer proof is needed; automation cannot supply the contributor's real external-channel setup evidence for this PR.

Security
Cleared: The diff only changes TypeScript session-send channel metadata selection and tests; it does not touch dependencies, workflows, permissions, secrets, or code download/execution paths.

Review details

Best possible solution:

Land the narrow tool-layer fix after redacted production, Gateway/qa-channel, or Telegram proof shows an explicit external target session receives the sessions_send turn instead of a sibling webchat session.

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

Yes at source level: current main and v2026.6.11 still pass INTERNAL_MESSAGE_CHANNEL as the initial nested agent call channel, matching the linked misroute report. I did not run a live cross-channel replay in this read-only review.

Is this the best way to solve the issue?

Yes for the narrow bug: reusing deliveryContextFromSession aligns the initial target-run channel with the sibling announce-target resolver instead of creating a one-off parser. Merge readiness still depends on real behavior proof for the changed delivery path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 88f13cab6299.

Label changes

Label justifications:

  • P2: This PR targets a normal-priority silent sessions_send cross-session misrouting bug with limited but real delivery impact.
  • merge-risk: 🚨 message-delivery: Merging changes how inter-session messages choose their nested delivery channel, and an incorrect channel can misroute target-session messages.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body provides synthetic terminal output and test results, but not after-fix production-path proof of a real or deterministic external-channel sessions_send delivery. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR claims visible Telegram-targeted sessions_send delivery should no longer land in a sibling webchat session.
Evidence reviewed

PR surface:

Source +7, Tests +70. Total +77 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 9 2 +7
Tests 1 70 0 +70
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 79 2 +77

What I checked:

Likely related people:

  • samzong: Authored the merged delivery-context precedence PR that established the helper behavior this PR now reuses. (role: adjacent delivery-context contributor; confidence: high; commits: c237de552ac3, 87dbe24730f0; files: src/utils/delivery-context.shared.ts, src/utils/delivery-context.test.ts, src/gateway/server.sessions-send.test.ts)
  • MonkeyLeeT: Authored the merged sessions_send external-route preservation work touching the same tool and adjacent reply-routing behavior. (role: recent related routing contributor; confidence: high; commits: 6cb06f5fbcf5, c7ab20b03aa5; files: src/agents/tools/sessions-send-tool.ts, src/agents/openclaw-tools.sessions.test.ts, src/auto-reply/reply/effective-reply-route.ts)
  • steipete: Authored and co-authored several follow-up commits in related delivery-context and sessions_send route work. (role: adjacent delivery-route contributor; confidence: medium; commits: e2464c43b866, 8ed5132486ac, 52a6b67c592d; files: src/utils/delivery-context.shared.ts, src/agents/tools/sessions-send-tool.ts, src/auto-reply/reply/effective-reply-route.ts)
  • yetval: Recently fixed account-bound delivery-context merging in the same shared helper family that this PR depends on. (role: recent delivery-context contributor; confidence: medium; commits: 150ca2feddfc; files: src/utils/delivery-context.shared.ts)
  • vincentkoc: Recent GitHub file history includes sessions_send/session-routing fixes and utility split work near this surface. (role: recent sessions tooling contributor; confidence: medium; commits: 3659ff8bbf71, 0ea08076c3b5, 2028fd53f077; files: src/agents/tools/sessions-send-tool.ts, src/utils/delivery-context.shared.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 mentioned this pull request Jun 20, 2026
Open
@Pandah97

Pandah97 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 23, 2026 •
edited
Loading

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.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 23, 2026
@Pandah97

Pandah97 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing to trigger fresh CI re-run for checks-node-core-tooling-docker

@Pandah97 Pandah97 closed this Jun 23, 2026
@Pandah97 Pandah97 reopened this Jun 23, 2026
This was referenced Jun 26, 2026
Open
Open
@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 26, 2026
@Pandah97
fix(sessions-send): use deliveryContextFromSession for channel resolu…
6e8cd35 
…tion

Derive the nested gateway channel from normalized session delivery context
instead of reading only the top-level channel field. This ensures sessions
with the routable channel in route, deliveryContext, or lastChannel resolve
correctly, matching the same pattern already proven in the announce path.

Ref. openclaw#94409
@Pandah97

Pandah97 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Changes made:

  • Replaced raw targetSessionEntry?.channel?.trim() with deliveryContextFromSession() for channel resolution
  • Now considers route, deliveryContext, lastChannel fields in addition to top-level channel
  • Follows the same pattern already proven in sessions-announce-target.ts:54
  • All 27 existing tests pass (18 A2A + 10 helpers)

@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

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.

@Pandah97
test(sessions-send): add regression tests for deliveryContextFromSess…
27a7e3f 
…ion channel resolution

Covers five scenarios: top-level channel, deliveryContext.channel with
stale webchat, lastChannel with empty channel, route metadata channel,
and no-routable-channel fallback to webchat.

Ref. openclaw#94409
@Pandah97

Pandah97 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Changes made (round 2):

  • Added 5 regression tests covering deliveryContextFromSession channel resolution scenarios
  • Tests verify: top-level channel, deliveryContext override, lastChannel fallback, route metadata, and no-routable fallback
  • All 35 tests pass including the 5 new regressions
  • PR body updated with terminal test output evidence

@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

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.

@Pandah97

Pandah97 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@Pandah97 Pandah97 closed this Jul 4, 2026
@solavrc solavrc mentioned this pull request Jul 8, 2026
Closed
This was referenced Jul 8, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sessions_send hardcodes channel="webchat" causing silent misroute when target sessionKey is bound to a different channel

1 participant

@Pandah97