fix(reply): preserve sessions_send external routes#88803
Conversation
openclaw-barnacle
Bot
added
size: S
triage: mock-only-proof
|
Codex review: needs real behavior proof before merge. Reviewed May 31, 2026, 11:32 PM ET / 03:32 UTC. Summary PR surface: Source +49, Tests +250. Total +299 across 8 files. Reproducibility: yes. from source and the linked issue logs: current main resolves non-system Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow routing fix after maintainers accept the persisted-route compatibility behavior and either receive fresh real Feishu/Lark transport proof or explicitly override that proof gap. Do we have a high-confidence way to reproduce the issue? Yes, from source and the linked issue logs: current main resolves non-system Is this the best way to solve the issue? Yes, the proposed code path is the best narrow owner-boundary fix I found: it extends effective route resolution, dispatch, and recovery contexts instead of adding channel-specific Feishu logic or parsing session keys alone. The remaining issue is proof and upgrade-safety acceptance, not a line-level code defect. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against ebcdb637bb77. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +49, Tests +250. Total +299 across 8 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
rating: 🧂 unranked krab
MonkeyLeeT
changed the title
|
@clawsweeper re-review Updated the PR body with #88044's redacted real Feishu/journalctl evidence and kept the after-fix route/dispatch proof tied to the same runtime shape. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
openclaw-barnacle
Bot
added
proof: supplied
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
@clawsweeper re-review Updated the PR body with patched-branch after-fix proof for the original #88044 route shape. The evidence shows a |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e095bc4a32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
proof: sufficient
openclaw-barnacle
Bot
removed
the
proof: sufficient
|
@clawsweeper re-review Addressed the inherited-route thread fallback finding in
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8fb236c56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const replyRoute = resolveEffectiveReplyRoute({ | ||
| ctx: params.sessionCtx, | ||
| entry: params.sessionEntry, | ||
| }); |
There was a problem hiding this comment.
When a sessions_send handoff inherits an external route, this helper still runs resolveSourceReplyPolicy() before resolving replyRoute, so the send-policy check uses the live internal webchat channel from resolveSourceReplyPolicy rather than the external channel that dispatch will actually send to. In configs that deny webchat (or default-deny and allow the external channel), live dispatch can still route because dispatch-from-config.ts later checks policy with routeReplyChannel, but restart/pending-final context is never persisted here; a restart during final delivery would then lose the Feishu/Slack/etc. recovery target.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
status: 📣 needs proof
b8fb236 to
52a6b67
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52a6b67c59
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| params.originatingThreadId ?? | ||
| resolveRoutedDeliveryThreadId({ | ||
| ctx: params.ctx, | ||
| sessionKey: deliverySessionKey, | ||
| }); |
There was a problem hiding this comment.
When a sessions_send handoff inherits an external route that has no stored thread and the ACP hook owns delivery, originatingThreadId should remain absent; this fallback instead pulls MessageThreadId or the thread-scoped session key from the internal/webchat ACP context and sends it as the Feishu/Slack/etc. thread. That can route an otherwise unthreaded persisted external conversation into the wrong thread or fail provider validation, so the ACP routed-delivery path needs to preserve undefined for inherited external routes rather than deriving a source-context thread.
Useful? React with 👍 / 👎.
|
Maintainer pass on exact head
Known gap: no live Feishu end-to-end run was performed here; coverage is regression/unit plus CI for the affected dispatch/session surfaces. |
* fix(reply): preserve sessions_send external routes * fix(reply): preserve inherited route thread ids * fix(reply): keep sessions_send delivery single-owner * fix(reply): satisfy dispatch route lint * fix(reply): preserve inherited ACP route metadata * test(reply): type inherited route event assertions * test(ci): satisfy current lint rules * fix(reply): avoid stale inherited route threads * fix(reply): trust explicit inherited route threads * fix(reply): require trusted route thread sources --------- Co-authored-by: Peter Steinberger <[email protected]>
* fix(reply): preserve sessions_send external routes * fix(reply): preserve inherited route thread ids * fix(reply): keep sessions_send delivery single-owner * fix(reply): satisfy dispatch route lint * fix(reply): preserve inherited ACP route metadata * test(reply): type inherited route event assertions * test(ci): satisfy current lint rules * fix(reply): avoid stale inherited route threads * fix(reply): trust explicit inherited route threads * fix(reply): require trusted route thread sources --------- Co-authored-by: Peter Steinberger <[email protected]>
* fix(reply): preserve sessions_send external routes * fix(reply): preserve inherited route thread ids * fix(reply): keep sessions_send delivery single-owner * fix(reply): satisfy dispatch route lint * fix(reply): preserve inherited ACP route metadata * test(reply): type inherited route event assertions * test(ci): satisfy current lint rules * fix(reply): avoid stale inherited route threads * fix(reply): trust explicit inherited route threads * fix(reply): require trusted route thread sources --------- Co-authored-by: Peter Steinberger <[email protected]>
2 participants
Summary
Fixes #88044.
sessions_sendhandoffs that enter through the internalwebchatsurfacedeliveryContext.threadIdfor inherited external routes so threaded transports stay on the stored threadsessions_send/ agent-step handoffs on automatic source delivery instead of forcingmessage_tool_only, so the inheritedrouteReplypath is the production pathBehavior
Before this change, an agent-to-agent
sessions_sendturn could arrive as internalwebchatand then fail to deliver a reply back to the existing Feishu session, even when the target session still had a valid externaldeliveryContext.After this change, only
sessions_sendinter-session handoffs with a persisted deliverable externaldeliveryContextinherit that external route. If the persisted route has athreadId, dispatch and restart/pending-final recovery preserve it only for that inherited route case. Normal Control UI/webchat turns continue to use their live webchat route, ordinary routed replies still derive thread context from the active turn or thread-scoped session key, andsessions_sendno longer requestsmessage_tool_onlysuppression.Compatibility / maintainer decision
This PR intentionally treats the target session's persisted external
deliveryContextas authoritative forsessions_sendinter-session replies that arrive through internalwebchat. Maintainers should accept that compatibility behavior before merge: existing sessions with stale stored channel, target, account, or thread metadata can affect live delivery, but the same persisted route is the only durable source tying an internal handoff back to the user's original external conversation. If that persisted-route behavior is not desired, this PR should pause for a different product direction rather than land with a parallel fallback.Verification
node --require ./node_modules/tsx/dist/preflight.cjs --import ./node_modules/tsx/dist/loader.mjs .artifacts/proof/sessions-send-feishu-route-proof.tsnode scripts/run-vitest.mjs src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/effective-reply-route.test.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts src/agents/openclaw-tools.sessions.test.ts src/agents/tools/agent-step.test.tspnpm format:check src/auto-reply/reply/dispatch-from-config.ts src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.tsgit diff --checkFocused regressions on
b8fb236c56:sessions_sendproduction gateway params and agent-step handoffs do not includesourceReplyDeliveryMode, inherited route resolution preservesdeliveryContext.threadId="thread:om_123", dispatch passes that same thread id torouteReply, unthreaded inherited external routes do not borrow the live internal webchat thread, restart recovery and pending-final replay keep the inherited Feishu thread instead of the live internal webchat thread, and the stale-thread regression still proves ordinary/non-inherited routes do not resurrect a normalized stored thread id.Real behavior proof
Behavior addressed:
sessions_sendreplies can route through a target session's persisted external delivery context instead of being trapped as internalwebchat; inherited external routes also preserve the persisted route thread when present, including restart-recovery and pending-final replay contexts.Real environment tested: original issue #88044 reports a real OpenClaw 2026.5.27 Feishu/Lark setup with 7 agents,
tools.agentToAgent.enabled=true, andtools.sessions.visibility="all". The after-fix proof ran on patched branchb8fb236c56using that same route shape: target session keyagent:B:feishu:B:direct:ou_xxx, persisteddeliveryContext.channel="feishu", incomingInputProvenance.kind="inter_session",sourceTool="sessions_send", and live provider/surface/originating channel allwebchat. To avoid live credentials in the PR, the proof registered a Feishu outbound adapter and exercised productionsessions_sendparameter construction, source-reply visibility policy, effective-route resolution, durable external-channel delivery, unthreaded inherited-route dispatch, and restart/pending-final recovery context persistence.Exact steps or command run after this patch:
Evidence after fix:
{ "head": "b8fb236c56", "productionSessionsSendAgentParams": { "sessionKey": "agent:B:feishu:B:direct:ou_xxx", "deliver": false, "channel": "webchat", "lane": "nested:agent:B:feishu:B:direct:ou_xxx", "inputProvenance": { "kind": "inter_session", "sourceTool": "sessions_send", "sourceChannel": "webchat" }, "sourceReplyDeliveryMode": "absent" }, "sourceReplyPolicy": { "sourceReplyDeliveryMode": "automatic", "suppressAutomaticSourceDelivery": false, "suppressDelivery": false }, "persistedDeliveryContext": { "channel": "feishu", "to": "user:ou_123", "accountId": "work", "threadId": "thread:om_123" }, "effectiveRoute": { "channel": "feishu", "to": "user:ou_123", "accountId": "work", "threadId": "thread:om_123", "inheritedExternalRoute": true }, "unthreadedInheritedRoute": { "liveInternalWebchatThread": "internal-webchat-thread", "routeReplyThreadId": "undefined" }, "recoveryContexts": { "liveInternalWebchatThread": "internal-webchat-thread", "restartRecoveryDeliveryContext": { "channel": "feishu", "to": "user:ou_123", "accountId": "work", "threadId": "thread:om_123" }, "pendingFinalDeliveryContext": { "channel": "feishu", "to": "user:ou_123", "accountId": "work", "threadId": "thread:om_123" } }, "feishuSends": [ { "adapter": "registered feishu outbound.sendText", "to": "user:ou_123", "accountId": "work", "text": "QA-SESSIONS-SEND-FEISHU-PROOF", "threadId": "thread:om_123" } ], "observed": "PASS: sessions_send handoff keeps automatic delivery and uses persisted Feishu thread route" }Observed result after fix: the latest production
sessions_sendhandoff no longer asks dispatch to suppress automatic source delivery, the visibility policy resolves toautomatic, the same route shape from #88044 resolves the final reply route to Feishu withthreadId="thread:om_123", durable delivery calls the registered Feishu outbound adapter with that target and proof marker, and restart/pending-final recovery stores the Feishu thread instead of the internal webchat thread.What was not tested: a fresh live Feishu tenant run against this patched branch was not rerun; the live Feishu evidence remains the original issue's redacted runtime logs, paired here with latest-head production-parameter, route, durable-delivery, and recovery-context proof for the same persisted Feishu route shape.