fix(agents): prefer target agent's bound Matrix account for subagent spawns#67508
Conversation
Greptile SummaryThis PR fixes a bug where Matrix (and other-channel) subagent spawns inherited the caller's Confidence Score: 5/5Safe to merge — no P0 or P1 issues found; the bug fix is correct and comprehensively tested. The four-tier precedence model in No files require special attention. Reviews (2): Last reviewed commit: "fix(agents): let id-embedded kind marker..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 7a0506b007
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00b393064c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8430fd70b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d662dc529b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3d8c0f635
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3d8c0f635
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f673354a4
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 173827d6a9
ℹ️ 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".
|
@codex review |
|
Install Smoke root cause identified — pre-existing build-system gap, not from this PR.
Why it surfaced here and not elsewhere: most recent Proposed one-line fix for - "build:docker": "node scripts/tsdown-build.mjs && node scripts/runtime-postbuild.mjs && node scripts/build-stamp.mjs && …",
+ "build:docker": "node scripts/tsdown-build.mjs && node scripts/runtime-postbuild.mjs && node --import tsx scripts/write-npm-update-compat-sidecars.ts && node scripts/build-stamp.mjs && …",(Insert Happy to include that fix in this PR if you'd like — it's pure build-script, trivially reviewable, and gets Install Smoke green. Otherwise I'll leave this PR focused on the routing fix and file the build gap separately. Your call. The CI |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
… normalization
Addresses two new Codex P1s on the bound-account lookup path:
1. Strip channel prefix before generic target-kind prefixes
(src/agents/subagent-spawn.ts)
normalizeRequesterPeerId stripped a single generic prefix before the
`<channel>:` namespace, so LINE delivery targets of the form
`line:group:<id>` ended up as `group:<id>` instead of `<id>` and the
exact peer-id binding was missed. The helper now peels the channel
namespace first, then loops over generic prefixes (room:, channel:,
chat:, user:, dm:, group:) until the raw peer id surfaces.
2. Enforce peer-kind safety for wildcard bindings when caller kind is
unknown (src/routing/bound-account-read.ts)
A `peer.id: "*"` binding with an incompatible kind (for example
direct/*) could still be accepted when the caller did not supply a
peerKind — channels whose plugin does not implement inferTargetChatType
(such as Matrix) could then have room-originated spawns resolve to
the wrong DM-bound account, which is worse than preserving the caller
account. Wildcard matches now require both sides to declare a
peer.kind and for the kinds to agree; otherwise the binding is
skipped. Exact peer-id matches still require id equality, with a
kind cross-check only when both sides declare a kind (peer ids are
channel-unique).
To preserve the existing Matrix-room happy path (the Matrix plugin
does not expose inferTargetChatType), inferPeerKindForChannel gains a
conservative fallback that recognizes `!`-prefixed room ids as
"channel" and `@`-prefixed user ids as "direct", matching the
existing `normalizeId` convention in extensions/matrix.
Regression coverage:
- src/routing/bound-account-read.test.ts:
- Skips wildcard peer bindings when caller's peerKind is unknown.
- Matches exact peer id even when caller's peerKind is unknown.
- Updated prefers-wildcard-over-channel-only test to pass an explicit
peerKind reflecting the new strict requirement.
- src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts:
- sessions_spawn peels channel prefix then kind prefix for
`<channel>:<kind>:<id>` targets — LINE-style `line:group:<id>`
resolves to the binding configured on the raw peer id.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…unt lookup Addresses three further review findings: 1. Parse non-Matrix peer kinds before wildcard account lookup (P1) Channels like LINE emit `line:group:<id>` / `line:room:<id>` targets, but the Matrix-style heuristic only recognized `@`, `!`, and `#` so the kind was lost and wildcard bindings with a declared kind were skipped. subagent-spawn now uses a single `extractRequesterPeer` helper that tries the channel plugin first, then peels the channel namespace, then loops stripping kind-prefixes (`room:`/`channel:` → `channel`, `group:` → `group`, `user:`/`dm:` → `direct`, `chat:` → `channel`) — capturing the kind from the prefix as authoritative — before falling back to the id-only heuristic for Matrix/IRC shapes. 2. Allow wildcard bindings in peerless bound-account fallback (P2) Peerless callers (cron delivery resolution passes only `channelId`/`agentId`) were blocked by the strict wildcard kind safety, silently regressing configs that only declare wildcard peer bindings. Peerless callers now accept wildcard bindings as a last- resort fallback — the kind-safety rule only applies when the caller supplies a peer to verify against. 3. Treat `group` and `channel` peer kinds as equivalent (P1) Routing elsewhere (`peerKindMatches` in `src/routing/resolve-route.ts`) intentionally accepts group/channel as compatible so a binding declared as `peer.kind: "group"` resolves for callers inferred as `channel` (Matrix rooms, Mattermost/Slack channels) and vice versa. `resolveFirstBoundAccountId` now applies the same semantics in both the wildcard and exact-id kind-cross-check branches. Regression coverage in src/routing/bound-account-read.test.ts: - Treats group and channel peer kinds as equivalent (both directions). - Accepts a wildcard peer binding as fallback for peerless callers. - Skips wildcard peer bindings when the caller's peerKind is unknown. Regression coverage in openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts: - sessions_spawn peels channel prefix then kind prefix for `<channel>:<kind>:<id>` targets (LINE `line:group:<id>` shape) and correctly picks the `group`-kinded binding over a `direct`-kinded wildcard binding for the same agent. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
MS Teams inbound context sets OriginatingTo to `conversation:<id>` while route bindings key on the bare conversationId. The previous hand-rolled prefix list (`room:`, `channel:`, `chat:`, `group:`, `user:`, `dm:`) missed `conversation:` (and any future channel-specific namespacing), so Teams cross-agent subagent spawns fell back to channel-only/caller account and posted from the wrong identity. extractRequesterPeer now uses a generic `^[a-z][a-z0-9_-]*:` regex to peel any lowercase-alpha token-colon prefix, looping until the raw peer id surfaces. Real-world peer ids never start with a lowercase-alpha token followed by `:` (Matrix uses `!`/`@`, IRC `#`, Slack/Discord/LINE alphanumerics, numeric Telegram/WhatsApp, or email-style `user@server`), so this is safe. Known prefixes are mapped to ChatType for peerKind inference (`conversation:`/`room:`/`channel:`/`chat:`/`thread:`/`topic:` → channel, `group:`/`team:` → group, `user:`/`dm:`/`pm:` → direct). Regression test: sessions_spawn strips `conversation:` prefix for Teams-style targets — a binding keyed on the raw conversation id resolves correctly when the caller's `agentTo` is `conversation:<id>`. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Matrix thread delivery encodes per-user DM targets as `room:@user:server` — the `room:` wrapper says "channel" but the embedded `@` id marker says "direct". The previous extractRequesterPeer gated the `@`/`!`/`#` heuristic on `!inferredKind`, so the prefix-derived kind won and a direct-kinded peer binding on the same user id was rejected by the kind-safety check in resolveFirstBoundAccountId. Cross-agent spawns whose target was bound as a direct Matrix peer could fall back to the caller account and send from the wrong identity. The fix removes the `!inferredKind` guard so id-embedded kind markers always have the final say — they are a more reliable signal than the delivery-target wrapper, because channel-side prefixes can wrap either a room or a user id. Regression test: sessions_spawn classifies Matrix `room:@user` targets as direct, not channel — the lifecycle suite now configures a conflicting `channel`-kinded binding on the same user id and asserts the `direct`-kinded binding wins when the caller's `agentTo` is `room:@user:example.org`. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
gumadeiras
force-pushed
the
fix/matrix-bound-account-subagent-spawn-squashed
branch
from
ec114e7 to
9300111
Compare
|
Merged via squash.
Thanks @lukeboyett! |
…spawns (openclaw#67508) Merged via squash. Prepared head SHA: 9300111 Co-authored-by: lukeboyett <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras
…spawns (openclaw#67508) Merged via squash. Prepared head SHA: 9300111 Co-authored-by: lukeboyett <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras
…spawns (openclaw#67508) Merged via squash. Prepared head SHA: 9300111 Co-authored-by: lukeboyett <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras
…spawns (openclaw#67508) Merged via squash. Prepared head SHA: 9300111 Co-authored-by: lukeboyett <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras
2 participants
Summary
Fixes Matrix (and other-channel) subagent spawns inheriting the caller's
accountIdinstead of the target agent's bound account, and hardensresolveFirstBoundAccountIdwith kind-aware multi-tier matching for peer bindings.src/agents/subagent-spawn.ts: addsresolveRequesterOriginForChild(...)that prefers the target agent's bound account viaresolveFirstBoundAccountId({ cfg, channelId, agentId: targetAgentId, peerId, peerKind })before falling back to the caller'saccountId. Same-agent spawns short-circuit the lookup and preserve the caller's account. A newextractRequesterPeerhelper peels known delivery-target prefixes (<channel>:namespace, then a generic<word>:loop) fromagentToto produce the raw peer id and — for channels whose plugin does not implementinferTargetChatType— inferpeerKindfrom kind-prefixes (room:/channel:/chat:→ channel,group:/team:→ group,user:/dm:/pm:→ direct) and id-embedded markers (Matrix!/@, IRC#). Id-embedded kind markers take precedence over prefix-derived kind so wrappers likeroom:@user:serverclassify correctly.src/routing/bound-account-read.ts:resolveFirstBoundAccountIdnow accepts optionalpeerIdandpeerKind, and uses a four-tier precedence model:peer.id: "*") match — only when the caller supplies a peer AND both sides declare a matching kind (treatinggroup/channelas equivalent, mirroringpeerKindMatchesinresolve-route.ts).peerId— preserves the pre-existing first-match semantics for cron delivery resolution.delivery-target.tscaller is unaffected: it still passes nopeerId/peerKind, and the helper returns the same binding it would have before on realistic configs.Why
A Matrix-bound parent session spawning a child for another agent could seed
deliveryContext.accountIdwith the caller's account before target bindings were consulted. A child intended to post as the target agent's bound identity could end up posting as the caller — broken speaker identity in shared rooms, nondeterministic debugging, and wrong attribution in multi-agent deployments.The original
resolveFirstBoundAccountIdalso matched only on channel + agent, so multi-room-bound agents with different accounts per room got "first binding wins" regardless of active room. The peer-id/peer-kind aware matching fixes that while staying backward-compatible for callers that don't pass a peer (cron).Regression coverage
Twelve unit tests in
src/routing/bound-account-read.test.ts(new file) exercise the four-tier lookup: exact peer match, wildcard peer beats channel-only (with caller kind), channel-only beats wildcard (no caller kind), peer-specific fallback for peerless callers, non-matching peer skipped, channel mismatch, kind filtering, kind-unknown wildcard skip, exact id with unknown kind, group/channel equivalence, and the agent-on-different-channel no-match case.Fifteen lifecycle tests in
src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts(eleven new):agentTobefore lookup (Matrixroom:<id>).<channel>:<kind>:<id>targets peel both prefixes (LINEline:group:<id>) and pick thegroup-kinded binding over a conflictingdirect-kinded wildcard.conversation:<id>prefix stripped for Teams-style targets.room:@user:serverclassified asdirect, notchannel, so direct-peer bindings match.Validation
pnpm build✓pnpm check✓ (typecheck, lint, import cycles, boundary checks)pnpm test:changed✓ (3777 passed on round 1)Review iterations
This PR went through eight rounds of bot review (Codex + Greptile) before final approval. Commit history reflects each iteration:
5d11d7bb1e5d0027479e*treated as literal → three-tier precedence00b393064cf8430fd70bpeer.kind→ kind-aware matching + plugin inferenced662dc529bnormalizeRequesterPeerId3f673354a4<channel>:<kind>:<id>parsing; peerless wildcard fallback restored;group/channelkind equivalence173827d6a9conversation:<id>→ generic^[a-z][a-z0-9_-]*:peeler249b292111room:@user→ direct)Final state: all twelve bot review threads resolved, Codex
Didn't find any major issueson the current HEAD.AI-assisted
Authored with Claude Code (Claude Opus 4.6, 1M-context). Fully tested locally. I've reviewed the change and understand what it does.
Related / prior art
deliveryContextfrom spawn params at the gateway layer — complementary, different layer.accountIdforwarding insendMessage.Known-red CI
Install Smokeis red due to a pre-existing build-system gap (build:dockerinpackage.jsondoesn't runscripts/write-npm-update-compat-sidecars.ts, unlikebuild-all.mjs). Root cause documented in this comment with a proposed one-line fix — happy to include here or split out per maintainer preference.Run the GPT-5.4 / Opus 4.6 parity gate against the qa-lab mockis red and looks unrelated to this PR.