refactor(config): add canonical group-policy scope-tree resolver and migrate the first six channels#106846
Conversation
There was a problem hiding this comment.
💡 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] : []; |
There was a problem hiding this comment.
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 👍 / 👎.
941f382 to
4547ab9
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 8:51 PM ET / July 14, 2026, 00:51 UTC. Summary 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 Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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
Maintainer decision needed
Security Review findings
Review detailsBest 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 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0934f561b392. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +143, Tests +545, Generated 0, Other +3. Total +691 across 19 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
Review history (3 earlier review cycles) |
4547ab9 to
b5ea4c1
Compare
b5ea4c1 to
eba3198
Compare
There was a problem hiding this comment.
💡 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, |
There was a problem hiding this comment.
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 👍 / 👎.
eba3198 to
f2bafed
Compare
|
Landing via admin merge under maintainer authorization. Evidence: on the previous head (eba3198), every hosted check passed except |
…policy onto the scope tree
…onto the scope tree
f2bafed to
bcb8e5f
Compare
…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
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):
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-leveltoolsBySender→toolswhole-object cascade reusingresolveToolsBySenderverbatim, plusbuildChannelGroupsScopeTree(13+ channels share the flatgroupsshape) andresolveScopeKeyCaseInsensitive(exact beats case-insensitive; wildcard lives in the defaults node). Exported viaopenclaw/plugin-sdk/channel-policy; surface budgets bumped by the exact delta with citation comments.resolveChannelGroups, preserving the single-account empty-groups:{}fallback quirk. Each plugin module ended smaller than before.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
messageProvidergating, 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
group-scope-tree/group-policy/plugin-sdksuites, thenextensions/googlechat extensions/imessage extensions/whatsapp, thenextensions/line extensions/qqbot extensions/mattermost(1,572 tests in the batch-2 run) — all green;check:changedlanes green each round;plugin-sdk-surface-report --checkverified exact on the final rebased tree with the regenerated API baseline hash committed.