feat(discord): resolve trusted principals via identity links#70944
feat(discord): resolve trusted principals via identity links#70944ericberic wants to merge 5 commits into
Conversation
Greptile SummaryThis PR extends the Discord channel to resolve a sender's Discord user ID to a canonical Two minor items worth noting:
Confidence Score: 4/5Safe to merge; no correctness or security issues found. One displaced import and an undocumented PluralKit behaviour worth addressing before or after merge. All findings are P2 (style and documentation). The core resolution logic is correct, well-tested across 5+ unit-test cases, and the refactoring of resolveCanonicalIdentityFromLinks preserves the existing internal call-site cleanly. Score is 4 rather than 5 only because the PluralKit trust semantics are non-obvious and currently undocumented in the code. extensions/discord/src/monitor/agent-components.ts (misplaced import), extensions/discord/src/monitor/sender-identity.ts (PluralKit trust comment) Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/discord/src/monitor/agent-components.ts
Line: 119
Comment:
**Import placed mid-file after function definitions**
The static import of `resolveDiscordTrustedPrincipalFromUserId` is placed at line 119, inside a block of function definitions, rather than at the top of the file with all other imports. While static imports are hoisted by the JS runtime, placing them here breaks the file's import-at-top convention and is likely a minor merge artifact.
Move this line to the top-level import block alongside the other imports.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/discord/src/monitor/sender-identity.ts
Line: 62-65
Comment:
**PluralKit trust lookup uses relay-bot ID, not the proxied member ID**
`trustedPrincipal` is always derived from `params.author.id` (the PluralKit relay bot's Discord snowflake), even though for PK messages `sender.id` is set to `pkMember.id`. Operators configuring identity links must map the relay bot's ID, which grants the same principal to *all* members of that PK system. If per-member granularity is ever needed, the lookup would need to be repeated for `memberId` after the PK branch is entered.
This appears intentional (the test asserts exactly this behaviour with `system_owner: ["discord:444555666"]`), but a brief code comment clarifying the relay-bot-as-trust-anchor semantics would help future maintainers.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Discord: resolve trusted principals via ..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1277ec4541
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…uting blocked on it Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Codex review: found issues before merge. Reviewed June 7, 2026, 1:11 AM ET / 05:11 UTC. Summary PR surface: Source +61, Tests +195, Docs +1, Generated 0. Total +257 across 21 files. Reproducibility: not applicable. as a bug reproduction: this is a feature PR. The PR body does provide a focused terminal proof for the changed resolver plus inbound metadata builder. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land a rebased version only after maintainers approve the trusted identity metadata and Plugin SDK export contract, with release notes kept in the PR body or squash message rather than Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this is a feature PR. The PR body does provide a focused terminal proof for the changed resolver plus inbound metadata builder. Is this the best way to solve the issue? Unclear until maintainer contract review: the implementation is focused and reuses the existing identity-link resolver, but maintainers need to decide whether this Discord-specific Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against fa614d0907e8. Label changesLabel justifications:
Evidence reviewedPR surface: Source +61, Tests +195, Docs +1, Generated 0. Total +257 across 21 files. View PR surface stats
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
|
1eca439 to
b2ea219
Compare
- Move sender-identity import to top-level import block in agent-components.ts - Add PluralKit relay-bot trust-anchor comment to sender-identity.ts - Remove stale oxlint-disable-next-line directive in process test (rule no longer applies after upstream rebase) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… messages For PluralKit-proxied messages, use the real sender's Discord user ID (pkInfo.sender) for identity-link trust lookups instead of the relay-bot webhook ID (author.id). Falls back to author.id when sender is absent. This lets operators configure identity links using actual Discord snowflakes rather than per-system-per-server webhook IDs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
b2ea219 to
b69f66b
Compare
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Gilded Lint Imp Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
|
This pull request has been automatically marked as stale due to inactivity. |
…uting blocked on it Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Completes identity-links support for the Discord plugin. When a Discord message, component interaction, or native slash-command context arrives, the sender's stable Discord user ID is resolved through the agent's
session.identityLinksconfig and passed into trusted inbound metadata assender_principal.Changes
resolveDiscordTrustedPrincipalFromUserId().sender_principalin the trusted inbound metadata prompt only when an operator-configured identity link matches.resolveCanonicalIdentityFromLinks()helper throughopenclaw/plugin-sdk/routinginstead of duplicating identity-link lookup logic.Real behavior proof
Behavior addressed: Discord senders mapped in
session.identityLinksnow produce trusted prompt metadata withsender_principal, while raw sender IDs and spoofable display names remain excluded from the trusted metadata block.Real environment tested: Local OpenClaw source checkout on macOS/Darwin, branch
feat/discord-resolve-trusted-principals-via-identity-links, running the changed Discord sender identity resolver and inbound metadata builder through Node + tsx from the working tree.Exact steps or command run after this patch:
Evidence after fix: Terminal output from the command above:
Observed result after fix: The trusted metadata payload emitted
sender_principal = "alice"from the configured stablediscord:123456789identity link, while raw sender ID and spoofable display-name values were not emitted in trusted metadata.What was not tested: End-to-end delivery through a live Discord bot/account was not tested in this local pass. Focused local Vitest attempts were stopped because local bundling fanned out and ran for several minutes; GitHub CI should provide the broad lane proof after the rebased branch is pushed.
Verification
node --import tsx --input-type=moduledirect Discord identity/inbound metadata probe above.node --import tsx --input-type=moduleimport smoke formessage-handler.context.ts,message-handler.preflight.ts,native-command-context.ts,native-command.ts,agent-components.dispatch.ts, andsrc/plugin-sdk/routing.ts.node_modules/.bin/oxfmt --check CHANGELOG.md docs/plugins/sdk-migration.md docs/plugins/sdk-subpaths.md extensions/discord/src/monitor/agent-components.dispatch.ts extensions/discord/src/monitor/message-handler.context.ts extensions/discord/src/monitor/message-handler.preflight.ts extensions/discord/src/monitor/native-command-context.ts extensions/discord/src/monitor/native-command.ts extensions/discord/src/monitor/sender-identity.ts extensions/discord/src/monitor/sender-identity.test.ts src/auto-reply/reply/inbound-context.ts src/auto-reply/reply/inbound-meta.ts src/auto-reply/reply/inbound-meta.test.ts src/auto-reply/templating.ts src/plugin-sdk/routing.ts src/routing/session-key.ts src/routing/session-key.test.tsgit diff --checknode --max-old-space-size=8192 --import tsx scripts/generate-plugin-sdk-api-baseline.ts --writeSafety notes
session.identityLinksand stable Discord snowflake IDs.pkInfo.senderwhen available so identity links target the real Discord sender instead of spoofable display metadata.Disclosure
AI-assisted, per the repository contribution policy.