Skip to content

feat(discord): resolve trusted principals via identity links#70944

Open
ericberic wants to merge 5 commits into
openclaw:mainfrom
ericberic:feat/discord-resolve-trusted-principals-via-identity-links
Open

feat(discord): resolve trusted principals via identity links#70944
ericberic wants to merge 5 commits into
openclaw:mainfrom
ericberic:feat/discord-resolve-trusted-principals-via-identity-links

Conversation

@ericberic

@ericberic ericberic commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

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.identityLinks config and passed into trusted inbound metadata as sender_principal.

Changes

  • Adds Discord sender-principal resolution through resolveDiscordTrustedPrincipalFromUserId().
  • Threads the resolved principal through current Discord preflight, turn-context, component, and native-command paths.
  • Renders sender_principal in the trusted inbound metadata prompt only when an operator-configured identity link matches.
  • Reuses the shared resolveCanonicalIdentityFromLinks() helper through openclaw/plugin-sdk/routing instead of duplicating identity-link lookup logic.
  • Documents the new routing helper in plugin SDK docs and refreshes the generated SDK API baseline hash.

Real behavior proof

Behavior addressed: Discord senders mapped in session.identityLinks now produce trusted prompt metadata with sender_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:

$ node --import tsx --input-type=module <<'EOF'
import { resolveDiscordSenderIdentity } from './extensions/discord/src/monitor/sender-identity.ts';
import { buildInboundMetaSystemPrompt } from './src/auto-reply/reply/inbound-meta.ts';
const sender = resolveDiscordSenderIdentity({
  author: { id: '123456789', username: 'alice', globalName: 'Spoofable Display Name' },
  member: { nickname: 'Spoofable Nickname' },
  pluralkitInfo: null,
  identityLinks: { alice: ['discord:123456789'] },
});
const prompt = buildInboundMetaSystemPrompt({
  TrustedSenderPrincipal: sender.trustedPrincipal,
  SenderId: sender.id,
  SenderName: sender.name,
  OriginatingChannel: 'discord',
  Provider: 'discord',
  Surface: 'discord',
  ChatType: 'direct',
}, { includeFormattingHints: false });
const jsonText = prompt.match(/```json\n([\s\S]*?)\n```/)?.[1];
const payload = JSON.parse(jsonText);
console.log(JSON.stringify({
  sender_principal: payload.sender_principal,
  sender_id_exposed: Object.hasOwn(payload, 'sender_id'),
  spoofable_name_exposed: JSON.stringify(payload).includes('Spoofable'),
}, null, 2));
EOF

Evidence after fix: Terminal output from the command above:

{
  "sender_principal": "alice",
  "sender_id_exposed": false,
  "spoofable_name_exposed": false
}

Observed result after fix: The trusted metadata payload emitted sender_principal = "alice" from the configured stable discord:123456789 identity 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=module direct Discord identity/inbound metadata probe above.
  • node --import tsx --input-type=module import smoke for message-handler.context.ts, message-handler.preflight.ts, native-command-context.ts, native-command.ts, agent-components.dispatch.ts, and src/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.ts
  • git diff --check
  • node --max-old-space-size=8192 --import tsx scripts/generate-plugin-sdk-api-baseline.ts --write

Safety notes

  • Principal resolution is based only on operator-configured session.identityLinks and stable Discord snowflake IDs.
  • For PluralKit messages, the resolver prefers pkInfo.sender when available so identity links target the real Discord sender instead of spoofable display metadata.
  • Trusted metadata contains the configured canonical principal only; raw sender IDs and human display names remain in untrusted/user context surfaces.

Disclosure

AI-assisted, per the repository contribution policy.

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: M labels Apr 24, 2026
@greptile-apps

greptile-apps Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the Discord channel to resolve a sender's Discord user ID to a canonical sender_principal via the agent's identityLinks config, threading it through preflight, process, and native-command handlers into the inbound meta system prompt. The refactoring is well-scoped — resolveCanonicalIdentityFromLinks is cleanly extracted from the private resolveLinkedPeerId and re-exported through the plugin SDK without duplicating logic.

Two minor items worth noting:

  • A static import in agent-components.ts was placed at line 119 (mid-file, after function definitions) instead of at the top-level import block — likely a merge artifact.
  • For PluralKit messages, trust is derived from the relay-bot's Discord ID (params.author.id) while sender.id resolves to the PK member ID; this is consistent with the test but means all proxied members of a PK system share the same principal, which may deserve a code comment for future maintainers.

Confidence Score: 4/5

Safe 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 AI
This 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

Comment thread extensions/discord/src/monitor/agent-components.ts Outdated
Comment thread extensions/discord/src/monitor/sender-identity.ts

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

Copy link
Copy Markdown

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: 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".

Comment thread extensions/discord/src/monitor/sender-identity.ts
ericberic added a commit to ericberic/openclaw that referenced this pull request Apr 24, 2026
…uting blocked on it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@clawsweeper

clawsweeper Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 7, 2026, 1:11 AM ET / 05:11 UTC.

Summary
The PR adds Discord identity-link resolution for message, component, and native-command senders, emits the mapped canonical identity as trusted inbound sender_principal, exports the shared identity-link resolver through plugin-sdk/routing, and updates focused tests plus SDK docs.

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.

  • Trusted metadata fields: 1 added. Adding sender_principal changes what agents may treat as authoritative system metadata.
  • Plugin SDK exports: 1 added. Exporting resolveCanonicalIdentityFromLinks through plugin-sdk/routing expands the public plugin contract.
  • Release-owned files: 1 direct CHANGELOG entry added. Normal PRs should leave changelog generation to the release flow to avoid stale conflicts and attribution drift.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

  • Remove the direct CHANGELOG.md entry and keep release-note context in the PR body or squash message.
  • Rebase onto current main and resolve the dirty merge state before maintainer approval.

Risk before merge

  • [P1] This changes a trusted system-prompt metadata boundary: maintainers need to approve sender_principal as the supported schema and confirm it should not wait for the broader requester-identity design in feat(auto-reply): expose safe requester identity metadata #82353 or Feature: Include trusted sender_name in inbound metadata envelope #45427.
  • [P1] The PR publicly exports resolveCanonicalIdentityFromLinks through plugin-sdk/routing, which makes the helper part of the third-party plugin API surface rather than an internal routing detail.
  • [P1] The live PR is stale against current main and includes a release-owned CHANGELOG.md edit, so it needs rebase/conflict cleanup before merge.

Maintainer options:

  1. Approve the trusted identity contract
    A maintainer can explicitly accept sender_principal as trusted prompt metadata sourced only from configured session.identityLinks and stable Discord user IDs before merging.
  2. Clean branch hygiene before merge
    Rebase onto current main, resolve the dirty merge state, and remove the direct CHANGELOG.md entry while keeping release-note context in the PR body or squash message.
  3. Defer to the broader identity design
    If maintainers prefer one cross-channel identity schema, pause or close this PR in favor of the broader requester-identity work already linked from the discussion.

Next step before merge

  • [P2] The remaining action is maintainer approval of the trusted metadata/Plugin SDK contract plus rebase cleanup, not a safe autonomous close or repair lane.

Security
Cleared: No concrete security defect or supply-chain concern was found in the diff, but the trusted metadata boundary still needs maintainer approval before merge.

Review findings

  • [P3] Remove the direct changelog entry — CHANGELOG.md:49
Review details

Best 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 CHANGELOG.md.

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 sender_principal schema and public SDK export are the best permanent API versus the broader requester-identity work.

Full review comments:

  • [P3] Remove the direct changelog entry — CHANGELOG.md:49
    CHANGELOG.md is release-owned in this repo; normal PRs should carry release-note context in the PR body or squash message instead. Please drop this added line so release generation owns the changelog and the branch has less stale-conflict surface.
    Confidence: 0.91

Overall correctness: patch is correct
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against fa614d0907e8.

Label changes

Label justifications:

  • P2: This is a normal-priority Discord identity metadata improvement with security-boundary review needs but no active outage shown.
  • merge-risk: 🚨 security-boundary: The PR changes trusted prompt metadata derived from identity links, so merge safety depends on approving the identity trust boundary.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a local OpenClaw checkout exercising the changed Discord identity resolver and trusted metadata builder with the expected sender_principal result.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a local OpenClaw checkout exercising the changed Discord identity resolver and trusted metadata builder with the expected sender_principal result.
Evidence reviewed

PR surface:

Source +61, Tests +195, Docs +1, Generated 0. Total +257 across 21 files.

View PR surface stats
Area Files Added Removed Net
Source 11 62 1 +61
Tests 6 209 14 +195
Docs 3 3 2 +1
Config 0 0 0 0
Generated 1 2 2 0
Other 0 0 0 0
Total 21 276 19 +257

What I checked:

  • PR diff adds trusted sender principal resolution: The PR head adds resolveDiscordTrustedPrincipalFromUserId, carries trustedPrincipal on DiscordSenderIdentity, and resolves PluralKit messages through pkInfo.sender when present. (extensions/discord/src/monitor/sender-identity.ts:34, b69f66be9fac)
  • PR diff adds trusted metadata field: The PR head adds sender_principal to the trusted inbound metadata payload from TrustedSenderPrincipal. (src/auto-reply/reply/inbound-meta.ts:411, b69f66be9fac)
  • Current main does not already implement the feature: Current main's trusted inbound metadata payload includes schema, account, channel, provider, surface, chat type, and response format, but no sender_principal; Discord sender identity also lacks trustedPrincipal. (src/auto-reply/reply/inbound-meta.ts:413, fa614d0907e8)
  • Plugin SDK boundary policy applies: The scoped Plugin SDK policy treats src/plugin-sdk as a public plugin contract and requires docs, export lists, package exports, API baseline, and contract checks to stay aligned when changing public subpaths. (src/plugin-sdk/AGENTS.md:1, fa614d0907e8)
  • Changelog policy applies: Root policy says CHANGELOG.md is release-owned and normal PRs should carry release-note context in the PR body, squash message, or direct commit instead. (AGENTS.md:230, fa614d0907e8)
  • Live PR state checked without mutation: The public PR API shows the PR is open, unmerged, non-draft, and still at head b69f66be9fac6cf19b1cf002c13db689464979dd; the provided GitHub context reports mergeability as dirty against current review data. (b69f66be9fac)

Likely related people:

  • steipete: Recent public commit history shows repeated work on Discord sender identity/preflight/native context, inbound metadata, and Plugin SDK docs around the affected surfaces. (role: recent area contributor; confidence: high; commits: 23716de4468b, 1507a9701b83, bbf50a406eee; files: extensions/discord/src/monitor/sender-identity.ts, extensions/discord/src/monitor/message-handler.preflight.ts, extensions/discord/src/monitor/native-command-context.ts)
  • Kaspre: Recent merged routing/session-key work touched the same session identity-link helper area that this PR promotes through the Plugin SDK. (role: adjacent routing contributor; confidence: medium; commits: 7eefb26bc8d8, eb7f3b7b50c5; files: src/routing/session-key.ts)
  • vincentkoc: The current shallow checkout blames the existing resolveLinkedPeerId and inbound metadata code to a recent repository snapshot by Vincent Koc, and adjacent channel-routing history includes recent work by the same author. (role: recent area contributor; confidence: medium; commits: 3060ebf0523a, d1cd74b2431d; files: src/routing/session-key.ts, src/auto-reply/reply/inbound-meta.ts, extensions/discord/src/monitor/agent-components.dispatch.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.

@ericberic ericberic force-pushed the feat/discord-resolve-trusted-principals-via-identity-links branch from 1eca439 to b2ea219 Compare May 6, 2026 13:51
@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. docs Improvements or additions to documentation labels May 6, 2026
ericberic and others added 5 commits May 18, 2026 06:58
- 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>
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 18, 2026
@ericberic ericberic force-pushed the feat/discord-resolve-trusted-principals-via-identity-links branch from b2ea219 to b69f66b Compare May 18, 2026 14:37
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels May 18, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 🥚 common Gilded Lint Imp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: collects tiny proofs.
Image traits: location flaky test forest; accessory shell-shaped keyboard; palette coral, mint, and warm cream; mood determined; pose holding its accessory up for inspection; shell paper lantern shell; lighting soft studio lighting; background quiet workflow signs.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Gilded Lint Imp in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 7, 2026
ericberic added a commit to ericberic/openclaw that referenced this pull request Jun 7, 2026
…uting blocked on it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: discord Channel integration: discord docs Improvements or additions to documentation merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M stale Marked as stale due to inactivity status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant