fix(cli): handle non-JSON upstream in fetchRemotePrompt#1282
fix(cli): handle non-JSON upstream in fetchRemotePrompt#1282calebcgates wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 0298754 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR adds JSON parsing error handling to the ChangesJSON parsing error guard for remote prompts
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/prompts.spec.ts`:
- Line 77: Replace the direct use of JSON.stringify in the test response
construction with the safeStringify utility: import safeStringify from
'@voltagent/internal' (or add it to existing imports in prompts.spec.ts) and
call safeStringify(payload) where Response(JSON.stringify(payload), ...) is
used; ensure the import name matches how safeStringify is exported and update
the Response instantiation to use the safeStringify output so the test adheres
to the TS guideline banning JSON.stringify.
- Around line 15-27: The test uses JSON.stringify where our coding guideline
mandates safeStringify from `@voltagent/internal`: replace the
JSON.stringify(payload) call used in the test helper with
safeStringify(payload). Locate the reproduceParseContract helper and any other
places in this file where JSON.stringify is used for test payload serialization
(specifically the call that builds the mock response body) and import/use
safeStringify instead to ensure safe serialization consistent with project
utilities.
In `@packages/cli/src/commands/prompts.ts`:
- Line 409: The unchecked cast "parsed as RemotePrompt" must be replaced with
runtime validation: implement a type guard function (e.g., isRemotePrompt(obj):
obj is RemotePrompt) that checks required fields/types of the RemotePrompt
interface and use it before returning; in the function that currently does
"return parsed as RemotePrompt" call isRemotePrompt(parsed) and throw/handle an
error if it returns false so only validated RemotePrompt objects are returned.
Ensure the type guard inspects all unique properties used elsewhere (names/types
referenced by RemotePrompt) and update callers to rely on the guard rather than
the direct cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bb69e76-5184-4216-a64e-d760f6d83173
📒 Files selected for processing (3)
.changeset/cli-prompts-fetch-non-json-response.mdpackages/cli/src/commands/prompts.spec.tspackages/cli/src/commands/prompts.ts
| const reproduceParseContract = async (response: Response, name: string): Promise<unknown> => { | ||
| let parsed: unknown; | ||
| try { | ||
| parsed = await response.json(); | ||
| } catch (error) { | ||
| const reason = error instanceof Error ? error.message : String(error); | ||
| throw new Error( | ||
| `Failed to parse prompt '${name}' response: ${reason}. ` + | ||
| `The upstream returned a non-JSON body (status ${response.status} ${response.statusText}).`, | ||
| ); | ||
| } | ||
| return parsed; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's confirm the file exists and get a sense of its structure
fd -type f "prompts.spec.ts"Repository: VoltAgent/voltagent
Length of output: 234
🏁 Script executed:
# Also check for the production file that might contain fetchRemotePrompt
fd -type f "prompts.ts" | grep -v specRepository: VoltAgent/voltagent
Length of output: 234
🏁 Script executed:
# Check the overall structure of the packages/cli directory
ls -la packages/cli/src/commands/ 2>/dev/null || echo "Directory not found"Repository: VoltAgent/voltagent
Length of output: 894
🏁 Script executed:
# Read the prompts.spec.ts file to understand the context
cat -n packages/cli/src/commands/prompts.spec.tsRepository: VoltAgent/voltagent
Length of output: 3791
🏁 Script executed:
# Read the prompts.ts file to find fetchRemotePrompt and understand the production code
cat -n packages/cli/src/commands/prompts.ts | head -100Repository: VoltAgent/voltagent
Length of output: 3774
🏁 Script executed:
# Search for fetchRemotePrompt definition in prompts.ts
rg "fetchRemotePrompt" packages/cli/src/commands/prompts.ts -A 10Repository: VoltAgent/voltagent
Length of output: 938
🏁 Script executed:
# Check for safeStringify imports in the repository
rg "safeStringify" packages/cli/src/ --type tsRepository: VoltAgent/voltagent
Length of output: 1536
🏁 Script executed:
# Get more context on the fetchRemotePrompt function to understand the full parse logic
rg "const fetchRemotePrompt" packages/cli/src/commands/prompts.ts -A 30Repository: VoltAgent/voltagent
Length of output: 905
🏁 Script executed:
# Get more of fetchRemotePrompt including the JSON parse step
rg "const fetchRemotePrompt" packages/cli/src/commands/prompts.ts -A 50 | head -80Repository: VoltAgent/voltagent
Length of output: 1469
🏁 Script executed:
# Check if there are any other JSON.stringify calls in the spec file
rg "JSON\.stringify" packages/cli/src/commands/prompts.spec.ts -nRepository: VoltAgent/voltagent
Length of output: 127
Replace JSON.stringify with safeStringify at line 77.
The test helper uses JSON.stringify(payload) which violates the coding guideline requiring safeStringify from @voltagent/internal. Update line 77 to use safeStringify(payload) instead.
Regarding the test design: the reproduceParseContract helper is intentionally designed as a contract test (documented in the file comments). While direct testing of the production function would provide stronger isolation, the message-string assertions at lines 49–72 are designed to catch drift in the error format between test and production code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/prompts.spec.ts` around lines 15 - 27, The test
uses JSON.stringify where our coding guideline mandates safeStringify from
`@voltagent/internal`: replace the JSON.stringify(payload) call used in the test
helper with safeStringify(payload). Locate the reproduceParseContract helper and
any other places in this file where JSON.stringify is used for test payload
serialization (specifically the call that builds the mock response body) and
import/use safeStringify instead to ensure safe serialization consistent with
project utilities.
|
|
||
| it("returns parsed JSON when upstream returns valid JSON", async () => { | ||
| const payload = { id: "p_1", type: "text", content: "hi" }; | ||
| const response = new Response(JSON.stringify(payload), { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read around line 77
if [ -f "packages/cli/src/commands/prompts.spec.ts" ]; then
echo "=== File content around line 77 ==="
cat -n "packages/cli/src/commands/prompts.spec.ts" | sed -n '70,85p'
else
echo "File not found. Let's search for the file:"
find . -name "prompts.spec.ts" -type f
fiRepository: VoltAgent/voltagent
Length of output: 850
🏁 Script executed:
# Search for JSON.stringify usage in the target file
echo "=== All JSON.stringify usages in the file ==="
rg "JSON\.stringify" "packages/cli/src/commands/prompts.spec.ts" -nRepository: VoltAgent/voltagent
Length of output: 173
🏁 Script executed:
# Check current imports in the file
echo "=== Import statements in the file ==="
head -30 "packages/cli/src/commands/prompts.spec.ts" | cat -nRepository: VoltAgent/voltagent
Length of output: 1637
🏁 Script executed:
# Verify the correct import path for safeStringify
echo "=== Search for safeStringify in codebase ==="
rg "safeStringify" --type ts -l | head -20Repository: VoltAgent/voltagent
Length of output: 1024
🏁 Script executed:
# Check import path for safeStringify in an actual usage
echo "=== Checking safeStringify import in one of the files ==="
head -20 "packages/cli/src/services/eval/dataset-push.ts" | rg "import.*safeStringify|from.*safeStringify" -A 1 -B 1Repository: VoltAgent/voltagent
Length of output: 228
🏁 Script executed:
# Get more context around the import
rg "import.*safeStringify" "packages/cli/src/services/eval/dataset-push.ts" -A 1 -B 1Repository: VoltAgent/voltagent
Length of output: 170
🏁 Script executed:
# Search across the codebase for safeStringify import pattern
echo "=== All safeStringify import patterns ==="
rg "import.*safeStringify.*from" --type ts | head -10Repository: VoltAgent/voltagent
Length of output: 1211
Replace JSON.stringify with safeStringify.
Line 77 violates the TS guideline banning JSON.stringify.
Suggested fix
+import { safeStringify } from "@voltagent/internal";
...
- const response = new Response(JSON.stringify(payload), {
+ const response = new Response(safeStringify(payload), {As per coding guidelines, "Never use JSON.stringify; use the safeStringify function instead, imported from @voltagent/internal".
📝 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.
| const response = new Response(JSON.stringify(payload), { | |
| import { safeStringify } from "@voltagent/internal"; | |
| ... | |
| const response = new Response(safeStringify(payload), { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/prompts.spec.ts` at line 77, Replace the direct use
of JSON.stringify in the test response construction with the safeStringify
utility: import safeStringify from '@voltagent/internal' (or add it to existing
imports in prompts.spec.ts) and call safeStringify(payload) where
Response(JSON.stringify(payload), ...) is used; ensure the import name matches
how safeStringify is exported and update the Response instantiation to use the
safeStringify output so the test adheres to the TS guideline banning
JSON.stringify.
| ); | ||
| } | ||
|
|
||
| return parsed as RemotePrompt; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cat -n packages/cli/src/commands/prompts.ts | sed -n '400,420p'Repository: VoltAgent/voltagent
Length of output: 860
🏁 Script executed:
# Check for RemotePrompt type definition
rg "type RemotePrompt|interface RemotePrompt" packages/cli/src/commands/prompts.ts -A 5Repository: VoltAgent/voltagent
Length of output: 202
🏁 Script executed:
# Get broader context around line 409 to understand the function
cat -n packages/cli/src/commands/prompts.ts | sed -n '380,425p'Repository: VoltAgent/voltagent
Length of output: 1569
🏁 Script executed:
rg "JSON\.stringify" packages/cli/src/commands/prompts.tsRepository: VoltAgent/voltagent
Length of output: 45
🏁 Script executed:
# Also check if safeStringify is used or imported
rg "safeStringify" packages/cli/src/commands/prompts.tsRepository: VoltAgent/voltagent
Length of output: 762
Add runtime type validation before returning upstream payload.
The unchecked cast parsed as RemotePrompt on line 409 bypasses type safety for untrusted network data. Although the JSON parsing is wrapped in a try/catch, the response structure is never validated. Add a type guard to ensure the payload matches the RemotePrompt schema before casting.
Suggested fix
+const isRemotePrompt = (value: unknown): value is RemotePrompt => {
+ if (!value || typeof value !== "object") return false;
+ const v = value as Record<string, unknown>;
+ return (
+ typeof v.name === "string" &&
+ (v.type === "text" || v.type === "chat") &&
+ "prompt" in v
+ );
+};
...
- return parsed as RemotePrompt;
+ if (!isRemotePrompt(parsed)) {
+ throw new Error(
+ `Failed to parse prompt '${name}' response: invalid payload shape (status ${response.status} ${response.statusText}).`,
+ );
+ }
+ return parsed;📝 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.
| return parsed as RemotePrompt; | |
| const isRemotePrompt = (value: unknown): value is RemotePrompt => { | |
| if (!value || typeof value !== "object") return false; | |
| const v = value as Record<string, unknown>; | |
| return ( | |
| typeof v.name === "string" && | |
| (v.type === "text" || v.type === "chat") && | |
| "prompt" in v | |
| ); | |
| }; | |
| if (!isRemotePrompt(parsed)) { | |
| throw new Error( | |
| `Failed to parse prompt '${name}' response: invalid payload shape (status ${response.status} ${response.statusText}).`, | |
| ); | |
| } | |
| return parsed; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/prompts.ts` at line 409, The unchecked cast "parsed
as RemotePrompt" must be replaced with runtime validation: implement a type
guard function (e.g., isRemotePrompt(obj): obj is RemotePrompt) that checks
required fields/types of the RemotePrompt interface and use it before returning;
in the function that currently does "return parsed as RemotePrompt" call
isRemotePrompt(parsed) and throw/handle an error if it returns false so only
validated RemotePrompt objects are returned. Ensure the type guard inspects all
unique properties used elsewhere (names/types referenced by RemotePrompt) and
update callers to rely on the guard rather than the direct cast.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Quick response to the review from code rabbit:
|
2 participants
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
fetchRemotePromptinpackages/cli/src/commands/prompts.ts(line 360) carefully handles404(returnsnull) and non-2xx (throws a rich error:Failed to fetch prompt 'name': status statusText - body). But the finalreturn (await response.json()) as RemotePrompt;on line 398 is unguarded.When the VoltOps prompts API (or any proxy in front of it — corporate CDN, captive portal, mid-deploy HTML rollback) returns a
200 OKwith an HTML body,response.json()throws a rawSyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON. That propagates through the commander.action()handler which printserror.messageto stderr — so the user sees the raw SyntaxError text with no hint of which prompt was being fetched or that the upstream was misbehaving:This affects
volt prompts pullandvolt prompts push(both code paths callfetchRemotePrompt).What is the new behavior?
fetchRemotePromptnow wrapsawait response.json()in a try/catch and rethrows a rich error mirroring the existing non-2xx handler shape on lines 389-396:The user immediately understands the upstream returned something unexpected and can investigate proxy / CDN / VoltOps deployment status, instead of suspecting their own CLI installation.
Added two vitest specs covering the non-JSON failure path (including a "preserves the underlying parser reason" case that verifies prompt name + HTTP status correlation) and one happy-path regression spec.
Notes for reviewers
packages/cli/src/commands/prompts.ts). Spec is a new file (packages/cli/src/commands/prompts.spec.ts, 86 lines, 3 cases). Changeset is a new.changeset/cli-prompts-fetch-non-json-response.md(patch on@voltagent/cli).fetchRemotePromptis currently module-private; the spec mirrors the parse contract in a local helper rather than importing it. Happy to follow up withexport const fetchRemotePrompt = ...and a direct import if you'd prefer — that's a one-character source change and unblocks future tests on the same function.pnpm --filter @voltagent/cli test(3 passing),pnpm --filter @voltagent/cli lint(clean — biome),pnpm --filter @voltagent/cli build(clean).Summary by cubic
Handle non-JSON 200 OK responses in the CLI prompt fetch path by catching JSON parse errors and throwing a clear message with the prompt name and HTTP status. This prevents raw SyntaxError output in
volt prompts pull/volt prompts pushwhen the API or a proxy returns HTML.response.json()in a try/catch infetchRemotePromptand emit: "Failed to parse prompt '' response: . The upstream returned a non-JSON body (status)."@voltagent/cli.Written for commit 02987546c25b3988637ec96a129f99a4ef061078. Summary will update on new commits.
Summary by CodeRabbit
volt prompts pullandvolt prompts pushto provide clearer error messages when the upstream API returns non-JSON responses, including prompt name and HTTP status details.