Skip to content

fix: prevent A2A target from reverse-calling sessions_send to requester (#39476)#91161

Closed
sumitaich1998 wants to merge 1 commit into
openclaw:mainfrom
sumitaich1998:fix/issue-39476-a2a-reverse-send
Closed

fix: prevent A2A target from reverse-calling sessions_send to requester (#39476)#91161
sumitaich1998 wants to merge 1 commit into
openclaw:mainfrom
sumitaich1998:fix/issue-39476-a2a-reverse-send

Conversation

@sumitaich1998

@sumitaich1998 sumitaich1998 commented Jun 7, 2026

Copy link
Copy Markdown

Summary

A2A sessions_send could post duplicate messages in the requester's channel. When Agent A calls sessions_send(agentId: "agentB", ...), Agent B's reply already flows back to A through the sessions_send tool result (and the announce flow). But because the A2A-routed turn still exposed the sessions_send tool to Agent B, B could reverse-call sessions_send back to A during the same turn, so A received the same content twice.

This change removes sessions_send from any turn that is itself processing an inter-agent sessions_send message, so the target can no longer reverse-call the requester. Result delivery is unchanged (it still happens via the tool return value / announce flow).

Fixes #39476

Root cause

The initial send (src/agents/tools/sessions-send-tool.ts) and the A2A flow (runSessionsSendA2AFlow in src/agents/tools/sessions-send-tool.a2a.ts) route the requester's message into the target session as an inter-session turn (inputProvenance.kind = "inter_session", sourceTool = "sessions_send") and expose the requester's session via buildAgentToAgentMessageContext. The target's per-turn tool list (built in createOpenClawTools) still included sessions_send, so the target model could call it back at the requester and duplicate the reply that the tool already returns.

The fix (Option A: code-level tool guard)

Drop sessions_send from the target's tool set for the A2A-routed turn:

  • src/sessions/input-provenance.ts: add isAgentToAgentSendInputProvenance(), true when kind === "inter_session" and sourceTool === "sessions_send".
  • src/agents/embedded-agent-runner/run/attempt.tool-run-context.ts: derive interAgentSendTurn from the run's inputProvenance and forward it into tool construction.
  • src/agents/agent-tools.ts and src/agents/openclaw-tools.ts: thread interAgentSendTurn through and skip creating sessions_send when it is set.

This covers all A2A target turns (initial send, ping-pong, announce) because they all carry the same provenance, and it is per-turn: a normal (non-A2A) turn restores sessions_send. Other session tools (sessions_list, sessions_history) stay available during the routed turn. subagent_announce and other inter-session handoffs use a different sourceTool and are unaffected.

Note: this guards the OpenClaw agent loop's sessions_send tool. Native-harness (e.g. Codex MCP-bridged) tool exposure is a separate surface and is not changed here.

Verification

Focused colocated unit tests, run single-worker to keep local CPU low:

  • OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/agents/openclaw-tools.inter-agent-send.test.ts -> 2 passed (proves sessions_send is present on a normal turn and absent on an A2A turn, while sessions_list/sessions_history remain).
  • node scripts/run-vitest.mjs src/sessions/input-provenance.test.ts -> passed (new isAgentToAgentSendInputProvenance cases).
  • node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/attempt.test.ts -t "buildEmbeddedAttemptToolRunContext" -> passed (provenance -> interAgentSendTurn derivation).

Intentionally NOT run in this environment to limit local CPU/heat: tsgo/typecheck, pnpm build, and the full test suite. OpenClaw was NOT run locally (no gateway/CLI); a live multi-agent A2A round-trip was not exercised.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 7, 2026
@clawsweeper

clawsweeper Bot commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 3:55 PM ET / 19:55 UTC.

Summary
The PR threads A2A sessions_send provenance into OpenClaw tool construction and conditionally omits sessions_send from routed inter-agent target turns.

PR surface: Source +42, Tests +91. Total +133 across 7 files.

Reproducibility: yes. at source level: current main marks routed A2A turns as inter-session sessions_send input and still builds the normal sessions_send tool for non-embedded target runs. I did not run a live Discord, gateway, or multi-agent transport reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Model-visible tool surface: 1 core messaging tool conditionally hidden. The diff changes sessions_send availability for provenance-scoped A2A turns, so maintainers need behavior proof beyond unit construction tests.

Stored data model
Persistent data-model change detected: serialized state: src/sessions/input-provenance.test.ts, serialized state: src/sessions/input-provenance.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #39476
Summary: This PR is the open candidate fix for the canonical reverse sessions_send duplicate-delivery bug; related open work covers adjacent delivery, compaction, or dispatch semantics rather than a safe replacement for this guard.

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 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 real A2A proof showing one requester-visible delivery and no reverse duplicate.
  • Update the PR body with the proof so ClawSweeper can re-review automatically; redact IPs, tokens, phone numbers, non-public endpoints, and other private details.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides unit-test output only and explicitly says no live gateway, CLI, or multi-agent A2A round trip was exercised. 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.

Proof path suggestion
A real two-agent run would materially prove the user-visible duplicate-delivery behavior that unit tests cannot show. Mantis is proof-only, so it must not be asked to change code or mutate GitHub state.
Use ClawSweeper's repair, apply, or automerge lanes for code changes, branch updates, labels, comments, PR repair, closes, or merges.

Risk before merge

  • [P1] The PR conditionally removes a model-visible messaging tool from one routed-turn class, so a real A2A run should prove it blocks reverse duplicates without suppressing normal top-level sends or expected non-send inter-session handoffs.
  • [P1] The contributor proof is unit-test only; no live gateway, CLI, or multi-agent A2A round trip has shown one requester-visible delivery and no reverse duplicate.

Maintainer options:

  1. Require a real A2A proof run (recommended)
    Ask for a redacted CLI, gateway, or transport run showing a requester-to-target sessions_send returns once and does not produce a reverse duplicate.
  2. Accept source-level proof only
    Maintainers may intentionally land the scoped guard based on source review and unit tests while owning any missed live A2A delivery regression.
  3. Pause for A2A semantics review
    If reverse sessions_send calls during routed A2A turns are desired in any deployment, pause and define that product contract before merging.

Next step before merge

  • [P1] Do not queue a duplicate repair branch because this open PR is already the linked implementation candidate; the remaining blocker is contributor real behavior proof plus maintainer review.

Security
Cleared: Cleared: the diff threads an internal boolean and adds focused tests; it does not touch workflows, dependency sources, lockfiles, secrets, permissions, or third-party code execution.

Review details

Best possible solution:

Land the provenance-scoped OpenClaw tool-loop guard after redacted real A2A proof shows one requester-visible delivery, no reverse duplicate, and normal non-A2A tool availability unchanged.

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

Yes at source level: current main marks routed A2A turns as inter-session sessions_send input and still builds the normal sessions_send tool for non-embedded target runs. I did not run a live Discord, gateway, or multi-agent transport reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, pending proof: using input provenance to hide only sessions_send during sessions_send-routed A2A processing is narrower than prompt-only mitigation or the broader one-way dispatch-mode redesign. The PR explicitly covers the OpenClaw tool loop, not native harness tool exposure.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 076da567f434.

Label changes

Label justifications:

  • P1: The PR targets duplicate requester-visible delivery in core agent-to-agent sessions_send workflows.
  • merge-risk: 🚨 message-delivery: The patch conditionally removes a messaging tool from routed A2A turns and could alter delivery behavior if the provenance guard is too broad.
  • 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 real behavior proof before merge: The PR body provides unit-test output only and explicitly says no live gateway, CLI, or multi-agent A2A round trip was exercised. 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.
Evidence reviewed

PR surface:

Source +42, Tests +91. Total +133 across 7 files.

View PR surface stats
Area Files Added Removed Net
Source 4 43 1 +42
Tests 3 91 0 +91
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 7 134 1 +133

What I checked:

  • Repository policy read: Root and scoped agent policies were read fully; they require whole-path review for agent/session message delivery changes and real behavior proof for user-visible delivery changes. (AGENTS.md:1, 076da567f434)
  • Current main A2A provenance: sessions_send builds target-run input provenance with kind: "inter_session" and sourceTool: "sessions_send" before starting the target run. (src/agents/tools/sessions-send-tool.ts:585, 076da567f434)
  • Current main tool exposure: createOpenClawTools still creates sessions_send for non-embedded runs with no provenance-scoped A2A exclusion. (src/agents/openclaw-tools.ts:506, 076da567f434)
  • A2A follow-up source tool: runSessionsSendA2AFlow forwards ping-pong and announce turns with sourceTool: "sessions_send", so they share the routed-turn class the PR guards. (src/agents/tools/sessions-send-tool.a2a.ts:165, 076da567f434)
  • PR implementation: The PR adds interAgentSendTurn wiring and skips createSessionsSendTool when that flag is set while leaving sessions_list and sessions_history available in the focused test. (src/agents/openclaw-tools.ts:498, 11ba938e4777)
  • Proof gap: The PR body lists focused unit-test commands and explicitly says OpenClaw was not run locally and no live multi-agent A2A round trip was exercised. (11ba938e4777)

Likely related people:

  • vincentkoc: Recent GitHub path history shows multiple sessions_send changes in the same send and A2A reply lifecycle, including delayed reply and explicit-key handling. (role: recent area contributor; confidence: high; commits: 0ea08076c3b5, 3659ff8bbf71, b0a6543838ac; files: src/agents/tools/sessions-send-tool.ts, src/agents/tools/sessions-send-tool.a2a.ts)
  • steipete: History shows earlier session tool, inter-session provenance, and A2A/session delivery work that this guard relies on. (role: feature-history contributor; confidence: high; commits: 817e6e810b7d, 85409e401b65, c5c08c074a44; files: src/agents/tools/sessions-send-tool.a2a.ts, src/sessions/input-provenance.ts, src/agents/tools/sessions-send-helpers.ts)
  • jalehman: Authored sessions_send announce delivery work in the same A2A announce lifecycle affected by duplicate requester-visible delivery. (role: adjacent announce-routing contributor; confidence: medium; commits: 8359e5f584d3; files: src/agents/tools/sessions-send-tool.a2a.ts)
  • stainlu: Scoped nested agent lanes per target session across the same A2A execution path used by sessions_send reply and announce turns. (role: adjacent nested-lane contributor; confidence: medium; commits: 4da808da5028; files: src/agents/tools/sessions-send-tool.ts, src/agents/tools/sessions-send-tool.a2a.ts, src/agents/lanes.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: 🦪 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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 7, 2026
This was referenced Jun 10, 2026
Open
Open
Open
This was referenced Jun 18, 2026
Open
Closed
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 19, 2026
@cavit99 cavit99 mentioned this pull request Jun 27, 2026
Merged
This was referenced Jun 27, 2026
Closed
Merged
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Open
@dankarization

dankarization commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

@sumitaich1998 Could you add the requested redacted live A2A behavior proof, or indicate whether help is needed to produce it?

We have reproduced user-visible duplicate Telegram delivery in a real main -> sessions_send -> delegated agent workflow. This PR appears to address the reverse-call part of that path, but the current review still blocks merge on live proof. A rebase against current main and maintainer review would also help move it forward.

@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: prevent A2A target from reverse-calling sessions_send to requester (#39476) This is item 1/1 in the current shard. Shard 0/20.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@dankarization dankarization mentioned this pull request Jul 21, 2026
Open
@dankarization

dankarization commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

I ported this fix and its tests onto current main in PR #112326, with explicit credit to this PR and its author.

The current runtime now groups conversation tools next to sessions_send, so the replacement narrows the guard to remove only sessions_send while preserving sessions_list, sessions_history, sessions_search, and conversations_list. The external source branch was not writable from my account and had substantial drift, so a replacement was the safe path.

@sumitaich1998

sumitaich1998 commented Jul 21, 2026

Copy link
Copy Markdown
Author

I ported this fix and its tests onto current main in PR #112326, with explicit credit to this PR and its author.

The current runtime now groups conversation tools next to sessions_send, so the replacement narrows the guard to remove only sessions_send while preserving sessions_list, sessions_history, sessions_search, and conversations_list. The external source branch was not writable from my account and had substantial drift, so a replacement was the safe path.

thankyou brother @dankarization

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 merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. 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. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A2A sessions_send: target agent can call sessions_send back, causing duplicate messages

2 participants

@sumitaich1998 @dankarization