feat(discord): extract embed fields and footer for agent visibility#53288
feat(discord): extract embed fields and footer for agent visibility#53288jeremyknows wants to merge 2 commits into
Conversation
…Text Previously, `resolveDiscordEmbedText` only extracted title and description from the first embed. Many bot messages (status dashboards, alerts, moderation logs, cross-agent communication) put critical information in embed fields and footer, and may include multiple embeds per message. Changes: - Expand `resolveDiscordEmbedText` to extract title, description, fields (as "Name: Value"), and footer text - Extract from all embeds (not just embeds[0]) at all three call sites: resolveDiscordMessageText, snapshot resolution, and thread starter - Add `DiscordEmbedLike` type and export it for cross-file use - Add 5 test cases covering fields, footer, combined extraction, multiple embeds, and whitespace-only field filtering The embed text fallback chain is unchanged: agent sees message content first, then media placeholder, then embed text. Embed extraction only activates when message content is empty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
openclaw-barnacle
Bot
added
channel: discord
Greptile SummaryThis PR expands Discord embed extraction so agents can see fields and footers (not just title/description), and processes all embeds in a message rather than only the first one. The changes are focused, well-tested, and consistent across all three call sites ( Key changes:
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/discord/src/monitor/message-utils.test.ts
Line: 723-740
Comment:
**Test title claims "name or value" but only covers whitespace `name`**
The test description says "skips fields with whitespace-only name **or** value", but only a whitespace-only `name` is exercised. The implementation correctly short-circuits on either (`f.name?.trim() && f.value?.trim()`), so there's no bug — but the complementary case (whitespace-only `value`) isn't covered. Consider adding a second field to keep the promise made by the test name:
```suggestion
it("skips fields with whitespace-only name or value", () => {
const text = resolveDiscordMessageText(
asMessage({
content: "",
embeds: [
{
title: "Test",
fields: [
{ name: " ", value: "val" },
{ name: "Real", value: " " },
{ name: "Real", value: "data" },
],
},
],
}),
);
expect(text).toBe("Test\nReal: data");
});
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(discord): extract embed fields and ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("skips fields with whitespace-only name or value", () => { | ||
| const text = resolveDiscordMessageText( | ||
| asMessage({ | ||
| content: "", | ||
| embeds: [ | ||
| { | ||
| title: "Test", | ||
| fields: [ | ||
| { name: " ", value: "val" }, | ||
| { name: "Real", value: "data" }, | ||
| ], | ||
| }, | ||
| ], | ||
| }), | ||
| ); | ||
|
|
||
| expect(text).toBe("Test\nReal: data"); | ||
| }); |
There was a problem hiding this comment.
name
The test description says "skips fields with whitespace-only name or value", but only a whitespace-only name is exercised. The implementation correctly short-circuits on either (f.name?.trim() && f.value?.trim()), so there's no bug — but the complementary case (whitespace-only value) isn't covered. Consider adding a second field to keep the promise made by the test name:
| it("skips fields with whitespace-only name or value", () => { | |
| const text = resolveDiscordMessageText( | |
| asMessage({ | |
| content: "", | |
| embeds: [ | |
| { | |
| title: "Test", | |
| fields: [ | |
| { name: " ", value: "val" }, | |
| { name: "Real", value: "data" }, | |
| ], | |
| }, | |
| ], | |
| }), | |
| ); | |
| expect(text).toBe("Test\nReal: data"); | |
| }); | |
| it("skips fields with whitespace-only name or value", () => { | |
| const text = resolveDiscordMessageText( | |
| asMessage({ | |
| content: "", | |
| embeds: [ | |
| { | |
| title: "Test", | |
| fields: [ | |
| { name: " ", value: "val" }, | |
| { name: "Real", value: " " }, | |
| { name: "Real", value: "data" }, | |
| ], | |
| }, | |
| ], | |
| }), | |
| ); | |
| expect(text).toBe("Test\nReal: data"); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/discord/src/monitor/message-utils.test.ts
Line: 723-740
Comment:
**Test title claims "name or value" but only covers whitespace `name`**
The test description says "skips fields with whitespace-only name **or** value", but only a whitespace-only `name` is exercised. The implementation correctly short-circuits on either (`f.name?.trim() && f.value?.trim()`), so there's no bug — but the complementary case (whitespace-only `value`) isn't covered. Consider adding a second field to keep the promise made by the test name:
```suggestion
it("skips fields with whitespace-only name or value", () => {
const text = resolveDiscordMessageText(
asMessage({
content: "",
embeds: [
{
title: "Test",
fields: [
{ name: " ", value: "val" },
{ name: "Real", value: " " },
{ name: "Real", value: "data" },
],
},
],
}),
);
expect(text).toBe("Test\nReal: data");
});
```
How can I resolve this? If you propose a fix, please make it concise.…test Addresses Greptile review feedback: the test title promised coverage for whitespace-only name "or value" but only exercised the name case. Added a field with whitespace-only value to confirm both branches of the trim guard are tested. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This pull request has been automatically marked as stale due to inactivity. |
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: yes. By source inspection, current main and Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Replay the extraction onto the current Do we have a high-confidence way to reproduce the issue? Yes. By source inspection, current main and Is this the best way to solve the issue? No, not as this branch currently lands. The behavior direction is narrow and maintainable, but the patch must be replayed onto the current split helpers and preserve newer Components v2 fallback behavior. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 48fb4bade84c. |
|
Codex review: needs maintainer review before merge. What this changes: The PR expands Discord inbound fallback text extraction to include embed fields, footer text, and all embeds for normal messages, forwarded snapshots, and thread starters, with focused monitor tests. Maintainer follow-up before merge: This is a narrow, useful open implementation PR, but the remaining action is maintainer review, branch refresh, and normal validation rather than an autonomous replacement or repair job. Review detailsBest possible solution: Discord fallback text should preserve existing precedence while exposing title, description, nonblank fields, footer text, and all embeds for empty-content normal messages, forwarded snapshots, and thread starters, with refreshed tests on the current Discord monitor implementation. Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against e46dccb35374. |
1 participant
Summary
resolveDiscordEmbedTextto extract title, description, fields, and footer (previously only title and description)embeds[0], at all three call sitesDiscordEmbedLiketype for cross-file reuseMotivation
Many Discord bot messages use embeds for structured content — status dashboards, alerts, monitoring cards, cross-agent communication. When
message.contentis empty and data lives in embed fields/footer, agents currently see nothing. This is especially impactful in multi-agent setups where bots communicate primarily through rich embeds.Changes
extensions/discord/src/monitor/message-utils.ts:DiscordEmbedLiketype withtitle,description,fields, andfooterresolveDiscordEmbedText()rewritten to extract all embed parts, joining fields asName: ValueresolveDiscordMessageText()andresolveDiscordSnapshotMessageText()iterate all embeds with---separatorextensions/discord/src/monitor/threading.ts:DiscordEmbedLikeand iterate all embedsextensions/discord/src/monitor/message-utils.test.ts:Design decisions
message.contentis empty — no change for messages that already have text contentName: Value: simple, readable, no parsing overhead for agents---separator between embeds: distinguishes multi-embed boundariesauthor.namenot included: intentionally left for a follow-up — keeps this PR minimalKnown limitation
When a message has both text content AND embeds, only the content is shown (embed text is fallback-only). This is existing behavior and intentional — changing it would require a larger design discussion about how to present both without context bloat.
Test plan
tsgo --noEmitclean🤖 Generated with Claude Code