Skip to content

refactor(config): add canonical group-policy scope-tree resolver and migrate the first six channels#106846

Merged
steipete merged 10 commits into
mainfrom
claude/chanref-scope-batch2
Jul 14, 2026
Merged

refactor(config): add canonical group-policy scope-tree resolver and migrate the first six channels#106846
steipete merged 10 commits into
mainfrom
claude/chanref-scope-batch2

Conversation

@steipete

@steipete steipete commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

16 channel plugins each answer the same group-policy questions — effective requireMention, tool policy (including sender-scoped tools), intro hints — through a mix of one shared resolver and private per-channel walkers over channel-specific config nests. Same decision, many implementations, measurable drift.

Why This Change Was Made

This PR carries stage 1 plus the first two channel batches of the approved group-policy scope normalization (originally staged as #106819#106830 → this; consolidated to one landing because each stacked landing pays a full CI + generated-baseline cycle against a fast-moving main — the inner PRs will be closed as landed-via with this commit):

  1. Core ScopeTree resolver (src/config/group-scope-tree.ts): flat keyed scope nodes + channel-ordered paths (a nested tree cannot express telegram's wildcard-group-topic-outranks-exact-scalar precedence), most-specific-wins scalars with before-/after-config override ordering identical to the existing shared resolver, per-level toolsBySendertools whole-object cascade reusing resolveToolsBySender verbatim, plus buildChannelGroupsScopeTree (13+ channels share the flat groups shape) and resolveScopeKeyCaseInsensitive (exact beats case-insensitive; wildcard lives in the defaults node). Exported via openclaw/plugin-sdk/channel-policy; surface budgets bumped by the exact delta with citation comments.
  2. Batch 1 — googlechat, imessage, whatsapp (pure delegates): adapters now build a tree and emit a single-scope path; account scoping stays core-owned via resolveChannelGroups, preserving the single-account empty-groups:{} fallback quirk. Each plugin module ended smaller than before.
  3. Batch 2 — line, qqbot, mattermost (one twist each): LINE's prefixed-key candidate pre-resolution feeds the scope path; QQBot's case-insensitive group match resolves at path-build time; Mattermost forwards its chatmode-derived requireMention override as a resolver parameter with after-config ordering (groups config beats chatmode; chatmode beats the final default).

User Impact

None — strict behavior parity, pinned by table-driven tests per channel (exact/wildcard/absent, account override, single-account fallback, sender-scoped tools with messageProvider gating, LINE candidate precedence, QQBot exact-beats-CI and wildcard-never-CI-matched, Mattermost chatmode-vs-groups precedence). Group ids are matched untrimmed, exactly as before. Six of sixteen channels are now on the canonical resolver; remaining batches (zalo/irc/nextcloud-talk, zalouser/slack/matrix, feishu/msteams, discord, telegram) follow, after which ~600 LOC of per-channel walkers and the duplicate discord fallback in core get deleted.

Evidence

  • Blacksmith Testbox per stage: group-scope-tree/group-policy/plugin-sdk suites, then extensions/googlechat extensions/imessage extensions/whatsapp, then extensions/line extensions/qqbot extensions/mattermost (1,572 tests in the batch-2 run) — all green; check:changed lanes green each round; plugin-sdk-surface-report --check verified exact on the final rebased tree with the regenerated API baseline hash committed.
  • Autoreview (codex gpt-5.6-sol, xhigh, per stage): three clean passes — "patch is correct" 0.88 (core), 0.86 (batch 1), 0.86 (batch 2); zero actionable findings across all three.
  • Hosted CI green on the pre-rebase heads (one unrelated agents-core flake and one media download-timeout flake rerun to green); fresh CI running on the rebased head.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: line Channel integration: line channel: mattermost Channel integration: mattermost channel: whatsapp-web Channel integration: whatsapp-web scripts Repository scripts channel: qqbot size: L maintainer Maintainer-authored PR labels Jul 13, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 941f382888

ℹ️ 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".

};

function resolveScopePath(params: WhatsAppGroupContext) {
return params.groupId ? [params.groupId] : [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Trim flat group ids before scope lookup

When a WhatsApp group id arrives with incidental surrounding whitespace, this raw path no longer matches the configured exact group key, so the resolver falls through to wildcard/default policy for both requireMention and tool policy. The previous resolveChannelGroupRequireMention/resolveChannelGroupToolsPolicy path trimmed groupId before lookup, so this migration regresses those inputs; the same raw-path pattern appears in the other flat adapters migrated in this commit.

Useful? React with 👍 / 👎.

@steipete
steipete force-pushed the claude/chanref-scope-batch2 branch from 941f382 to 4547ab9 Compare July 13, 2026 22:13
@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 8:51 PM ET / July 14, 2026, 00:51 UTC.

Summary
Adds a canonical group-policy scope-tree resolver, exports it through the Plugin SDK, and migrates Google Chat, iMessage, WhatsApp, LINE, QQBot, and Mattermost policy resolution with expanded parity tests.

PR surface: Source +143, Tests +545, Generated 0, Other +3. Total +691 across 19 files.

Reproducibility: yes. —source inspection gives a high-confidence reproduction: configure one LINE account with groups: {}, root group:g123 and * entries with different mention policies, then resolve raw group ID g123 for that account. Current main reaches the wildcard policy; this head can select the prefixed exact root policy.

Review metrics: 1 noteworthy metric.

  • Plugin SDK surface: 8 exports added: 5 functions, 3 types. These exports establish a new compatibility-sensitive policy API that external plugins may adopt.

Root-cause cluster
Relationship: canonical
Canonical: #106846
Summary: This PR explicitly consolidates the two earlier stacked branches and contains their resolver and first-channel-batch commits.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Restore LINE account-scope candidate semantics and add the combined regression test.
  • Provide redacted live-channel output or logs demonstrating representative migrated policy behavior.
  • Obtain explicit maintainer confirmation for the permanent Plugin SDK surface.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides extensive tests and CI but no after-fix real-channel evidence; add redacted live output, logs, or recordings for representative LINE precedence, QQBot sender policy, and Mattermost override behavior. Redact credentials, phone numbers, IPs, private endpoints, and identifiers; updating the PR body should trigger review, otherwise ask a maintainer to comment @clawsweeper re-review. 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.

Risk before merge

  • [P1] Existing single-account LINE configurations with an empty account groups map can select a root prefixed exact policy instead of the root wildcard/default policy, changing whether messages require a mention.
  • [P1] The new ScopeTree helpers become a public Plugin SDK compatibility contract before the permanent abstraction and upgrade expectations have recorded maintainer confirmation.
  • [P1] Tests and CI do not demonstrate real runtime parity for sender-scoped tool policy or channel-specific precedence across the six migrated channels.
  • [P1] The current head is reported as dirty against main and needs a refreshed merge result before final approval.

Maintainer options:

  1. Restore LINE parity before merge (recommended)
    Resolve prefixed LINE candidates against the original account groups, add the combined fallback regression test, and rerun focused policy validation.
  2. Accept the LINE behavior change explicitly
    Document and test the changed upgrade semantics and obtain maintainer approval for configurations that will select a different root policy.
  3. Pause the public resolver rollout
    Keep the current channel implementations until the SDK boundary and all channel-specific precedence cases are fully settled.

Next step before merge

  • [P1] The protected maintainer-labeled PR requires a human SDK-boundary decision, a contributor-owned live-proof update, a clean rebase, and repair of the LINE compatibility finding before it can proceed.

Maintainer decision needed

  • Question: Should ScopeTree and its generic policy resolvers become a supported public Plugin SDK contract in this migration, or remain an internal bundled-plugin seam until the full channel normalization is complete?
  • Rationale: The PR adds eight externally visible SDK exports whose future compatibility and ownership cannot be inferred safely from implementation quality alone.
  • Likely owner: steipete — They authored the proposed abstraction and channel migration and are the strongest available routing candidate for its intended long-term boundary.
  • Options:
    • Approve the narrow SDK contract (recommended): Treat the eight exports as the canonical plugin-facing policy seam after the LINE parity defect is repaired and live proof is supplied.
    • Keep the resolver internal for now: Stage the normalization behind a bundled-plugin-only facade and publish an external contract only after all channel precedence requirements are settled.

Security
Needs attention: The LINE scope-selection regression can apply a less restrictive mention-gating policy than current main for an existing configuration.

Review findings

  • [P1] Preserve LINE candidate lookup before account fallback — extensions/line/src/group-policy.ts:13
Review details

Best possible solution:

Preserve LINE’s candidate lookup against the original account-scoped groups before applying generic fallback, add the combined empty-account/root-prefixed regression case, obtain explicit approval for the public SDK contract, rebase cleanly, and provide representative redacted live-channel parity evidence.

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

Yes—source inspection gives a high-confidence reproduction: configure one LINE account with groups: {}, root group:g123 and * entries with different mention policies, then resolve raw group ID g123 for that account. Current main reaches the wildcard policy; this head can select the prefixed exact root policy.

Is this the best way to solve the issue?

No—not at the current head. A shared resolver is maintainable, but each adapter must preserve its existing candidate-resolution order, and the public Plugin SDK boundary needs explicit ownership approval.

Full review comments:

  • [P1] Preserve LINE candidate lookup before account fallback — extensions/line/src/group-policy.ts:13
    Resolve prefixed candidates against the original account-scoped groups, not the already fallback-expanded tree.scopes. With a sole account whose groups is empty, this line exposes root group:<id> entries that current main cannot candidate-match before fallback, potentially replacing the root wildcard mention policy with a root exact policy. Add a regression covering empty account groups plus differing root prefixed and wildcard policies. This was present on the previously reviewed eba3198 head and is a late discovery.
    Confidence: 0.97
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0934f561b392.

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P2: This is an unmerged but substantial policy refactor with a bounded compatibility defect and required proof work, not a current production emergency.
  • merge-risk: 🚨 compatibility: The LINE migration can change policy selection for an existing single-account empty-groups configuration, and the PR also adds a permanent public SDK surface.
  • merge-risk: 🚨 message-delivery: Selecting an exact root policy instead of the wildcard policy can change whether LINE group messages are accepted without a mention.
  • merge-risk: 🚨 security-boundary: The refactored resolver governs mention gating and sender-scoped tool permissions, so incorrect scope selection can apply a less restrictive policy.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides extensive tests and CI but no after-fix real-channel evidence; add redacted live output, logs, or recordings for representative LINE precedence, QQBot sender policy, and Mattermost override behavior. Redact credentials, phone numbers, IPs, private endpoints, and identifiers; updating the PR body should trigger review, otherwise ask a maintainer to comment @clawsweeper re-review. 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 +143, Tests +545, Generated 0, Other +3. Total +691 across 19 files.

View PR surface stats
Area Files Added Removed Net
Source 10 228 85 +143
Tests 7 568 23 +545
Docs 0 0 0 0
Config 0 0 0 0
Generated 1 2 2 0
Other 1 8 5 +3
Total 19 806 115 +691

Security concerns:

  • [medium] Incorrect fallback can weaken LINE mention gating — extensions/line/src/group-policy.ts:13
    Searching fallback-expanded root scopes for a prefixed key may select requireMention: false where current behavior uses a stricter wildcard policy, causing messages without mentions to activate the agent.
    Confidence: 0.94

What I checked:

  • Current LINE behavior: Current main resolves LINE prefixed candidate keys against the original account-scoped groups before the generic resolver applies its single-account empty-map fallback. (extensions/line/src/group-policy.ts, 0934f561b392)
  • Proposed LINE behavior: The PR first builds a tree through resolveChannelGroups, then searches tree.scopes; an empty sole-account groups map may therefore expose root prefixed exact keys that the current candidate lookup cannot see. (extensions/line/src/group-policy.ts:13, eba3198ab027)
  • Regression coverage gap: The added LINE tests cover candidate precedence and account fallback separately, but not an empty sole-account map combined with a root prefixed exact key and a different root wildcard policy. (extensions/line/src/group-keys.test.ts:65, eba3198ab027)
  • Public API expansion: The PR exports five resolver/builder functions and three ScopeTree types through openclaw/plugin-sdk/channel-policy, making the abstraction a compatibility-sensitive external contract. (src/plugin-sdk/channel-policy.ts:51, eba3198ab027)
  • Proof record: The PR body reports Testbox suites, checks, CI, and automated reviews, but contains no after-fix live channel output, recording, or runtime logs for the migrated policy paths. (eba3198ab027)
  • Review continuity: The LINE account-fallback concern is present on the same eba3198 head examined by the latest completed ClawSweeper review, so it is reported as a late finding rather than author churn. (extensions/line/src/group-policy.ts:13, eba3198ab027)

Likely related people:

  • steipete: Authored the complete ScopeTree series and its six-channel migration, and is the clearest available owner for the intended precedence semantics and Plugin SDK boundary; current-main ownership history could not be independently expanded beyond the supplied review context. (role: proposed resolver and channel-migration owner; confidence: medium; commits: 9e658efc8387, 64b7dbdef1f7, 2c7f72fc184a; files: src/config/group-scope-tree.ts, src/plugin-sdk/channel-policy.ts, extensions/line/src/group-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.
Review history (3 earlier review cycles)
  • reviewed 2026-07-13T22:31:48.296Z sha 4547ab9 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-13T23:22:52.043Z sha b5ea4c1 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-14T00:38:40.442Z sha eba3198 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 13, 2026
@steipete
steipete force-pushed the claude/chanref-scope-batch2 branch from 4547ab9 to b5ea4c1 Compare July 13, 2026 23:10
@steipete steipete changed the title refactor(channels): migrate line, qqbot, and mattermost group policy onto the scope tree refactor(config): add canonical group-policy scope-tree resolver and migrate the first six channels Jul 13, 2026
@steipete
steipete force-pushed the claude/chanref-scope-batch2 branch from b5ea4c1 to eba3198 Compare July 14, 2026 00:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eba3198ab0

ℹ️ 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".

accountId: params.accountId,
const tree = buildChannelGroupsScopeTree(params.cfg, "line", params.accountId);
const matchedKey = resolveExactLineGroupConfigKey({
groups: tree.scopes,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep LINE prefix lookup on the original account groups

In single-account LINE configs with accounts.<id>.groups: {} and root groups containing a prefixed key such as group:g123, buildChannelGroupsScopeTree has already fallen back to the root groups before this lookup runs, so raw g123 now resolves to that root prefixed entry. The previous code did the prefixed-key lookup against resolveLineGroupsConfig first, which returned the empty account map, and only then let the shared resolver fall back to the root wildcard/default; this can flip existing migrated account configs from the root default mention policy to a root exact prefixed policy.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 14, 2026
@steipete
steipete force-pushed the claude/chanref-scope-batch2 branch from eba3198 to f2bafed Compare July 14, 2026 01:18
@steipete

Copy link
Copy Markdown
Contributor Author

Landing via admin merge under maintainer authorization. Evidence: on the previous head (eba3198), every hosted check passed except check-dependencies, which flagged two exports in src/agents/embedded-agent-runner/run/attempt-prompt-context.ts — a file this PR does not touch, owned by a separate in-flight stream that has broken and re-fixed the deadcode gate several times tonight (#106917/#106925 fixed the prior round). Content validation: Blacksmith Testbox suites green per stage (core resolver, batch 1, batch 2 — 1,572 tests in the batch-2 run), check:changed lanes green each round, plugin-sdk-surface-report --check verified exact on the rebased tree, and three clean codex autoreviews (0.88/0.86/0.86, zero actionable findings). Delta since the evaluated head is a rebase whose conflicts were confined to the exact-count surface budgets (delta-resolved: main +1/+1 drift absorbed; final 10665/5371 verified by the checker) and the regenerated API baseline hash. Inner stacked PRs #106819 and #106830 will be closed as landed via this commit.

@steipete
steipete force-pushed the claude/chanref-scope-batch2 branch from f2bafed to bcb8e5f Compare July 14, 2026 01:23
@steipete
steipete merged commit b9c4815 into main Jul 14, 2026
52 checks passed
@steipete
steipete deleted the claude/chanref-scope-batch2 branch July 14, 2026 01:24
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 14, 2026
…migrate the first six channels (openclaw#106846)

* feat(config): add canonical group-policy scope-tree resolver

* chore(plugin-sdk): refresh API baseline hash for scope-tree exports

* refactor(channels): migrate googlechat, imessage, and whatsapp group policy onto the scope tree

* chore(plugin-sdk): refresh API baseline hash for groups scope-tree builder

* refactor(channels): migrate line, qqbot, and mattermost group policy onto the scope tree

* chore(plugin-sdk): refresh API baseline hash for case-insensitive scope key helper

* chore(plugin-sdk): refresh API baseline hash after rebase onto current main

* chore(plugin-sdk): refresh API baseline hash after rebase

* chore(plugin-sdk): refresh API baseline hash after rebase

* chore(plugin-sdk): refresh API baseline hash after rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: line Channel integration: line channel: mattermost Channel integration: mattermost channel: qqbot channel: whatsapp-web Channel integration: whatsapp-web docs Improvements or additions to documentation maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant