Merged
Conversation
|
| Name | Type |
|---|---|
| @voltagent/ag-ui | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
This comment has been minimized.
This comment has been minimized.
Contributor
📝 WalkthroughWalkthroughMaps VoltAgent "message-metadata" stream parts to a Changes
Sequence Diagram(s)sequenceDiagram
participant Volt as VoltAgent Stream
participant Conv as AG-UI Converter
participant AGUI as AG-UI (Event Bus / DOM)
participant App as App Consumer
Volt->>Conv: emit stream parts (start / text / message-metadata / ...)
alt part.type == "message-metadata"
Conv->>AGUI: dispatch CustomEvent "voltagent.message_metadata" { messageId, metadata }
AGUI->>App: listener receives metadata event -> map messageId to feedback URL
App->>App: post feedback (thumbs up/down) to VoltOps
else other part types
Conv->>AGUI: emit converted events (start / text / tool / ...)
AGUI->>App: listener handles normal events
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Deploying voltagent with
|
| Latest commit: |
a5e7c7b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5799935c.voltagent.pages.dev |
| Branch Preview URL: | https://fix-ag-ui-feedback-metadata.voltagent.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ag-ui/src/voltagent-agent.ts (1)
415-426: Nit: themessageId || undefinedcoercion on line 433 is effectively a no-op.
messageIdfalls back tofallbackMessageId(line 419), which is always a non-empty UUID fromgenerateId(). The|| undefinedguard on line 433 can never trigger. Not harmful, but it's dead code that might confuse future readers into thinking an empty/missingmessageIdis a realistic scenario here.Proposed simplification
value: { - messageId: messageId || undefined, + messageId, metadata: messageMetadata, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ag-ui/src/voltagent-agent.ts` around lines 415 - 426, The messageId null-coercion is dead code: in the message-metadata branch computeMessageId (the variable named messageId which already falls back to fallbackMessageId generated by generateId()) can never be empty, so remove the "|| undefined" coercion and use messageId directly where it is passed/returned; update any references in the message-metadata handling block (partType === "message-metadata") to rely on messageId without the redundant "|| undefined".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ag-ui/src/voltagent-agent.ts`:
- Around line 415-426: The messageId null-coercion is dead code: in the
message-metadata branch computeMessageId (the variable named messageId which
already falls back to fallbackMessageId generated by generateId()) can never be
empty, so remove the "|| undefined" coercion and use messageId directly where it
is passed/returned; update any references in the message-metadata handling block
(partType === "message-metadata") to rely on messageId without the redundant "||
undefined".
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ag-ui/src/voltagent-agent.ts (2)
402-403: Minor inconsistency:partTypeextracted but unused in the switch.
partTypeis extracted on line 403 for the"message-metadata"check, but theswitchon line 431 usespart.typedirectly. This works but is slightly confusing — either usepartTypeconsistently or inline the check on line 405.Use partType in the switch for consistency
- switch (part.type) { + switch (partType) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ag-ui/src/voltagent-agent.ts` around lines 402 - 403, The code extracts partType but then uses part.type in the switch, which is inconsistent; update the switch in the function containing payload and partType so it uses the extracted partType variable (or remove the partType extraction if you prefer inline access) ensuring the case that checks "message-metadata" uses partType consistently with the earlier extraction; reference the variables payload and partType and the switch that handles "message-metadata" to locate and apply the change.
405-429: Metadata validation looks good; consider exporting the constant for downstream consumers.The guard on line 414 (
!messageMetadata || typeof messageMetadata !== "object") correctly rejects nullish and primitive values before emitting the event. The dual fallback formessageMetadatavsmetadata(lines 411-412) is a reasonable compat measure.One thing to note:
VOLTAGENT_MESSAGE_METADATA_EVENT_NAMEis module-scoped (constat line 278) but not exported. If downstream AG-UI consumers need to match on this event name (e.g.event.name === "voltagent.message_metadata"), they'll end up duplicating the string. Consider exporting it.Export the constant
-const VOLTAGENT_MESSAGE_METADATA_EVENT_NAME = "voltagent.message_metadata"; +export const VOLTAGENT_MESSAGE_METADATA_EVENT_NAME = "voltagent.message_metadata";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ag-ui/src/voltagent-agent.ts` around lines 405 - 429, VOLTAGENT_MESSAGE_METADATA_EVENT_NAME is currently a module-scoped const but should be exported so downstream consumers can reference the canonical event name; change its declaration from "const VOLTAGENT_MESSAGE_METADATA_EVENT_NAME" to "export const VOLTAGENT_MESSAGE_METADATA_EVENT_NAME" where it is defined (line ~278) and ensure it is re-exported from the package's public API/barrel if one exists so consumers can import it instead of duplicating the string; leave all internal uses (e.g., in the message-metadata branch that constructs CustomEvent) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ag-ui/src/voltagent-agent.ts`:
- Around line 402-403: The code extracts partType but then uses part.type in the
switch, which is inconsistent; update the switch in the function containing
payload and partType so it uses the extracted partType variable (or remove the
partType extraction if you prefer inline access) ensuring the case that checks
"message-metadata" uses partType consistently with the earlier extraction;
reference the variables payload and partType and the switch that handles
"message-metadata" to locate and apply the change.
- Around line 405-429: VOLTAGENT_MESSAGE_METADATA_EVENT_NAME is currently a
module-scoped const but should be exported so downstream consumers can reference
the canonical event name; change its declaration from "const
VOLTAGENT_MESSAGE_METADATA_EVENT_NAME" to "export const
VOLTAGENT_MESSAGE_METADATA_EVENT_NAME" where it is defined (line ~278) and
ensure it is re-exported from the package's public API/barrel if one exists so
consumers can import it instead of duplicating the string; leave all internal
uses (e.g., in the message-metadata branch that constructs CustomEvent)
unchanged.
…tadata-stream # Conflicts: # .changeset/wise-pans-arrive.md # packages/ag-ui/src/voltagent-agent.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/docs/ui/copilotkit.md (1)
247-283: Consider safer type narrowing for event payload.The type assertion at lines 260-267 uses
asto castevent.valuewithout runtime validation. If the event payload doesn't match the expected shape, this could lead to runtime errors or unexpected behavior. Consider adding runtime validation or using a type guard.🛡️ Example with runtime validation
const payload = event.value as | { messageId?: string; metadata?: { feedback?: VoltFeedbackMetadata; }; } | undefined; const messageId = payload?.messageId; const feedback = payload?.metadata?.feedback; - if (!messageId || !feedback?.url) return; + if ( + !messageId || + typeof messageId !== 'string' || + !feedback?.url || + typeof feedback.url !== 'string' + ) return; + setFeedbackByMessageId((prev) => ({ ...prev, [messageId]: feedback }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/ui/copilotkit.md` around lines 247 - 283, The handler in useVoltFeedbackMap currently casts event.value with "as" which is unsafe; replace the assertion with runtime validation or a type guard: inside agent.subscribe onCustomEvent check that event.value is a non-null object, that messageId is a string, and that metadata is an object containing a feedback object with a url string before using them; update the logic around VOLTAGENT_MESSAGE_METADATA_EVENT_NAME to derive messageId and feedback only after these checks (or call a small type-guard function like isVoltPayload(value) that returns a typed predicate) and then call setFeedbackByMessageId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/docs/ui/copilotkit.md`:
- Around line 289-320: submitVoltFeedback currently posts to feedback.url
without validation; validate the URL before sending to prevent SSRF: parse
feedback.url with the URL constructor inside submitVoltFeedback, ensure protocol
is https: (or a safe protocol) and that the origin (hostname+port) matches a
configured allowlist or the app's same-origin, and return early if parsing fails
or the origin is not allowed; add or call a small helper like
isAllowedFeedbackOrigin(url) and wrap the fetch call in that guard so fetch is
only executed for validated, trusted origins.
- Around line 289-320: submitVoltFeedback currently performs fetch and
response.json() without try/catch which can cause unhandled rejections on
network or parse failures; wrap the network call and JSON parsing in a try/catch
around the fetch(...) and await response.json() so any errors are caught, return
early on errors (do not mark feedback as provided), and record or log the error
(e.g., include an error field in setFeedbackByMessageId update or call a logger)
so callers can surface failure; keep existing checks for response.ok and only
set provided: true and feedbackId when the request and parsing succeed.
---
Nitpick comments:
In `@website/docs/ui/copilotkit.md`:
- Around line 247-283: The handler in useVoltFeedbackMap currently casts
event.value with "as" which is unsafe; replace the assertion with runtime
validation or a type guard: inside agent.subscribe onCustomEvent check that
event.value is a non-null object, that messageId is a string, and that metadata
is an object containing a feedback object with a url string before using them;
update the logic around VOLTAGENT_MESSAGE_METADATA_EVENT_NAME to derive
messageId and feedback only after these checks (or call a small type-guard
function like isVoltPayload(value) that returns a typed predicate) and then call
setFeedbackByMessageId.
Comment on lines
+289
to
+320
| async function submitVoltFeedback(input: { | ||
| message: Message; | ||
| type: "thumbsUp" | "thumbsDown"; | ||
| feedbackByMessageId: Record<string, VoltFeedbackMetadata>; | ||
| setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>; | ||
| }) { | ||
| const feedback = input.feedbackByMessageId[input.message.id]; | ||
| if (!feedback?.url || isProvided(feedback)) return; | ||
|
|
||
| const score = input.type === "thumbsUp" ? 1 : 0; | ||
|
|
||
| const response = await fetch(feedback.url, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: safeStringify({ | ||
| score, | ||
| feedback_source_type: "app", | ||
| }), | ||
| }); | ||
|
|
||
| if (!response.ok) return; | ||
|
|
||
| const data = (await response.json()) as { id?: string }; | ||
| setFeedbackByMessageId((prev) => ({ | ||
| ...prev, | ||
| [input.message.id]: { | ||
| ...feedback, | ||
| provided: true, | ||
| feedbackId: data.id, | ||
| }, | ||
| })); | ||
| } |
Contributor
There was a problem hiding this comment.
Add URL validation to prevent SSRF attacks.
The function makes a POST request to feedback.url without validating that the URL belongs to a trusted origin. If the metadata stream is compromised or misconfigured, this could allow an attacker to make arbitrary POST requests from the client to any URL (SSRF).
🛡️ Proposed fix with origin validation
+const TRUSTED_FEEDBACK_ORIGINS = [
+ 'https://api.voltagent.dev',
+ // Add other trusted origins
+];
+
+function isUrlTrusted(url: string): boolean {
+ try {
+ const parsed = new URL(url);
+ return TRUSTED_FEEDBACK_ORIGINS.some(origin => parsed.origin === origin);
+ } catch {
+ return false;
+ }
+}
+
async function submitVoltFeedback(input: {
message: Message;
type: "thumbsUp" | "thumbsDown";
feedbackByMessageId: Record<string, VoltFeedbackMetadata>;
setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>;
}) {
const feedback = input.feedbackByMessageId[input.message.id];
if (!feedback?.url || isProvided(feedback)) return;
+
+ if (!isUrlTrusted(feedback.url)) {
+ console.error('Untrusted feedback URL:', feedback.url);
+ return;
+ }
const score = input.type === "thumbsUp" ? 1 : 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function submitVoltFeedback(input: { | |
| message: Message; | |
| type: "thumbsUp" | "thumbsDown"; | |
| feedbackByMessageId: Record<string, VoltFeedbackMetadata>; | |
| setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>; | |
| }) { | |
| const feedback = input.feedbackByMessageId[input.message.id]; | |
| if (!feedback?.url || isProvided(feedback)) return; | |
| const score = input.type === "thumbsUp" ? 1 : 0; | |
| const response = await fetch(feedback.url, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: safeStringify({ | |
| score, | |
| feedback_source_type: "app", | |
| }), | |
| }); | |
| if (!response.ok) return; | |
| const data = (await response.json()) as { id?: string }; | |
| setFeedbackByMessageId((prev) => ({ | |
| ...prev, | |
| [input.message.id]: { | |
| ...feedback, | |
| provided: true, | |
| feedbackId: data.id, | |
| }, | |
| })); | |
| } | |
| const TRUSTED_FEEDBACK_ORIGINS = [ | |
| 'https://api.voltagent.dev', | |
| // Add other trusted origins | |
| ]; | |
| function isUrlTrusted(url: string): boolean { | |
| try { | |
| const parsed = new URL(url); | |
| return TRUSTED_FEEDBACK_ORIGINS.some(origin => parsed.origin === origin); | |
| } catch { | |
| return false; | |
| } | |
| } | |
| async function submitVoltFeedback(input: { | |
| message: Message; | |
| type: "thumbsUp" | "thumbsDown"; | |
| feedbackByMessageId: Record<string, VoltFeedbackMetadata>; | |
| setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>; | |
| }) { | |
| const feedback = input.feedbackByMessageId[input.message.id]; | |
| if (!feedback?.url || isProvided(feedback)) return; | |
| if (!isUrlTrusted(feedback.url)) { | |
| console.error('Untrusted feedback URL:', feedback.url); | |
| return; | |
| } | |
| const score = input.type === "thumbsUp" ? 1 : 0; | |
| const response = await fetch(feedback.url, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: safeStringify({ | |
| score, | |
| feedback_source_type: "app", | |
| }), | |
| }); | |
| if (!response.ok) return; | |
| const data = (await response.json()) as { id?: string }; | |
| setFeedbackByMessageId((prev) => ({ | |
| ...prev, | |
| [input.message.id]: { | |
| ...feedback, | |
| provided: true, | |
| feedbackId: data.id, | |
| }, | |
| })); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/docs/ui/copilotkit.md` around lines 289 - 320, submitVoltFeedback
currently posts to feedback.url without validation; validate the URL before
sending to prevent SSRF: parse feedback.url with the URL constructor inside
submitVoltFeedback, ensure protocol is https: (or a safe protocol) and that the
origin (hostname+port) matches a configured allowlist or the app's same-origin,
and return early if parsing fails or the origin is not allowed; add or call a
small helper like isAllowedFeedbackOrigin(url) and wrap the fetch call in that
guard so fetch is only executed for validated, trusted origins.
Add error handling for network and parsing failures.
The function lacks try-catch blocks around the fetch call and response.json() parsing. Network failures, timeouts, or malformed JSON responses would result in unhandled promise rejections. This could lead to silent failures where users believe their feedback was submitted but it actually failed.
🛡️ Proposed fix with error handling
async function submitVoltFeedback(input: {
message: Message;
type: "thumbsUp" | "thumbsDown";
feedbackByMessageId: Record<string, VoltFeedbackMetadata>;
setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>;
}) {
const feedback = input.feedbackByMessageId[input.message.id];
if (!feedback?.url || isProvided(feedback)) return;
const score = input.type === "thumbsUp" ? 1 : 0;
+ try {
- const response = await fetch(feedback.url, {
+ const response = await fetch(feedback.url, {
- method: "POST",
+ method: "POST",
- headers: { "Content-Type": "application/json" },
+ headers: { "Content-Type": "application/json" },
- body: safeStringify({
+ body: safeStringify({
- score,
+ score,
- feedback_source_type: "app",
+ feedback_source_type: "app",
- }),
+ }),
- });
+ signal: AbortSignal.timeout(5000), // 5s timeout
+ });
- if (!response.ok) return;
+ if (!response.ok) {
+ console.error('Feedback submission failed:', response.status);
+ return;
+ }
- const data = (await response.json()) as { id?: string };
- setFeedbackByMessageId((prev) => ({
+ const data = (await response.json()) as { id?: string };
+ input.setFeedbackByMessageId((prev) => ({
- ...prev,
+ ...prev,
- [input.message.id]: {
+ [input.message.id]: {
- ...feedback,
+ ...feedback,
- provided: true,
+ provided: true,
- feedbackId: data.id,
+ feedbackId: data.id,
- },
+ },
- }));
+ }));
+ } catch (error) {
+ console.error('Failed to submit feedback:', error);
+ // Optionally: show user notification
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function submitVoltFeedback(input: { | |
| message: Message; | |
| type: "thumbsUp" | "thumbsDown"; | |
| feedbackByMessageId: Record<string, VoltFeedbackMetadata>; | |
| setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>; | |
| }) { | |
| const feedback = input.feedbackByMessageId[input.message.id]; | |
| if (!feedback?.url || isProvided(feedback)) return; | |
| const score = input.type === "thumbsUp" ? 1 : 0; | |
| const response = await fetch(feedback.url, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: safeStringify({ | |
| score, | |
| feedback_source_type: "app", | |
| }), | |
| }); | |
| if (!response.ok) return; | |
| const data = (await response.json()) as { id?: string }; | |
| setFeedbackByMessageId((prev) => ({ | |
| ...prev, | |
| [input.message.id]: { | |
| ...feedback, | |
| provided: true, | |
| feedbackId: data.id, | |
| }, | |
| })); | |
| } | |
| async function submitVoltFeedback(input: { | |
| message: Message; | |
| type: "thumbsUp" | "thumbsDown"; | |
| feedbackByMessageId: Record<string, VoltFeedbackMetadata>; | |
| setFeedbackByMessageId: Dispatch<SetStateAction<Record<string, VoltFeedbackMetadata>>>; | |
| }) { | |
| const feedback = input.feedbackByMessageId[input.message.id]; | |
| if (!feedback?.url || isProvided(feedback)) return; | |
| const score = input.type === "thumbsUp" ? 1 : 0; | |
| try { | |
| const response = await fetch(feedback.url, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: safeStringify({ | |
| score, | |
| feedback_source_type: "app", | |
| }), | |
| signal: AbortSignal.timeout(5000), // 5s timeout | |
| }); | |
| if (!response.ok) { | |
| console.error('Feedback submission failed:', response.status); | |
| return; | |
| } | |
| const data = (await response.json()) as { id?: string }; | |
| input.setFeedbackByMessageId((prev) => ({ | |
| ...prev, | |
| [input.message.id]: { | |
| ...feedback, | |
| provided: true, | |
| feedbackId: data.id, | |
| }, | |
| })); | |
| } catch (error) { | |
| console.error('Failed to submit feedback:', error); | |
| // Optionally: show user notification | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/docs/ui/copilotkit.md` around lines 289 - 320, submitVoltFeedback
currently performs fetch and response.json() without try/catch which can cause
unhandled rejections on network or parse failures; wrap the network call and
JSON parsing in a try/catch around the fetch(...) and await response.json() so
any errors are caught, return early on errors (do not mark feedback as
provided), and record or log the error (e.g., include an error field in
setFeedbackByMessageId update or call a logger) so callers can surface failure;
keep existing checks for response.ok and only set provided: true and feedbackId
when the request and parsing succeed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes (issue)
Notes for reviewers
Summary by cubic
Fixes lost assistant feedback metadata in AG-UI streams by mapping VoltAgent message-metadata to a stable CUSTOM event. Fully removes legacy markers and filtering for reliable delivery, and adds docs for wiring CopilotKit thumbs to VoltOps feedback via these events.
Written for commit a5e7c7b. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Documentation