fix(sessions_send): prefer sessionKey over label when both are present (AI-assisted) by RevisitMoon · Pull Request #56203 · openclaw/openclaw

RevisitMoon · 2026-03-28T05:41:22Z

Summary

Problem: sessions_send exposed both sessionKey and label in its tool schema, but hard-failed when both were present.
Why it matters: real tool-call pipelines can redundantly provide both fields, causing cross-session sends to fail even when sessionKey is a precise target.
What changed: sessions_send now prefers sessionKey when both fields are present and only falls back to label resolution when sessionKey is absent.
What did NOT change (scope boundary): label-only targeting still works, and no cross-agent policy / visibility rules were changed.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Related [Bug]: sessions_spawn(runtime="subagent") can receive ACP-only streamTo from the current tool-call bridge #56193
This PR fixes a bug or regression

Root Cause / Regression History (if applicable)

Root cause: sessions_send accepted both sessionKey and label in its schema but treated their coexistence as an error, even though upstream tool-call adapters / hook-based param merging can legitimately surface both.
Missing detection / guardrail: there was no test asserting deterministic behavior when both selectors are present, and no executor-side preference for the less ambiguous selector.
Prior context (git blame, prior PR, issue, or refactor if known): not fully traced to a single introducing change; reviewed current sessions_send implementation plus surrounding tool adapter / before_tool_call paths.
Why this regressed now: real automated tool invocation paths are more tolerant/expansive than the manual “exactly one field” assumption encoded in the executor.
If unknown, what was ruled out: I ruled out an obvious dedicated upstream adapter that explicitly copies sessionKey into label; the failure appears to come from interface fragility rather than one single injector.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file:
- src/agents/tools/sessions.test.ts
Scenario the test should lock in:
- when sessionKey and label are both present, sessions_send should prefer sessionKey, skip sessions.resolve, and complete normally
Why this is the smallest reliable guardrail:
- the behavior lives directly in the executor branching logic, so unit coverage at the tool layer is the narrowest stable check
Existing test that already covers this (if any):
- existing tests covered sessionKey-only and label-only flows, but not coexistence
If no new test is added, why not:
- N/A

User-visible / Behavior Changes

sessions_send no longer errors with Provide either sessionKey or label (not both) when both fields are present.
When both are present, sessionKey wins.
label remains supported as a fallback targeting mechanism when sessionKey is absent.

Diagram (if applicable)

Before:
tool call -> { sessionKey, label } -> sessions_send -> hard error

After:
tool call -> { sessionKey, label } -> sessions_send -> use sessionKey -> send succeeds

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (Yes)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

This changes tool execution behavior for sessions_send conflict handling.
Risk: callers that relied on the previous strict error could now proceed.
Mitigation: the new behavior prefers the more explicit selector (sessionKey) and preserves all existing visibility, agent-to-agent policy, and label-only resolution checks.

Repro + Verification

Environment

OS: Linux (WSL2)
Runtime/container: local OpenClaw install + source checkout
Model/provider: ai/gpt-5.4
Integration/channel (if any): inter-session send from explore to main
Relevant config (redacted): default local setup; no special config required for repro beyond available session tools

Steps

Call sessions_send with both sessionKey and label
Target a valid session such as main
Observe executor behavior before and after patch

Expected

The executor should prefer sessionKey
It should not fail just because label is also present
It should only resolve by label when sessionKey is absent

Actual

Before patch: immediate error Provide either sessionKey or label (not both)
After patch: request proceeds normally and uses sessionKey

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Evidence gathered:

src/agents/tools/sessions.test.ts → 19 passed
src/agents/openclaw-tools.sessions.test.ts → 24 passed
src/gateway/server.sessions-send.test.ts → 2 passed

Runtime validation:

Before patch: sessions_send from explore -> main failed with Provide either sessionKey or label (not both)
After patch:
- timeoutSeconds=10 → reached send/wait flow, returned timeout
- timeoutSeconds=30 → returned status: "ok", reply: "收到2"

This confirms the original parameter-conflict bug is fixed.

Human Verification (required)

Verified scenarios:
- confirmed source change in src/agents/tools/sessions-send-tool.ts
- added focused regression test
- ran targeted tool-layer, integration-layer, and gateway-layer tests successfully
- verified real runtime behavior against the installed OpenClaw instance after rebuilding and deploying patched artifacts
Edge cases checked:
- sessionKey only still works
- label only still works
- sessionKey + label now uses sessionKey
- no regression observed in higher-level session tool test suites
What you did not verify:
- full pnpm check
- full repository pnpm test
- remote deployment / packaged release flow

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:
- N/A

Risks and Mitigations

Risk:
- A caller that intentionally depended on the old hard error for mixed selectors will now proceed instead.
- Mitigation:
  - sessionKey is the more explicit selector, so preferring it is deterministic and safer than failing on redundant input.
Risk:
- There may still be another upstream path that injects redundant params unexpectedly.
- Mitigation:
  - This patch makes the executor robust to that class of redundancy and adds regression coverage.

greptile-apps · 2026-03-28T05:43:15Z

Greptile Summary

This PR removes a strict mutual-exclusion guard in sessions_send that rejected tool calls providing both sessionKey and label, replacing it with a simple "prefer sessionKey" fallback policy. The logic change is minimal (7 lines removed, 2-line comment added) and the downstream cross-agent visibility guard (createSessionVisibilityGuard) is unaffected, so the security posture is preserved.

Key points:

Correctness: The new branching (if (!sessionKey && labelParam)) correctly skips label resolution when sessionKey is supplied, exactly matching the stated intent.
Security: agentId-based cross-agent A2A policy checks inside the label-resolution block are also bypassed when sessionKey is present — this is intentional and safe because sessionKey is already fully-qualified and the visibility guard is still applied unconditionally afterward.
Test coverage: The added test exercises the full agent → agent.wait → chat.history path with both selectors present and asserts that sessions.resolve is never invoked, which is the exact invariant being added.
One minor nit: The comment on line 99 uses "the less ambiguous selector" where "the more explicit selector" would be clearer (see inline comment).

Confidence Score: 5/5

Safe to merge — the fix is correct, the security model is unchanged, and regression coverage is in place.

All findings are P2 (a single wording nit in a comment). The core behavioral change is sound: the label-resolution block is only entered when sessionKey is absent, the cross-agent visibility guard is applied unconditionally, and the new test locks in the preference ordering. No correctness, data-integrity, or security concerns were identified.

No files require special attention.

Important Files Changed

Filename	Overview
src/agents/tools/sessions-send-tool.ts	Removes the hard-fail guard for simultaneous sessionKey + label, instead silently preferring sessionKey. Cross-agent visibility guard (createSessionVisibilityGuard) is still applied on the resolved key, so the security model is intact.
src/agents/tools/sessions.test.ts	Adds a focused regression test covering the sessionKey-wins-over-label path; asserts sessions.resolve is never called and the full send/wait/history flow completes normally.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/agents/tools/sessions-send-tool.ts
Line: 98-99

Comment:
**Minor wording nit in comment**

The phrase `"sessionKey is the less ambiguous selector"` is technically correct English ("less ambiguous" = "more precise"), but it reads a bit awkwardly in context—"the more explicit selector" or "the unambiguous selector" would match the PR description's own phrasing and be immediately clear at a glance.

```suggestion
      // Be tolerant when upstream adapters or model tool-call layers redundantly send both.
      // sessionKey is the more explicit selector, so prefer it and ignore label when present.
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(sessions_send): prefer sessionKey ov..." | Re-trigger Greptile}

greptile-apps · 2026-03-28T05:43:19Z

+      // Be tolerant when upstream adapters or model tool-call layers redundantly send both.
+      // sessionKey is the less ambiguous selector, so prefer it and ignore label when present.


Minor wording nit in comment

The phrase "sessionKey is the less ambiguous selector" is technically correct English ("less ambiguous" = "more precise"), but it reads a bit awkwardly in context—"the more explicit selector" or "the unambiguous selector" would match the PR description's own phrasing and be immediately clear at a glance.

Suggested change

// Be tolerant when upstream adapters or model tool-call layers redundantly send both.

// sessionKey is the less ambiguous selector, so prefer it and ignore label when present.

// Be tolerant when upstream adapters or model tool-call layers redundantly send both.

// sessionKey is the more explicit selector, so prefer it and ignore label when present.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/tools/sessions-send-tool.ts Line: 98-99 Comment: **Minor wording nit in comment** The phrase `"sessionKey is the less ambiguous selector"` is technically correct English ("less ambiguous" = "more precise"), but it reads a bit awkwardly in context—"the more explicit selector" or "the unambiguous selector" would match the PR description's own phrasing and be immediately clear at a glance. ```suggestion // Be tolerant when upstream adapters or model tool-call layers redundantly send both. // sessionKey is the more explicit selector, so prefer it and ignore label when present. ``` How can I resolve this? If you propose a fix, please make it concise.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

EronFan · 2026-03-28T07:14:06Z

This fix directly resolves a critical failure path in multi-agent orchestration workflows where the sessions_send tool is called with both sessionKey and label present (common pattern when adapter/hook layers inject redundant parameters). Without this fix, requests fail immediately with Provide either sessionKey or label (not both) instead of routing via the explicit sessionKey.

Verified against our internal agent coordination pipeline — the change is minimal (+47/-7), semantically correct, and has solid test coverage. A clear shipit from the affected use case.

Requesting maintainer review. Thanks!

openclaw-clownfish · 2026-04-29T09:08:21Z

Thanks @RevisitMoon. I’m closing this as superseded by #39551 because both PRs implement the same sessions_send sessionKey-first behavior, and #39551 is now the maintained canonical contributor branch.

Clownfish is keeping review and validation on #39551. The regression coverage and discussion here remain credited source PR context for the canonical path. If this still covers a distinct reproduction after #39551 lands, please reply and we can reopen or split it back out.

fix(sessions_send): prefer sessionKey over label when both are present

938cf9f

openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Mar 28, 2026

greptile-apps Bot reviewed Mar 28, 2026

View reviewed changes

RevisitMoon and others added 2 commits March 28, 2026 14:15

chore(sessions_send): tighten selector wording in comment

cc24055

Merge branch 'main' into fix/sessions-send-prefer-sessionkey

d6bede7

Merge branch 'main' into fix/sessions-send-prefer-sessionkey

b267a8a

This was referenced Apr 27, 2026

fix: prefer sessionKey over label in sessions_send instead of rejecting both #41255

Closed

fix(agents): prefer sessionKey in sessions_send #59324

Closed

openclaw-clownfish Bot mentioned this pull request Apr 29, 2026

fix(agents): prefer sessionKey in sessions_send #74009

Closed

openclaw-clownfish Bot closed this Apr 29, 2026

clawsweeper Bot mentioned this pull request Apr 29, 2026

[Bug]: sessions_send unexpectedly injects label, causing mutual-exclusion error with sessionKey #64699

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sessions_send): prefer sessionKey over label when both are present (AI-assisted)#56203

fix(sessions_send): prefer sessionKey over label when both are present (AI-assisted)#56203
RevisitMoon wants to merge 4 commits into
openclaw:mainfrom
RevisitMoon:fix/sessions-send-prefer-sessionkey

RevisitMoon commented Mar 28, 2026

Uh oh!

greptile-apps Bot commented Mar 28, 2026

Important Files Changed

Uh oh!

greptile-apps Bot Mar 28, 2026

Uh oh!

EronFan commented Mar 28, 2026

Uh oh!

openclaw-clownfish Bot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Be tolerant when upstream adapters or model tool-call layers redundantly send both.
		// sessionKey is the less ambiguous selector, so prefer it and ignore label when present.

Uh oh!

Conversation

RevisitMoon commented Mar 28, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause / Regression History (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Mar 28, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

greptile-apps Bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

EronFan commented Mar 28, 2026

Uh oh!

openclaw-clownfish Bot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants