fix(agents): derive conversation scope from trusted group facts by steipete · Pull Request #99816 · openclaw/openclaw

steipete · 2026-07-04T05:03:47Z

Summary

ResolvedConversationCapabilityProfile.conversation.scope now classifies the conversation audience exclusively from server-verified or trust-checked inputs, in this precedence:

Live chat type: direct -> direct; group/channel -> shared.
Server-derived session keys: when runSessionKey or sessionKey names a group/channel conversation (new sessionKeyNamesGroupConversation helper in src/agents/agent-tools.policy.ts), scope is shared even without a live chat type. This also covers runs whose policy key is a sandbox alias while the live run session is a group.
Trust-checked group facts: caller group facts count only when resolveTrustedGroupId vouched for them. When the trust check dropped the caller group id, scope resolves unknown instead of shared.

Previously the fallback branch (no usable chat type) read the raw caller-supplied groupId/groupChannel/groupSpace params, while the same profile published those fields trust-checked (null when dropped). An unvouched caller-controlled group value could therefore classify an unknown-audience conversation as shared in the exact field that conversation identity modes (#98542) and scoped brokered credentials (#98549 / #98707) are specified to consume for audience and credential decisions. The profile also now computes the masked groupChannel/groupSpace once and reuses them for group-policy resolution, the published conversation fields, and scope, so the three can no longer diverge.

conversation.scope has zero consumers on current main (verified by grep across src/, extensions/, packages/), so the semantics change is contract-safe now and becomes the guaranteed baseline before the first consumers land.

Proof (local)

pnpm tsgo clean.
pnpm vitest run src/agents/conversation-capability-profile.test.ts — 6 passed, including new regressions:
- dropped caller group facts -> scope: "unknown", published group fields null;
- group-scoped session key without live chat type -> shared;
- live run session key behind a sandbox policy key -> shared;
- session-vouched caller group -> shared, dropped: false.
oxfmt --check clean on touched files.

Non-goals

No change to group tool-policy resolution or resolveTrustedGroupId semantics; only scope classification and shared computation of the already-masked fields.
No new config or public API surface. sessionKeyNamesGroupConversation is an internal helper.

Scope fell back to caller-supplied groupId/groupChannel/groupSpace when chatType was unknown, while the profile publishes those same fields trust-checked (null when dropped). A dropped caller group could classify an unknown-audience conversation as shared for downstream audience and credential decisions. Scope now classifies only from live chat type, server-derived session keys (new sessionKeyNamesGroupConversation), and trust-checked group facts; dropped groups resolve to unknown.

clawsweeper · 2026-07-04T05:09:50Z

Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 1:12 AM ET / 05:12 UTC.

Summary
The PR changes the internal conversation capability profile so conversation.scope is derived from live chat type, server-derived session keys, and trust-checked group facts instead of raw caller group fields.

PR surface: Source +35, Tests +54. Total +89 across 3 files.

Reproducibility: yes. from source inspection: a DM session with caller-supplied group facts is trust-dropped for published group fields, while current main still lets raw group fields make conversation.scope shared. I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

Current scope consumers: 0 runtime consumers outside profile/tests found on current main. The semantics change is less likely to break existing users because dependent identity and credential consumers have not landed yet.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] This touches a security/session-state decision surface that planned identity and credential work will consume, so explicit maintainer review should confirm this is the intended baseline before those consumers land.

Maintainer options:

Decide the mitigation before merge
Land this narrow central profile fix if maintainers agree with the trust precedence, then let the dependent identity and credential PRs consume the corrected scope semantics.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] The protected maintainer label and security/session-scope semantics call for maintainer review and merge judgment; there is no narrow automated repair to queue.

Security
Cleared: The diff narrows an internal trust boundary and adds focused regression tests; it does not change dependencies, workflows, secrets handling, package metadata, or external code execution paths.

Review details

Best possible solution:

Land this narrow central profile fix if maintainers agree with the trust precedence, then let the dependent identity and credential PRs consume the corrected scope semantics.

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

Yes from source inspection: a DM session with caller-supplied group facts is trust-dropped for published group fields, while current main still lets raw group fields make conversation.scope shared. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes: centralizing masked group facts and session-key precedence inside the capability profile is the narrowest maintainable fix. Per-caller fixes would duplicate the trust check and are more likely to drift.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8822b66952b7.

Label changes

Label changes:

add P2: This is a normal-priority internal bug/security-hardening fix for agent conversation scope with limited immediate blast radius because current main has no runtime consumers of the scope field.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is maintainer-labeled and changes an internal profile classifier, so the external-contributor real behavior proof gate is not applied; source and regression-test evidence are the relevant proof here.

Label justifications:

P2: This is a normal-priority internal bug/security-hardening fix for agent conversation scope with limited immediate blast radius because current main has no runtime consumers of the scope field.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is maintainer-labeled and changes an internal profile classifier, so the external-contributor real behavior proof gate is not applied; source and regression-test evidence are the relevant proof here.

Evidence reviewed

PR surface:

Source +35, Tests +54. Total +89 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	47	12	+35
Tests	1	54	0	+54
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	101	12	+89

What I checked:

Current main source behavior: Current main computes trustedGroup and publishes dropped group fields as null, but scope still calls resolveConversationScope(params), whose fallback reads raw groupId, groupChannel, and groupSpace. (src/agents/conversation-capability-profile.ts:183, 8822b66952b7)
Trust helper contract: The existing group trust path fails closed when no server-derived session or spawned context can vouch for the caller group id, which is the invariant the PR extends to scope classification. (src/agents/agent-tools.policy.ts:282, 8822b66952b7)
PR head implementation: The PR masks groupChannel and groupSpace once, reuses the masked values for group policy and published conversation fields, and passes only trusted facts plus session keys into scope resolution. (src/agents/conversation-capability-profile.ts:189, cbd71c8b27a7)
PR head scope precedence: The new scope resolver keeps live chat type first, then treats group/channel session keys as shared, then returns unknown when the caller group was dropped before considering remaining trusted group facts. (src/agents/conversation-capability-profile.ts:361, cbd71c8b27a7)
PR head regression coverage: The added tests cover dropped caller group facts resolving to unknown, group-scoped session keys without live chat type resolving to shared, sandbox policy keys with live group run keys, and vouched caller group facts. (src/agents/conversation-capability-profile.test.ts:85, cbd71c8b27a7)
Caller surface checked: Embedded run attempts build one runtime capability profile with both sandbox policy key and live run session key, so fixing the central profile resolver covers the relevant caller path instead of duplicating logic in each caller. (src/agents/embedded-agent-runner/run/attempt.ts:1249, 8822b66952b7)

Likely related people:

steipete: GitHub PR metadata shows @steipete authored and merged the scoped profile foundation, and current-line blame for the profile/scope implementation points to Peter Steinberger-authored history in this checkout. (role: introduced behavior and recent area contributor; confidence: high; commits: 2a50fe904009, 147b25a92790, 4a354f76c1bf; files: src/agents/conversation-capability-profile.ts, src/agents/conversation-capability-profile.test.ts, src/agents/agent-tools.policy.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.

…claw#99816) Scope fell back to caller-supplied groupId/groupChannel/groupSpace when chatType was unknown, while the profile publishes those same fields trust-checked (null when dropped). A dropped caller group could classify an unknown-audience conversation as shared for downstream audience and credential decisions. Scope now classifies only from live chat type, server-derived session keys (new sessionKeyNamesGroupConversation), and trust-checked group facts; dropped groups resolve to unknown.

openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels Jul 4, 2026

steipete merged commit 23b878a into main Jul 4, 2026
143 of 151 checks passed

steipete deleted the fix/conversation-scope-trusted-group branch July 4, 2026 09:53

github-actions Bot mentioned this pull request Jul 4, 2026

📡 Upstream Digest — 2026-07-04 10:01 UTC curtismercier/openclaw-mods#1203

Open

steipete mentioned this pull request Jul 5, 2026

feat(workboard): refresh Control UI from live changes #99051

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): derive conversation scope from trusted group facts#99816

fix(agents): derive conversation scope from trusted group facts#99816
steipete merged 1 commit into
mainfrom
fix/conversation-scope-trusted-group

steipete commented Jul 4, 2026

Uh oh!

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jul 4, 2026

Summary

Proof (local)

Non-goals

Uh oh!

clawsweeper Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading