feat: add puck ai plugin #1022
feat: add puck ai plugin #1022
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces AI-powered page generation capabilities to the visual editor alongside significant infrastructure refactoring. It adds the Suggested labels
Suggested reviewers
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![semgrep-code-yext[bot]](https://avatars.githubusercontent.com/u/931250?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/vite-plugin/templates/route.ts`:
- Around line 5-18: The POST route handler currently calls
JSON.parse(request.body) and directly accesses body.config.components which can
throw; wrap the JSON.parse in a try-catch and return a 400 PagesHttpResponse for
invalid JSON, then defensively normalize body.config and body.config.components
(e.g., default body.config = body.config || {} and components =
body.config.components || {}) before building requestBodyWithFilteredConfig;
lastly filter using enabledAiComponents as before so Object.entries(...) never
receives undefined and parsing errors are handled.
- Around line 28-34: Validate PUCK_API_KEY before calling puckHandler: check
PUCK_API_KEY (used when building the options passed to puckHandler) and if it's
missing/undefined immediately return a 401 response with a clear error message
rather than invoking puckHandler with an invalid key; update the route handler
around the puckRequest/puckHandler call (and apply the same validation pattern
in starter/src/functions/http/api/puck/[route].ts) so callers receive a fast,
explicit auth error when the API key is not set.
- Around line 20-26: The puckRequest currently uses the client-supplied Origin
and request.url (which PagesHttpRequest doesn't provide); change puckRequest
construction to build an absolute URL from the Host header array: read host =
request.headers.host?.[0] (add a sensible fallback), derive a base URL (e.g.,
use https:// + host or infer scheme), and call new
Request(`${base}/api/puck/${request.pathParams.route}`, { method: "POST",
headers: { "content-type": "application/json" }, body:
JSON.stringify(requestBodyWithFilteredConfig) }); ensure you reference
puckRequest, request.headers.host, and request.pathParams.route when making the
change and handle missing host defensively.
In `@packages/visual-editor/THIRD-PARTY-NOTICES`:
- Around line 1164-1168: The THIRD-PARTY-NOTICES file is missing license entries
for `@puckeditor/plugin-ai` and `@puckeditor/cloud-client`; run the repository
notice-generation script (pnpm run generate-notices) to regenerate the
THIRD-PARTY-NOTICES file so those two packages (`@puckeditor/plugin-ai`@^0.5.0 and
`@puckeditor/cloud-client`@^0.5.0) are included, then verify the new file contains
their license texts and commit the updated THIRD-PARTY-NOTICES.
In `@starter/src/functions/http/api/puck/`[route].ts:
- Around line 20-34: The code constructs puckRequest using the client-controlled
request.headers.origin which allows SSRF; replace that with a trusted
server-side base URL (e.g., an env var like PUCK_BASE_URL or BASE_URL) when
building the Request for puckHandler, so change the puckRequest creation to use
the trusted base URL combined with request.pathParams.route; ensure puckHandler
is still called with the same options (ai context and PUCK_API_KEY) and validate
the env var is present before use.
- Around line 5-18: The POST handler currently calls JSON.parse(request.body)
and accesses body.config.components directly which throws on malformed or
missing input; wrap parsing in a try/catch inside the POST function to return a
400 PagesHttpResponse on parse errors, then validate that the parsed body has
the expected shape (e.g., required fields for the chat call and that body.config
and body.config.components are objects) before using them; if validation fails
return a 400 with a clear error message, and only then build
requestBodyWithFilteredConfig (using enabledAiComponents when safe) and call
chat. Ensure references: POST, request.body, chat, body.config.components, and
enabledAiComponents.
🧹 Nitpick comments (4)
packages/visual-editor/src/internal/types/templateMetadata.ts (1)
67-67: Environment variable "false" string will be treated as truthy.The double negation
!!import.meta.env.VITE_ENABLE_AI_PAGE_GENERATIONwill treat the string"false"as truthy. If a user setsVITE_ENABLE_AI_PAGE_GENERATION=falsein their.envfile, the feature will unexpectedly be enabled.Consider explicit string comparison:
🔧 Proposed fix
- aiPageGeneration: !!import.meta.env.VITE_ENABLE_AI_PAGE_GENERATION, + aiPageGeneration: import.meta.env.VITE_ENABLE_AI_PAGE_GENERATION === "true",starter/tsconfig.json (1)
1-14: Consider enabling stricter TypeScript settings.The configuration is functional but quite permissive. For better type safety, consider enabling additional options:
💡 Optional: Enable stricter settings
{ "compilerOptions": { "target": "ESNext", "module": "ESNext", "jsx": "react-jsx", "moduleResolution": "node", "types": ["vite/client", "node"], "allowJs": true, "checkJs": false, "resolveJsonModule": true, - "skipLibCheck": true + "skipLibCheck": true, + "strict": true, + "esModuleInterop": true, + "isolatedModules": true, + "noEmit": true }, "include": ["src"] }packages/visual-editor/package.json (1)
166-168: Consider making AI-related peer dependencies optional.Making
@puckeditor/plugin-aiand@puckeditor/cloud-clientrequired peer dependencies means all consumers of@yext/visual-editormust install these packages, even if they don't use the AI features (which requireVITE_ENABLE_AI_PAGE_GENERATION=true).Consider using
peerDependenciesMetato mark them as optional:💡 Mark AI dependencies as optional
Add after
peerDependencies:"peerDependenciesMeta": { "@puckeditor/cloud-client": { "optional": true }, "@puckeditor/plugin-ai": { "optional": true } }packages/visual-editor/src/internal/components/InternalLayoutEditor.tsx (1)
114-136: Gate the dynamic import byaiPageGenerationand include it in the dependency array.The module loads unconditionally even when AI is disabled, and if
templateMetadata.aiPageGenerationchanges, the effect won't re-run. Early guard prevents the unnecessary import, and addingtemplateMetadata.aiPageGenerationto the dependency array aligns with the pattern already used elsewhere in this file (seehandleHistoryChangewhich includestemplateMetadatain its dependency array).♻️ Suggested change
-React.useEffect(() => { - let cancelled = false; - - import("@puckeditor/plugin-ai") - .then((mod) => { - if (cancelled || !templateMetadata.aiPageGeneration) { - return; - } - setAiPlugin(mod.createAiPlugin({})); - }) - .catch(() => {}); - - return () => { - cancelled = true; - }; -}, []); +React.useEffect(() => { + if (!templateMetadata.aiPageGeneration) { + return; + } + let cancelled = false; + + import("@puckeditor/plugin-ai") + .then((mod) => { + if (cancelled) { + return; + } + setAiPlugin(mod.createAiPlugin({})); + }) + .catch(() => {}); + + return () => { + cancelled = true; + }; +}, [templateMetadata.aiPageGeneration]);
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/ai-backend/src/server.ts`:
- Around line 72-85: The code constructs a puckRequest and calls puckHandler
without verifying process.env.PUCK_API_KEY; add a fast-fail guard that checks
for the presence of the PUCK_API_KEY before building puckRequest and calling
puckHandler (use the same surrounding function that calls
readBody/createRequestOrigin), and if missing, immediately return or throw an
HTTP 500/400 response with a clear message and log the missing key so callers
get an explicit error instead of later opaque failures when puckHandler is
invoked without an API key.
In `@packages/visual-editor/src/internal/components/InternalLayoutEditor.tsx`:
- Around line 120-144: The effect that dynamically imports
"@puckeditor/plugin-ai" reads templateMetadata.aiPageGeneration but only depends
on localDev, causing stale plugin state; update the React.useEffect dependency
array to include templateMetadata.aiPageGeneration (or the specific flag) and
when the flag is false ensure you clear the plugin state by calling
setAiPlugin(null) (or equivalent) and abort any pending import (respecting the
cancelled flag) so the AI plugin loads and unloads correctly; locate the effect
around React.useEffect(...) and the setAiPlugin usage to implement these
changes.
packages/visual-editor/src/internal/components/InternalLayoutEditor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/ai-backend/src/server.ts`:
- Around line 100-113: The Request is being constructed with a plain JS object
as body which causes a TypeError in Node/undici; build a filteredBody object
(preserving origin, route, body, enabledAiComponents logic and the components
filter used in puckRequest) then pass JSON.stringify(filteredBody) as the
Request body and add a headers object with "Content-Type": "application/json"
when creating the Request (refer to puckRequest, Request, origin, route, body,
enabledAiComponents and the components filtering logic).
In `@packages/visual-editor/package.json`:
- Around line 24-28: The "./tailwind" package subpath in package.json currently
points its "types" entry at "./tailwind.d.ts" but the build emits declarations
into dist; update the "types" value for the "./tailwind" export to
"./dist/tailwind.d.ts" so TypeScript resolution matches the published files
(modify the "types" field under the "./tailwind" export in package.json).
In `@packages/visual-editor/src/docs/ai/components.d.ts`:
- Around line 49-59: Update the import list so HoursType, ImageType, and
ComplexImageType are imported from `@yext/types` instead of
`@yext/pages-components`: keep LinkType, AddressType, DayOfWeekNames,
HoursTableProps (alias HoursTableProps$1), Coordinate, and ListingType in the
`@yext/pages-components` import, and add a separate import for HoursType,
ImageType, ComplexImageType from `@yext/types`; apply the same change anywhere
else that currently imports those three types from `@yext/pages-components` (look
for usages of HoursType, ImageType, ComplexImageType in this file and other
files).
In `@starter/tsconfig.json`:
- Line 7: The tsconfig's "types" array currently includes "node", which brings
NodeJS global typings into the browser-facing compilation; remove "node" from
the "types" array so it only lists browser/Dev tooling types (e.g., keep
"vite/client" or any DOM-specific entries) to avoid NodeJS types like
NodeJS.Timeout from leaking into client-side code; update the "types" property
in tsconfig.json accordingly.
🧹 Nitpick comments (2)
starter/tsconfig.json (1)
4-6: Verifynodenextis intended for a Vite browser app.
nodenextmodule resolution enforces Node ESM semantics (including file extensions) that can conflict with Vite’s bundler behavior. If this is a client-only tsconfig, considermodule: "ESNext"andmoduleResolution: "bundler"(TS 5.0+).🔧 Alternative config
- "module": "nodenext", + "module": "ESNext", "jsx": "react-jsx", - "moduleResolution": "nodenext", + "moduleResolution": "bundler",packages/visual-editor/src/internal/components/InternalLayoutEditor.tsx (1)
118-126: Memoize AI plugin creation to keep plugin identity stable.
createAiPluginruns every render; if the plugin manages subscriptions/state, re-instantiation can reset or duplicate work. Memoize based onaiPageGenerationandlocalDev.♻️ Suggested change
- const aiPlugin = templateMetadata.aiPageGeneration - ? createAiPlugin( - localDev - ? { - host: "http://127.0.0.1:8787/api/puck/chat", - } - : undefined - ) - : undefined; + const aiPlugin = React.useMemo(() => { + if (!templateMetadata.aiPageGeneration) { + return undefined; + } + return createAiPlugin( + localDev + ? { + host: "http://127.0.0.1:8787/api/puck/chat", + } + : undefined + ); + }, [templateMetadata.aiPageGeneration, localDev]);Also applies to: 413-417
benlife5
requested review from
asanehisa,
briantstephan,
jwartofsky-yext and
mkilpatrick
|
@mkilpatrick I've reworked this to be a Cloudflare worker that can be run locally or deployed. I've used Hono for the server since that seemed to be the recommendation for a worker-friendly lightweight backend Going to test deployed worker now |
Legal RiskThe following dependencies were released under a license that RecommendationReach out to your security team or Semgrep admin to address this issue. In special cases, exceptions may be made for dependencies with violating licenses, however, the general recommendation is to avoid using a dependency under such a license LGPL-3.0-or-later
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/puck-ai-backend/.gitignore`:
- Around line 4-5: Update the .gitignore patterns to correctly match log files:
replace the incorrect pattern "_.log" with "*.log" to ignore all .log files, and
change "npm-debug.log_" to "npm-debug.log*" so npm debug logs with any suffix
are ignored; ensure these exact string patterns are replaced in the .gitignore
file.
In `@packages/puck-ai-backend/package.json`:
- Around line 13-18: Remove the unused "react" entry from the devDependencies
object in package.json (delete the "react": "^18.2.0" line) and then update your
lockfile by running your package manager install (npm/yarn/pnpm install) so the
dependency is pruned; verify there are no imports of React in the backend
codebase and commit the updated package.json and lockfile changes.
In `@packages/puck-ai-backend/src/puck.ts`:
- Around line 89-91: The catch block in puck.ts currently returns c.json({
success: false, error: "Failed to fetch members" }, 500) which is a misleading
copy/paste message; update the error response and log to reflect the actual
failure context (Puck AI request) by returning something like "Failed to fetch
Puck AI response" (or similar) and keep the existing console.error(error) so the
thrown error is preserved; modify the return in the catch around the Puck
request handler (the catch that uses error and c.json) to use the corrected
message.
- Around line 39-44: The code calls new URL(pathname, origin) using origin =
c.req.header("Origin") which can be undefined and throw; update the logic in
puck.ts (symbols: origin, pathname, new URL, c.req.header, c.req.param) to guard
against a missing/invalid Origin: obtain a safe base (for example derive from
request host/protocol or use a default base like "http://localhost"), validate
or normalize the base before calling new URL, and fall back to the safe base
when c.req.header("Origin") is falsy or invalid so new URL never receives
undefined.
In `@packages/visual-editor/src/editor/YextEntityFieldSelector.tsx`:
- Around line 192-221: The AI schema in YextEntityFieldSelector.tsx declares
hasLocalizedValue as type "boolean" but stored values are strings ("true"),
causing validation mismatches; update the ai schema inside the ai: { schema: {
properties: { constantValue: { properties: { hasLocalizedValue: ... } } } } }
block so hasLocalizedValue is type "string" (and keep enum: ["true"]) to match
existing stored data, or alternatively make producers/consumers write a boolean
consistently—pick one approach and apply it across the constantValue handling to
keep schema and stored values aligned.
🧹 Nitpick comments (6)
packages/puck-ai-backend/src/prompts.ts (1)
5-9: Minor: Whitespace in system context string.The template literal has line breaks that result in awkward spacing in the middle of sentences (e.g., "creating and\n managing"). Consider using a single line or trimming whitespace for cleaner output.
✨ Suggested fix
export const puckAiSystemContext = ` -You are an AI assistant for Yext Pages, a platform for creating and - managing web pages for brick-and-mortar business locations. - You will be assisting users who are actively editing pages in the Yext platform. +You are an AI assistant for Yext Pages, a platform for creating and managing web pages for brick-and-mortar business locations. You will be assisting users who are actively editing pages in the Yext platform. `;packages/puck-ai-backend/src/puck.ts (3)
1-1: Remove unused importchat.The
chatfunction is imported but never used in this file.🧹 Suggested fix
-import { puckHandler, chat } from "@puckeditor/cloud-client"; +import { puckHandler } from "@puckeditor/cloud-client";
22-26: Unsafe type cast bypasses validation.Casting
parseBody()result withas any as PuckChatRequestBodyfor form data provides no runtime guarantee that the parsed body matches the expected shape. If the form data is malformed, accessingbody.config.componentslater could throw.Consider validating the shape before use or documenting that form data input is not fully supported.
80-82: Note:Connection: keep-alivemay be ignored.Cloudflare Workers operate in a serverless context where the
Connectionheader is typically managed by the edge runtime and may be stripped or ignored. This shouldn't cause issues, but the header may have no effect in production.packages/puck-ai-backend/src/cors.ts (1)
34-37: Note: ReflectingAccess-Control-Request-Headersis permissive.Echoing back whatever headers the client requests is a common pattern for development flexibility, but in production you may want to explicitly allowlist specific headers (e.g.,
Content-Type,Authorization) to follow the principle of least privilege.packages/visual-editor/src/docs/ai/components.d.ts (1)
17-48: Confirm the hashed tailwind chunk import is stable for consumers.Importing from
./tailwind-D5fYUK0k.jscouples this declaration file to a build hash. If the chunk name changes or isn’t packaged, downstream TypeScript consumers can hit module-resolution errors. Consider re-pointing these type imports to a stable entry (e.g., the tailwind submodule) or ensure this chunk is always emitted and exported.
mkilpatrick
left a comment
mkilpatrick
left a comment
There was a problem hiding this comment.
- If you can't remove wrangler we can see if Vaccine will allow it since it's well-known.
- Robot nits
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/puck-ai-backend/src/puck.ts`:
- Around line 96-98: The catch block in the request handler currently returns
the raw Error object (return c.json({ success: false, error }, 500)), which
serializes to an empty object; change it to return a stable error message string
like the other branches (e.g., return c.json({ success: false, error: "Unable to
process request" }, 500) or include error.message), so update the catch in
puck.ts to send a consistent error string (and keep the existing
console.error(error) for server logs) instead of the raw Error object.
In `@packages/visual-editor/src/editor/YextEntityFieldSelector.tsx`:
- Around line 192-221: The AI schema currently hardcodes the locale key "en"
under the constantValue properties, which conflicts with runtime writes to
[locale] (i18n.language) in YextEntityFieldSelector; update the temporary ai
schema (the ai object in the props.filter branch) to allow arbitrary locale keys
by adding additionalProperties: { type: "string" } on the
constantValue.properties (or replace the explicit "en" property with a
patternProperties/additionalProperties entry) so AI-generated content can be
stored under dynamic locale keys; ensure this change is made where the ai schema
is defined in YextEntityFieldSelector.tsx so runtime writes to [locale] match
the schema.
🧹 Nitpick comments (1)
packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)
194-195: Condition only checks first element of types array.
props.filter.types?.[0] === "type.string"will not match if"type.string"exists in the array but is not the first element. Given the "TEMP" comment, this may be intentional, but if broader coverage is needed:- props.filter.types?.[0] === "type.string" && + props.filter.types?.includes("type.string") &&
packages/puck-ai-backend/src/puck.ts
Outdated
| } catch (error) { | ||
| console.error(error); | ||
| return c.json({ success: false, error }, 500); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JSON.stringify Error object JavaScript serialization message stack
💡 Result:
JSON.stringify(new Error("x")) often becomes "{}" because JSON.stringify() only serializes enumerable own properties, while Error’s important fields like message and stack are typically non-enumerable. (developer.mozilla.org)
Common ways to serialize an Error
1) Pick the fields you want (simple + explicit)
function serializeError(err) {
return {
name: err.name,
message: err.message,
stack: err.stack,
cause: err.cause, // if present
};
}
JSON.stringify(serializeError(error));stack exists in major engines but is non-standard and its format varies. (developer.mozilla.org)
2) Include all own properties (including non-enumerable)
JSON.stringify(error, Object.getOwnPropertyNames(error));This captures things like message and stack even though they’re non-enumerable. (zirkelc.dev)
3) Add toJSON() on a custom error class (clean for app errors)
class AppError extends Error {
toJSON() {
return { name: this.name, message: this.message, stack: this.stack, cause: this.cause };
}
}
JSON.stringify(new AppError("boom"));JSON.stringify() uses an object’s toJSON() if present. (developer.mozilla.org)
Note on circular references
If you attach objects to errors (e.g., err.meta = req) you can hit “circular structure” failures; use a replacer to handle cycles. (developer.mozilla.org)
Citations:
- 1: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack?utm_source=openai
- 3: https://zirkelc.dev/posts/stringify-and-parse-errors-in-javascript?utm_source=openai
- 4: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify?utm_source=openai
- 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value?utm_source=openai
🏁 Script executed:
cat -n packages/puck-ai-backend/src/puck.ts | sed -n '90,105p'Repository: yext/visual-editor
Length of output: 389
🏁 Script executed:
cat -n packages/puck-ai-backend/src/puck.ts | sed -n '1,50p'Repository: yext/visual-editor
Length of output: 1851
🏁 Script executed:
cat -n packages/puck-ai-backend/src/puck.ts | sed -n '70,100p'Repository: yext/visual-editor
Length of output: 1085
Replace raw Error object with stable message string in error response.
Returning the raw error object serializes to {} since Error properties like message and stack are non-enumerable. This leaves clients with no useful error information. Use a consistent message string instead, matching the pattern used elsewhere in this handler (lines 40, 45).
Suggested fix
} catch (error) {
console.error(error);
- return c.json({ success: false, error }, 500);
+ return c.json(
+ { success: false, error: "Failed to process Puck AI request" },
+ 500,
+ );
}📝 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.
| } catch (error) { | |
| console.error(error); | |
| return c.json({ success: false, error }, 500); | |
| } catch (error) { | |
| console.error(error); | |
| return c.json( | |
| { success: false, error: "Failed to process Puck AI request" }, | |
| 500, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@packages/puck-ai-backend/src/puck.ts` around lines 96 - 98, The catch block
in the request handler currently returns the raw Error object (return c.json({
success: false, error }, 500)), which serializes to an empty object; change it
to return a stable error message string like the other branches (e.g., return
c.json({ success: false, error: "Unable to process request" }, 500) or include
error.message), so update the catch in puck.ts to send a consistent error string
(and keep the existing console.error(error) for server logs) instead of the raw
Error object.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/puck-ai-backend/src/puckAiConfig.ts`:
- Around line 5-9: The puckAiSystemContext template literal includes unintended
leading indentation and a trailing space, so remove the embedded leading spaces
and the trailing space inside the string (or replace the template literal with a
left-aligned multiline string or call .trim() after the template) to ensure the
value of puckAiSystemContext contains no extra whitespace; update the
puckAiSystemContext declaration accordingly.
In `@packages/puck-ai-backend/src/puckRouter.ts`:
- Around line 59-73: The code builds puckRequest using body.config.components
which will throw if parseBody returned a body without config; update the request
construction in puckRouter (where puckRequest is created) to safely access
components (e.g., use optional chaining like body.config?.components or default
to an empty object) or, better, validate the parsed body early (after parseBody)
and return a 400 when body.config is missing/invalid before constructing
puckRequest; reference the symbols parseBody, puckRequest,
body.config.components and enabledAiComponents when making the change.
- Around line 18-31: The form/multipart branch currently casts the flat result
of c.req.parseBody() to PuckChatRequestBody which is unsafe and causes
downstream TypeErrors when accessing body.config.components; fix by either (A)
removing this form-parsing branch entirely (delete the if block that checks
contentType and uses c.req.parseBody()) if multipart/form-data isn’t a supported
intake for PuckChatRequestBody, or (B) implement proper reshaping of the flat
Record<string, string|File> returned by c.req.parseBody() into the nested
PuckChatRequestBody shape (reconstruct nested keys like config.components,
config.*, etc.) before returning, ensuring type checks and validation so
accesses such as body.config.components are safe. Ensure changes target the code
around c.req.parseBody() and references to PuckChatRequestBody and
body.config.components.
🧹 Nitpick comments (1)
packages/puck-ai-backend/src/puckRouter.ts (1)
87-89: Nit:Connection: keep-aliveis a hop-by-hop header and has no effect in Cloudflare Workers.Cloudflare (and HTTP/2 in general) strips hop-by-hop headers like
Connection. Setting it here is harmless but misleading.Cache-Control: no-cachealone is sufficient for SSE.Proposed fix
const headers = new Headers(puckResponse.headers); headers.set("Cache-Control", "no-cache"); - headers.set("Connection", "keep-alive");
|
With puck ai 0.6.0, we're able to move system prompt + component filtering into the visual-editor library -> will align with artifacts, smaller request payload. Also potentially allows auth headers if we end up wanting that |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/utils/ai/prepareRequest.ts`:
- Around line 11-30: preparePuckAiRequest currently shallow-copies opts so
mutations to updatedOpts.body and updatedOpts.body.config mutate the caller;
instead create new nested objects before writing: clone opts into updatedOpts,
then if opts.body exists set updatedOpts.body = { ...opts.body } (and when
touching config set updatedOpts.body.config = { ...opts.body.config } ), and
when filtering components set updatedOpts.body.config.components =
Object.fromEntries(...) operating on a spread copy of
opts.body.config.components; ensure you still assign
updatedOpts.body.systemPrompt = puckAiSystemContext and use enabledAiComponents
for the filter so the original opts.body and its config/components are never
mutated.
🧹 Nitpick comments (2)
packages/puck-ai-backend/src/puckRouter.ts (2)
56-65:systemPromptis forwarded both in the serialised body and in the options — confirm this is intentional.Line 58 serialises the entire
body(includingsystemPrompt) into theRequestpayload, while line 63 also passesbody.systemPromptvia theai.contextoption. IfpuckHandlerreads both, the prompt could be applied twice or conflict. Consider strippingsystemPromptfrom the body before forwarding:Proposed fix
+ const { systemPrompt, ...puckBody } = body; + const puckRequest = new Request(url, { method: "POST", - body: JSON.stringify(body), + body: JSON.stringify(puckBody), headers: { "content-type": "application/json" }, }); const puckResponse = await puckHandler(puckRequest, { - ai: { context: body.systemPrompt }, + ai: { context: systemPrompt }, apiKey: puckApiKey, });
74-76:Connection: keep-aliveis a hop-by-hop header and has no effect in Cloudflare Workers.Cloudflare Workers operate over HTTP/2 and do not support raw TCP keep-alive semantics. This header will be silently stripped by the runtime. It's harmless but misleading — consider removing it to avoid confusion.
| export const preparePuckAiRequest: PrepareRequestFn = (opts) => { | ||
| let updatedOpts = { ...opts }; | ||
|
|
||
| if (!updatedOpts.body) { | ||
| updatedOpts.body = {}; | ||
| } | ||
|
|
||
| updatedOpts.body.systemPrompt = puckAiSystemContext; | ||
|
|
||
| if (!updatedOpts.body?.config?.components) { | ||
| return updatedOpts; | ||
| } | ||
|
|
||
| updatedOpts.body.config.components = Object.fromEntries( | ||
| Object.entries(updatedOpts.body.config.components).filter(([component]) => | ||
| (enabledAiComponents as string[]).includes(component) | ||
| ) | ||
| ); | ||
|
|
||
| return updatedOpts; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for calls to preparePuckAiRequest to understand usage context
rg -n 'preparePuckAiRequest' --type=ts --type=tsx -B 5 -A 5Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Get the PrepareRequestFn type definition and surrounding context
rg -n 'PrepareRequestFn' --type=ts --type=tsx -B 3 -A 10Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Search for how the result of preparePuckAiRequest is used
rg -n 'preparePuckAiRequest' --type=ts --type=tsx | head -20
# Then search for the file containing the calls
fd 'prepareRequest' --type=fRepository: yext/visual-editor
Length of output: 160
🏁 Script executed:
#!/bin/bash
# Search for calls to preparePuckAiRequest without the tsx type
rg -n 'preparePuckAiRequest' -B 5 -A 5Repository: yext/visual-editor
Length of output: 3986
🏁 Script executed:
#!/bin/bash
# Look at the prepareRequest.ts file to understand context
cat -n packages/visual-editor/src/utils/ai/prepareRequest.tsRepository: yext/visual-editor
Length of output: 1154
🏁 Script executed:
#!/bin/bash
# Find where preparePuckAiRequest is imported or used
rg 'preparePuckAiRequest' --type=tsRepository: yext/visual-editor
Length of output: 582
🏁 Script executed:
#!/bin/bash
# Search for tests related to prepareRequest
find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "preparePuckAiRequest" 2>/dev/null || echo "No tests found for preparePuckAiRequest"Repository: yext/visual-editor
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about whether mutation is intentional
rg -n "preparePuckAiRequest|prepareRequest" packages/visual-editor/src/utils/ai/ -B 2 -A 10Repository: yext/visual-editor
Length of output: 1396
🏁 Script executed:
#!/bin/bash
# Look at the enabledAiComponents and puckAiSystemContext imports to understand the full context
cat -n packages/visual-editor/src/utils/ai/enabledComponents.ts 2>/dev/null || echo "File not found"
cat -n packages/visual-editor/src/utils/ai/systemPrompt.ts 2>/dev/null || echo "File not found"Repository: yext/visual-editor
Length of output: 3080
Shallow spread mutates the caller's body object.
{ ...opts } copies only the top-level reference; opts.body is still shared. Lines 18 and 24 then mutate the original body (and config) in place, contradicting the intent signaled by the spread operator.
Proposed fix: spread nested objects to avoid mutation
export const preparePuckAiRequest: PrepareRequestFn = (opts) => {
- let updatedOpts = { ...opts };
-
- if (!updatedOpts.body) {
- updatedOpts.body = {};
- }
-
- updatedOpts.body.systemPrompt = puckAiSystemContext;
-
- if (!updatedOpts.body?.config?.components) {
- return updatedOpts;
+ const body = opts.body ? { ...opts.body } : {};
+ body.systemPrompt = puckAiSystemContext;
+
+ if (!body.config?.components) {
+ return { ...opts, body };
}
- updatedOpts.body.config.components = Object.fromEntries(
- Object.entries(updatedOpts.body.config.components).filter(([component]) =>
+ const filteredComponents = Object.fromEntries(
+ Object.entries(body.config.components).filter(([component]) =>
(enabledAiComponents as string[]).includes(component)
)
);
- return updatedOpts;
+ return {
+ ...opts,
+ body: {
+ ...body,
+ config: { ...body.config, components: filteredComponents },
+ },
+ };
};🤖 Prompt for AI Agents
In `@packages/visual-editor/src/utils/ai/prepareRequest.ts` around lines 11 - 30,
preparePuckAiRequest currently shallow-copies opts so mutations to
updatedOpts.body and updatedOpts.body.config mutate the caller; instead create
new nested objects before writing: clone opts into updatedOpts, then if
opts.body exists set updatedOpts.body = { ...opts.body } (and when touching
config set updatedOpts.body.config = { ...opts.body.config } ), and when
filtering components set updatedOpts.body.config.components =
Object.fromEntries(...) operating on a spread copy of
opts.body.config.components; ensure you still assign
updatedOpts.body.systemPrompt = puckAiSystemContext and use enabledAiComponents
for the filter so the original opts.body and its config/components are never
mutated.
2 participants
This PR adds the Puck AI plugin and successfully forwards requests to the Puck cloud server. It is not connected to any components so it cannot build any layouts, but the chat works.
Adds a Cloudflare worker using a Hono backend to proxy the Puck AI requests. The worker can be run locally or deployed.
To run:
pnpm run devwithPUCK_API_KEYin.dev.varspnpm run dev:aito runCodex made some package changes to separate the tailwind imports from the rest of the package so that the postcss build issues pass. They seem reasonable and the starter works but we should verify in-platform