feat(session): add session.groupScope to fold group chats into the agent main session#91702
feat(session): add session.groupScope to fold group chats into the agent main session#91702ikenraf wants to merge 4 commits into
Conversation
…main session Mirrors session.dmScope. groupScope: per-group (default, current behavior) | main (route group/channel chats into the agent main session). Also supported as a per-binding override (bindings[].session.groupScope) so specific conversations can opt in/out independently of the global default. Addresses openclaw#7524.
…baseline Add tests, documentation, and config-doc baseline for the session.groupScope option (folds group chats into the agent main session, mirroring dmScope). - routing tests: global groupScope:"main" folds a group peer to the agent main key; default per-group keeps the group's own key; per-binding override beats the global default in both directions; buildAgentPeerSessionKey unit cases. - docs: groupScope subsection in concepts/session.md, routing-table note, and a session-key note in reference/session-management-compaction.md. - changelog: entry under 2026.6.1 Changes. - regenerate config-doc baseline hash to capture the new config surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Codex review: needs real behavior proof before merge. Reviewed June 9, 2026, 10:46 AM ET / 14:46 UTC. Summary PR surface: Source +124, Tests +351, Docs +27, Generated 0. Total +502 across 16 files. Reproducibility: yes. for the review findings by source inspection: PR head calls the outbound binding matcher with no parent, guild, team, or role metadata while exposing Review metrics: 1 noteworthy metric.
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:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land an additive opt-in group-scope contract only after inbound and outbound route matching share the same available binding metadata, shared group/main context has maintainer-approved audit/docs posture, the changelog edit is removed, and redacted real group-chat proof is attached. Do we have a high-confidence way to reproduce the issue? Yes for the review findings by source inspection: PR head calls the outbound binding matcher with no parent, guild, team, or role metadata while exposing Is this the best way to solve the issue? No as submitted. The feature direction is plausible, but the maintainable fix needs one supported binding-aware session-scope path for inbound and outbound behavior plus explicit security/audit posture before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 7cdec2870604. Label changesLabel justifications:
Evidence reviewedPR surface: Source +124, Tests +351, Docs +27, Generated 0. Total +502 across 16 files. View PR surface stats
Security concerns:
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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35c0e8e23d
ℹ️ 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 memberRoleIds = input.memberRoleIds ?? []; | ||
| const memberRoleIdSet = new Set(memberRoleIds); | ||
| const dmScope = input.cfg.session?.dmScope ?? "main"; | ||
| const groupScope = input.cfg.session?.groupScope; |
There was a problem hiding this comment.
Include groupScope in route-cache key
When cfg.session.groupScope is changed on an existing config/session object, this new value is read here but the resolved-route cache key still only includes dmScope (buildResolvedRouteCacheKey receives no groupScope). A cached group route can therefore keep returning agent:<id>:main after switching back to per-group (or the reverse) until the session object reference changes, which makes live config updates route group chats to the wrong transcript bucket.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 326ac5a4df — groupScope is now included in buildResolvedRouteCacheKey alongside dmScope, so changing session.groupScope on an existing config object yields a distinct cache key and the route is recomputed rather than served stale. Thanks for catching it.
| accountId: params.accountId, | ||
| peer: params.peer, | ||
| dmScope: params.cfg.session?.dmScope ?? "main", | ||
| groupScope: params.cfg.session?.groupScope, |
There was a problem hiding this comment.
Honor binding groupScope for outbound route keys
For outbound-only sends to a group that has a matching bindings[].session.groupScope override, this helper only applies the global cfg.session.groupScope and never resolves the route binding. Inbound resolveAgentRoute can now put that selected group in agent:<id>:main, while the outbound mirror path writes the same conversation to agent:<id>:<channel>:group:<id> when the global default is per-group, so the advertised per-binding override is inconsistent across inbound and outbound state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate — thanks. buildOutboundBaseSessionKey applies the global scope for both dmScope and groupScope and doesn't resolve per-binding overrides today, so groupScope here mirrors the existing dmScope behavior.
Normal replies inherit the inbound-resolved key (which does honor the per-binding override via resolveAgentRoute), so the inconsistency is limited to outbound-only / proactive sends to a group that carries a per-binding session.groupScope override.
Closing that gap means having this helper resolve per-binding overrides — but that would also change dmScope's outbound-only behavior, so I left it out to keep this PR scoped to groupScope. Happy to add it here (covering both dmScope and groupScope symmetrically) if you'd prefer the outbound-only path resolve bindings.
There was a problem hiding this comment.
Update — implemented in 66350914e5.
buildOutboundBaseSessionKey now resolves the matching binding's session override (both dmScope and groupScope) and applies override ?? global, so inbound and outbound resolve to the same session for a peer/group carrying a per-binding override. To avoid a second copy of the matching logic, I extracted the binding-matching tier loop from resolveAgentRoute into a shared findMatchingBinding, and added resolveOutboundBindingSessionScope that reuses it — so the two paths can't drift. The caller's explicit agentId is preserved; only the scope now follows the binding.
Tests: new src/infra/outbound/base-session-key.test.ts covers a per-binding groupScope override in both directions (fold-in vs keep-separate), a dmScope override, the no-binding global fallback, and that the caller's agentId is preserved over the binding's agent. Full routing + outbound suites pass (137 + 31).
…onfig metadata - include groupScope in buildResolvedRouteCacheKey (parity with dmScope) - add session.groupScope + bindings[].session.groupScope to schema.help and schema.labels - regenerate config-doc baseline
|
Thanks for the review — addressed both points in
Behavior: the tests in One related note for your call: |
…on keys buildOutboundBaseSessionKey now resolves the matching binding's dmScope/groupScope override (via a shared findMatchingBinding extracted from resolveAgentRoute) instead of using only the global session config, so inbound and outbound resolve to the same session for peers/groups with a per-binding override. Addresses PR review.
What
Adds
session.groupScope, mirroring the existingsession.dmScope:"per-group"(default) — each group keeps its own session (unchanged behavior)"main"— group chats route into the agent's main session (agent:<id>:main)Also supported as a per-binding override (
bindings[].session.groupScope) so a specific conversation can opt in/out independently of the global default — the same shape as the existing per-bindingdmScope.Why
Closes #7524. There is currently no way to fold a group chat into the agent's main session for unified context;
dmScopeonly covers DMs. This mirrors that capability for groups.How
src/routing/session-key.ts—buildAgentPeerSessionKeyreturns the main key for a non-direct (group) peer whengroupScope === "main", mirroring the existingdmScope === "main"direct-peer fold.src/routing/resolve-route.ts—resolveAgentRoutereadssession.groupScopeand the per-binding override (effectiveGroupScope).src/infra/outbound/base-session-key.ts— outbound replies honor it too, so they route consistently with inbound.zod-schema.session.ts,zod-schema.agents.ts(per-binding override),types.base.ts,types.agents.ts.Tests
src/routing/session-key.test.tsandsrc/routing/resolve-route.test.tscover: globalmainfolds a group peer into main; defaultper-groupkeepsagent:<id>:telegram:group:<id>; per-binding override in both directions (fold one group against aper-groupdefault; keep one group separate against amaindefault); and a guard thatgroupScopedoes not affect direct peers.config:docs:gen) andconfig:docs:checkpasses.Compatibility
Optional; unset =
per-group= identical to current behavior. No migration needed.