Add Hotel Booking Agent #238
Conversation
|
|
📝 WalkthroughWalkthroughAdds a hotel-booking sample: a FastAPI chat agent backed by a LangGraph graph and tools, a mock Hotel API (search, bookings, ingestion), OpenAPI specs, environment examples, pinned requirements, sample datasets, and policy PDF metadata and ingestion tooling. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client
participant AgentAPI as Agent Service
participant Graph as Graph Engine
participant LLM as Language Model
participant HotelAPI as Hotel API
participant Pinecone as Policy Store
participant Weather as Weather API
User->>Client: user message
Client->>AgentAPI: POST /chat
AgentAPI->>AgentAPI: validate & wrap message
AgentAPI->>Graph: invoke(HumanMessage, thread_id)
Graph->>LLM: prompt + tool context
alt tools invoked
LLM-->>Graph: tool_calls
Graph->>HotelAPI: search/create/update/list
Graph->>Pinecone: policy query
Graph->>Weather: forecast request
HotelAPI-->>Graph: results
Pinecone-->>Graph: policy matches
Weather-->>Graph: forecast
Graph->>LLM: continue reasoning with tool results
else final response
LLM-->>Graph: final response
end
Graph-->>AgentAPI: final message
AgentAPI-->>Client: ChatResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@samples/travel` planner agent/backend/agent/requirements.txt:
- Around line 1-11: The requirements.txt pins vulnerable packages; update the
pinned versions in requirements.txt so fastapi, langchain, and langgraph (and
any transitive packages) are at safe releases: bump langchain to >=0.3.80
(addresses langchain-core CVE), ensure langchain-text-splitters is >=0.3.9 (if
directly listed or via transitive constraints), bump langgraph to >=3.0.0 (fixes
langgraph-checkpoint), and upgrade fastapi to a release that allows Starlette
>=0.47.2 (so Starlette gets >=0.47.2 to mitigate CVE-2024/2025 issues); also
update related pins (langchain-openai, etc.) to compatible latest stable
releases, then regenerate your lock/install (pip-compile or pip install -U) and
run the test suite to verify compatibility before merging.
In `@samples/travel` planner agent/backend/agent/tools.py:
- Around line 486-488: The weather API call (the requests.get /
response.raise_for_status block in the weather tool function in tools.py) lacks
error handling; wrap the requests.get(...) and response.raise_for_status() calls
in a try/except that catches requests.RequestException as e (same pattern used
by the booking tools), and handle it consistently (either return a clear error
message or raise the same ToolException/error type the booking tools use) so
network errors and timeouts don't propagate and crash the agent.
In `@samples/travel` planner agent/backend/booking_api/booking_api.py:
- Around line 240-244: The endpoint create_booking (and other handlers that call
_resolve_user_id and return _error_response) currently returns error payloads
with 200 OK; change these to return proper HTTP status codes by raising
fastapi.HTTPException(status_code=401, detail=_error_response("Authentication
required", "AUTH_REQUIRED")) or by returning
fastapi.responses.JSONResponse(status_code=401, content=_error_response(...));
update the error return sites where _resolve_user_id is used (e.g.,
create_booking and the other endpoints that return _error_response) to use
HTTPException or JSONResponse so clients receive correct HTTP status codes for
auth and other errors.
- Around line 103-110: The _decode_jwt_payload function currently decodes the
JWT payload without verifying the signature, allowing forged tokens; replace
this by using a proper JWT library (e.g., PyJWT) and call its decode method with
the correct secret or public key and allowed algorithms (e.g., HS256/RS256) to
perform signature verification, and ensure you obtain the secret/key from a
secure configuration (env var or key store); if you must keep a non-verifying
branch for local demo, make it explicit with a clear WARNING comment and a
dedicated flag (e.g., verify_signature=False) so production code always uses
verified decoding in _decode_jwt_payload.
- Around line 26-33: The CORS setup in app.add_middleware using CORSMiddleware
is overly permissive (allow_origins=["*"] with allow_credentials=True); restrict
it by replacing the wildcard origin with an explicit allowlist (e.g., load a
list of allowed domains from config/env) and keep allow_credentials=True only
when origins are specific, or set allow_credentials=False if you must allow all
origins; update the CORSMiddleware call in booking_api.py to use the explicit
allow_origins list (and optionally allow_methods/allow_headers as needed) to
harden production deployments.
- Around line 203-214: The current file-based persistence in _load_bookings and
_save_bookings races under concurrent requests; fix by introducing an OS-level
file lock around read-modify-write sequences (use a library like portalocker or
fcntl) to ensure exclusive access to DATA_PATH, acquire the lock before calling
_load_bookings, perform updates (e.g. in create_booking/update/delete handlers),
and hold the lock until an atomic write is completed (write to a temp file and
os.replace to DATA_PATH) inside _save_bookings; update callers to use this
lock-protected sequence so no two operations can read stale data and overwrite
each other.
In `@samples/travel` planner agent/backend/booking_api/requirements.txt:
- Line 1: Replace the pinned fastapi==0.115.0 entry in requirements.txt with a
newer FastAPI that depends on Starlette >=0.47.2 (so the Starlette CVEs are
patched); for example change the constraint to a modern minimum like
fastapi>=0.125.0 (instead of fastapi==0.115.0), then reinstall/lock dependencies
(pip install -U or regenerate your lockfile) and run your test suite to ensure
compatibility with the updated fastapi/starlette transitive dependency.
In `@samples/travel` planner agent/frontend/package.json:
- Around line 65-71: The `@types` packages in package.json are on the React 19
line while the project uses React 18; update the dependency entries for
"@types/react" and "@types/react-dom" to the corresponding React 18 typings (use
the 18.x `@types` line), then reinstall dependencies and rebuild (or update the
lockfile) so typechecking uses the correct 18.x typedefs; look for the
dependency keys "@types/react" and "@types/react-dom" in package.json to make
the change.
In `@samples/travel` planner agent/frontend/src/components/HotelCard.tsx:
- Around line 23-26: The TypeScript error occurs because the React type
namespace (used in React.SyntheticEvent within the handleImageError function
inside HotelCard) isn't imported; add a type-only import for React at the top of
the file (e.g., import type React from "react") so the React namespace is
available for types like React.SyntheticEvent without affecting runtime, keeping
HotelCardProps, FALLBACK_IMAGE and handleImageError unchanged.
In `@samples/travel` planner agent/frontend/src/components/SearchHero.tsx:
- Around line 29-49: The input and icon-only submit button lack accessible
names, so update the SearchHero input (the element using value={query} and
onChange={(e) => setQuery(e.target.value)}) to include an aria-label (e.g.,
aria-label="Search travel plans") or a visually hidden <label> tied via
id/aria-labelledby, and add an aria-label on the submit button (the <button>
that renders the Send component) such as aria-label="Submit search"; ensure the
labels remain conditional with compact styling if needed and choose clear,
concise text for screen readers.
In `@samples/travel` planner agent/frontend/src/components/ui/badge.tsx:
- Around line 6-26: The badgeVariants utility uses an undefined CSS utility
class hover-elevate and CSS variable --badge-outline; add a global CSS variable
--badge-outline (in :root) with the intended HSL/RGB value and define a utility
class .hover-elevate in your main stylesheet's utility layer (e.g., `@layer`
utilities in src/index.css) that applies the desired transform/transition on
hover; update the file so badgeVariants (from badge.tsx) can rely on the new
.hover-elevate class and --badge-outline variable being present.
In `@samples/travel` planner agent/frontend/src/components/ui/chart.tsx:
- Around line 70-99: The ChartStyle component currently injects CSS via
dangerouslySetInnerHTML using unescaped id and config keys (see ChartStyle, id,
config, THEMES, colorConfig, itemConfig, key), which risks CSS injection; remove
dangerouslySetInnerHTML, build the CSS string safely by escaping the selector id
and variable names with CSS.escape (guarded with typeof CSS !== 'undefined' or a
fallback escape utility), e.g. escape id when creating the [data-chart=...]
selector and escape key when creating --color-<key> variable names, then render
the stylesheet as a regular React child like <style>{css}</style> so the values
are escaped and not injected raw.
In `@samples/travel` planner agent/frontend/src/components/ui/resizable.tsx:
- Around line 8-11: Add an explicit React namespace import so type references
like React.ComponentProps resolve: at the top of the file import React (e.g.
import * as React from 'react' or import React from 'react'), then keep usages
around ResizablePanelGroup and the other ComponentProps reference (for
ResizablePrimitive.PanelGroup and the other prop typing at line ~27) unchanged;
this provides the React type namespace without affecting the JSX runtime.
In `@samples/travel` planner agent/frontend/src/pages/HotelDetails.tsx:
- Around line 119-138: The Mapbox token in the static image URL is a
non-functional placeholder; replace the hardcoded token with a public env var
(e.g. NEXT_PUBLIC_MAPBOX_TOKEN) and build the src using that variable in
HotelDetails.tsx (the <img> tag where src is constructed with
hotel.longitude/hotel.latitude), add a safe fallback when the token or image
fails (either render a neutral placeholder/map-silhouette or use an onError
handler on the <img> to swap to a fallback image/div), and ensure you do not
leak any secret server tokens to the client (use only a public Mapbox token
exposed via a NEXT_PUBLIC_*/VITE_* env var).
In `@samples/travel` planner agent/resources/ingest/ingest.py:
- Around line 99-101: Replace the hardcoded embedding model string in the call
to self.openai_client.embeddings.create with the configured embedding model from
your config (e.g., OPENAI_EMBEDDING_MODEL) so the code reads
model=OPENAI_EMBEDDING_MODEL; import or reference the same variable you use in
config.py and add a sensible fallback/default if missing, ensuring the change is
applied where embeddings are created (the
self.openai_client.embeddings.create(...) invocation) to prevent
embedding-dimension mismatches when the environment config differs.
🟡 Minor comments (20)
samples/travel planner agent/frontend/src/components/ui/progress.tsx-11-23 (1)
11-23: Respectmaxand clamp progress percentage.The indicator assumes a 0–100 scale and ignores the
maxprop. Whenmaxdiffers from 100 (which Radix Progress supports), progress renders incorrectly. Out-of-range values can also over/underfill the bar. Additionally,value === null(indeterminate state) is treated as0instead of being handled separately.Suggested fix
- >(({ className, value, ...props }, ref) => ( + >(({ className, value, max = 100, ...props }, ref) => { + const clamped = + value == null ? null : Math.min(Math.max(value, 0), max) + const percent = clamped == null ? null : (clamped / max) * 100 + return ( <ProgressPrimitive.Root ref={ref} className={cn( "relative h-4 w-full overflow-hidden rounded-full bg-secondary", className )} {...props} > <ProgressPrimitive.Indicator className="h-full w-full flex-1 bg-primary transition-all" - style={{ transform: `translateX(-${100 - (value || 0)}%)` }} + style={ + percent == null + ? undefined + : { transform: `translateX(-${100 - percent}%)` } + } /> </ProgressPrimitive.Root> + )})samples/travel planner agent/frontend/src/hooks/use-mobile.tsx-5-16 (1)
5-16: GuardmatchMediausage and add legacy listener fallback for older browser support.
window.matchMediacan be missing in some environments (server-side rendering, older test runners, or older browsers), which will throw at runtime. Additionally, whileMediaQueryList.addEventListeneris the modern standard (supported in Chrome 39+, Firefox 55+, Safari 14+), older browsers require a fallback toaddListener/removeListener. Consider guarding access to both APIs and usingmql.matchesfor consistency with the media query instead ofwindow.innerWidth.🛠️ Proposed fix
export function useIsMobile() { - const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) + const [isMobile, setIsMobile] = React.useState<boolean | undefined>(() => { + if (typeof window === "undefined") return undefined + return window.innerWidth < MOBILE_BREAKPOINT + }) React.useEffect(() => { - const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) - const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) - } - mql.addEventListener("change", onChange) - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) - return () => mql.removeEventListener("change", onChange) + if (typeof window === "undefined" || !window.matchMedia) return + const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) + const onChange = () => setIsMobile(mql.matches) + if (typeof mql.addEventListener === "function") { + mql.addEventListener("change", onChange) + } else { + mql.addListener(onChange) + } + setIsMobile(mql.matches) + return () => { + if (typeof mql.removeEventListener === "function") { + mql.removeEventListener("change", onChange) + } else { + mql.removeListener(onChange) + } + } }, []) return !!isMobile }samples/travel planner agent/frontend/src/components/ui/alert.tsx-35-44 (1)
35-44: Type mismatch: ref type doesn't match rendered element.
AlertTitledeclaresforwardRef<HTMLParagraphElement>but renders an<h5>element (which isHTMLHeadingElement). This type inconsistency could cause issues when consumers try to access heading-specific properties via the ref.Suggested fix
const AlertTitle = React.forwardRef< - HTMLParagraphElement, + HTMLHeadingElement, React.HTMLAttributes<HTMLHeadingElement> >(({ className, ...props }, ref) => (samples/travel planner agent/frontend/src/components/ui/alert.tsx-47-56 (1)
47-56: Type mismatch: ref type doesn't match rendered element.
AlertDescriptiondeclaresforwardRef<HTMLParagraphElement>but renders a<div>element. Update the ref type to match the actual element.Suggested fix
const AlertDescription = React.forwardRef< - HTMLParagraphElement, + HTMLDivElement, React.HTMLAttributes<HTMLParagraphElement> >(({ className, ...props }, ref) => (samples/travel planner agent/frontend/src/components/ui/carousel.tsx-107-119 (1)
107-119: Missing cleanup for"reInit"event listener.The effect registers listeners for both
"select"and"reInit"events (lines 113-114), but the cleanup function only removes"select". This could cause memory leaks or stale callbacks if the component unmounts orapichanges.Proposed fix
return () => { + api?.off("reInit", onSelect) api?.off("select", onSelect) }samples/travel planner agent/frontend/src/hooks/use-toast.ts-174-182 (1)
174-182: Incorrect dependency inuseEffectcauses unnecessary re-subscriptions.The
statevariable is included in the dependency array, but the effect only needs to subscribe once on mount. Includingstatecauses the listener to be removed and re-added on every state change.Proposed fix
React.useEffect(() => { listeners.push(setState) return () => { const index = listeners.indexOf(setState) if (index > -1) { listeners.splice(index, 1) } } - }, [state]) + }, [])samples/travel planner agent/frontend/src/lib/queryClient.ts-27-42 (1)
27-42: Type mismatch:getQueryFncan returnnullbutTdoesn't include it.When
on401is"returnNull", the function returnsnull(line 37), but the return typeQueryFunction<T>doesn't account for this. Consumers usingon401: "returnNull"will receiveT | nullat runtime but TypeScript believes it'sT.Proposed fix
-export const getQueryFn: <T>(options: { - on401: UnauthorizedBehavior; -}) => QueryFunction<T> = +export const getQueryFn: <T>(options: { + on401: "throw"; +}) => QueryFunction<T>; +export const getQueryFn: <T>(options: { + on401: "returnNull"; +}) => QueryFunction<T | null>; +export const getQueryFn = <T>({ on401: unauthorizedBehavior }: { + on401: UnauthorizedBehavior; +}): QueryFunction<T | null> =>Alternatively, use a simpler approach with conditional type or just type the return as
T | nullwhenon401is"returnNull".samples/travel planner agent/frontend/src/components/ui/breadcrumb.tsx-91-105 (1)
91-105: Fix displayName typo.
This keeps devtools lookup consistent.🐛 Proposed fix
-BreadcrumbEllipsis.displayName = "BreadcrumbElipssis" +BreadcrumbEllipsis.displayName = "BreadcrumbEllipsis"samples/travel planner agent/frontend/src/components/ui/form.tsx-18-73 (1)
18-73: Guard check inuseFormFieldis ineffective and executed too late.Both
FormFieldContextandFormItemContextare initialized with non-null defaults ({}), making guard checks always false. Additionally,getFieldStateis called on line 49 withfieldContext.namebefore the guard check on line 51, anditemContextis accessed on line 55 without any guard at all. This creates runtime errors when the hook is used outside its expected provider hierarchy.🐛 Proposed fix
-const FormFieldContext = React.createContext<FormFieldContextValue>( - {} as FormFieldContextValue -) +const FormFieldContext = React.createContext<FormFieldContextValue | null>(null)-const FormItemContext = React.createContext<FormItemContextValue>( - {} as FormItemContextValue -) +const FormItemContext = React.createContext<FormItemContextValue | null>(null)const useFormField = () => { const fieldContext = React.useContext(FormFieldContext) const itemContext = React.useContext(FormItemContext) const { getFieldState, formState } = useFormContext() + if (!fieldContext) { + throw new Error("useFormField should be used within <FormField>") + } + if (!itemContext) { + throw new Error("useFormField should be used within <FormItem>") + } + const fieldState = getFieldState(fieldContext.name, formState) - - if (!fieldContext) { - throw new Error("useFormField should be used within <FormField>") - } - const { id } = itemContextsamples/travel planner agent/frontend/src/components/ui/sidebar.tsx-603-639 (1)
603-639: Potential hydration mismatch withMath.random()in render.Using
Math.random()during render (even insideuseMemowith empty dependencies) can cause hydration mismatches in SSR/SSG environments because the server and client will generate different random values. The "use client" directive doesn't prevent the initial server render.🔧 Suggested fix using useId or stable width
function SidebarMenuSkeleton({ className, showIcon = false, ...props }: React.ComponentProps<"div"> & { showIcon?: boolean }) { - // Random width between 50 to 90%. - const width = React.useMemo(() => { - return `${Math.floor(Math.random() * 40) + 50}%` - }, []) + // Use a stable default width to avoid hydration mismatch. + // Randomization can be applied client-side after mount if desired. + const [width, setWidth] = React.useState("70%") + + React.useEffect(() => { + setWidth(`${Math.floor(Math.random() * 40) + 50}%`) + }, []) return (samples/travel planner agent/backend/booking_api/booking_api.py-20-20 (1)
20-20: In-memory user store is not persisted.The
userslist is stored in memory and will be lost on server restart. This creates inconsistency since bookings are persisted to disk but user records are not.samples/travel planner agent/frontend/src/components/ui/menubar.tsx-237-237 (1)
237-237: Typo:displaynameshould bedisplayName.The property name uses lowercase 'n' while all other components use
displayName(capital N). This inconsistency means React DevTools won't recognize the component name.Suggested fix
-MenubarShortcut.displayname = "MenubarShortcut" +MenubarShortcut.displayName = "MenubarShortcut"samples/travel planner agent/frontend/src/pages/Landing.tsx-31-38 (1)
31-38:group-hoverwon’t trigger without agroupclass on the button.
Right now the arrow animation won’t fire.🛠️ Proposed fix
- <Button + <Button size="lg" onClick={() => setLocation("/assistant")} - className="mt-10 bg-[`#4da3ff`] hover:bg-[`#2f86f0`] text-white font-semibold text-base md:text-lg px-8 py-4 rounded-full h-auto shadow-lg relative" + className="group mt-10 bg-[`#4da3ff`] hover:bg-[`#2f86f0`] text-white font-semibold text-base md:text-lg px-8 py-4 rounded-full h-auto shadow-lg relative" >samples/travel planner agent/frontend/src/config/auth.ts-5-6 (1)
5-6: Guardwindowaccess at module scope.
This will throw in non-browser contexts (SSR/tests). A small guard avoids hard crashes.🛠️ Proposed fix
-const authConfig = { +const defaultRedirect = + typeof window === "undefined" ? "/" : window.location.origin; + +const authConfig = { clientId: process.env.REACT_APP_ASGARDEO_CLIENT_ID || "HYzBc0mxP4KI6_HjoMr8KfZEVCEa", baseUrl: process.env.REACT_APP_ASGARDEO_BASE_URL || "https://api.asgardeo.io/t/smthing", scopes: (process.env.REACT_APP_ASGARDEO_SCOPES || "openid profile").split(" "), - afterSignInUrl: process.env.REACT_APP_REDIRECT_SIGN_IN || window.location.origin, - afterSignOutUrl: process.env.REACT_APP_REDIRECT_SIGN_OUT || window.location.origin, + afterSignInUrl: process.env.REACT_APP_REDIRECT_SIGN_IN || defaultRedirect, + afterSignOutUrl: process.env.REACT_APP_REDIRECT_SIGN_OUT || defaultRedirect, storage: "localStorage" as const, };samples/travel planner agent/frontend/src/pages/Landing.tsx-15-20 (1)
15-20: Ensure the gradient overlay is scoped to the section and doesn’t intercept clicks.
Without a positioned parent, the absolute overlay can escape the section, and it can still intercept pointer events in some stacking cases.🛠️ Proposed fix
- <section - className="h-full flex items-center justify-center bg-cover bg-center" - style={{ backgroundImage: "url('/images/orchid-hotels.webp')" }} - > - <div className="absolute inset-0 bg-gradient-to-r from-transparent via-[`#0f4c9f`]/25 to-[`#0f4c9f`]/55" /> - <div className="text-left px-4 max-w-3xl space-y-6"> + <section + className="relative h-full flex items-center justify-center bg-cover bg-center" + style={{ backgroundImage: "url('/images/orchid-hotels.webp')" }} + > + <div + className="absolute inset-0 bg-gradient-to-r from-transparent via-[`#0f4c9f`]/25 to-[`#0f4c9f`]/55 pointer-events-none" + aria-hidden="true" + /> + <div className="relative text-left px-4 max-w-3xl space-y-6">samples/travel planner agent/frontend/src/pages/HotelDetails.tsx-72-80 (1)
72-80: Potential runtime error iftagsis empty.Accessing
hotel.tags[0]will returnundefinedif the tags array is empty, which could cause the Badge to render incorrectly or show "undefined".Suggested fix
<div className="flex flex-wrap items-center gap-3 mb-3"> - <Badge className="bg-accent text-accent-foreground border-none px-3 py-1 text-sm font-semibold"> - {hotel.tags[0]} - </Badge> + {hotel.tags?.[0] && ( + <Badge className="bg-accent text-accent-foreground border-none px-3 py-1 text-sm font-semibold"> + {hotel.tags[0]} + </Badge> + )}samples/travel planner agent/frontend/src/pages/Home.tsx-93-93 (1)
93-93: Unused endpoint constant referencing non-existent API.
CHAT_SESSIONS_URLis constructed for a/sessionsendpoint that doesn't exist in the backend (app.pyonly defines/travelPlanner/chat). TheloadSessionseffect on lines 211-241 will always fail silently.Consider either:
- Removing the sessions fetch logic if server-side session persistence isn't planned
- Adding the sessions endpoint to the backend if this feature is intended
samples/travel planner agent/backend/agent/tools.py-531-536 (1)
531-536: Edge case bug in "this weekend" calculation.When today is Saturday or Sunday (
now.weekday() >= 5), the conditionnow.weekday() > 5only catches Sunday. On Saturday (weekday == 5),5 > 5is False, so it incorrectly calculates5 - 5 = 0days ahead, which is correct. However, on Sunday (weekday == 6), it calls_next_weekday(5, now)which returns next Saturday, skipping the current weekend entirely.Suggested fix
if "this weekend" in lowered: # Upcoming Saturday/Sunday based on current week. - saturday = _next_weekday(5, now) if now.weekday() > 5 else now + timedelta(days=(5 - now.weekday())) + if now.weekday() == 5: # Saturday + saturday = now + elif now.weekday() == 6: # Sunday + saturday = now - timedelta(days=1) + else: + saturday = now + timedelta(days=(5 - now.weekday())) sunday = saturday + timedelta(days=1)samples/travel planner agent/backend/agent/app.py-85-86 (1)
85-86: Potential IndexError if messages list is empty.If
result["messages"]is empty, accessing[-1]will raise anIndexError. Add a guard to handle this edge case.Suggested fix
- last_message = result["messages"][-1] - return ChatResponse(message=last_message.content) + messages = result.get("messages", []) + if not messages: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="No response generated.", + ) + return ChatResponse(message=messages[-1].content)samples/travel planner agent/frontend/src/components/ui/chart.tsx-188-245 (1)
188-245: Render zero values and use a stable key.Two issues:
item.value &&hides0values. The formatter path correctly usesitem?.value !== undefined, but the default render path treats zero as falsy. Use explicit null/undefined check instead.key={item.dataKey}can be undefined. The code computes a fallback key on line 189 but doesn't use it. Use the computed key with index for stability.Suggested fix
{payload.map((item, index) => { const key = `${nameKey || item.name || item.dataKey || "value"}` const itemConfig = getPayloadConfigFromPayload(config, item, key) const indicatorColor = color || item.payload.fill || item.color return ( <div - key={item.dataKey} + key={`${key}-${index}`} className={cn( "flex w-full flex-wrap items-stretch gap-2 [&>svg]:h-2.5 [&>svg]:w-2.5 [&>svg]:text-muted-foreground", indicator === "dot" && "items-center" )} > {formatter && item?.value !== undefined && item.name ? ( formatter(item.value, item.name, item, index, item.payload) ) : ( <> {itemConfig?.icon ? ( <itemConfig.icon /> ) : ( !hideIndicator && ( <div className={cn( "shrink-0 rounded-[2px] border-[--color-border] bg-[--color-bg]", { "h-2.5 w-2.5": indicator === "dot", "w-1": indicator === "line", "w-0 border-[1.5px] border-dashed bg-transparent": indicator === "dashed", "my-0.5": nestLabel && indicator === "dashed", } )} style={ { "--color-bg": indicatorColor, "--color-border": indicatorColor, } as React.CSSProperties } /> ) )} <div className={cn( "flex flex-1 justify-between leading-none", nestLabel ? "items-end" : "items-center" )} > <div className="grid gap-1.5"> {nestLabel ? tooltipLabel : null} <span className="text-muted-foreground"> {itemConfig?.label || item.name} </span> </div> - {item.value && ( + {item.value !== undefined && item.value !== null && ( <span className="font-mono font-medium tabular-nums text-foreground"> {item.value.toLocaleString()} </span> )}
🧹 Nitpick comments (38)
samples/travel planner agent/frontend/.gitignore (1)
1-211: Optional: remove duplicate ignore entries to reduce noise.There are repeated entries (e.g.,
node_modules/,coverage/,.eslintcache,.vscode/,.idea/,*.tsbuildinfo,.env.local,dist). Cleaning these up keeps the file easier to maintain.samples/travel planner agent/frontend/src/components/Footer.tsx (1)
10-12: Consider addingaria-hiddento the decorative icon.The Compass icon is decorative (the adjacent text already conveys the meaning). Adding
aria-hidden="true"prevents screen readers from announcing it unnecessarily.♿ Suggested accessibility improvement
<div className="bg-accent text-accent-foreground p-1 rounded-md"> - <Compass className="w-5 h-5" /> + <Compass className="w-5 h-5" aria-hidden="true" /> </div>samples/travel planner agent/frontend/src/components/ui/drawer.tsx (1)
51-51: Consider addingaria-hiddento the decorative drag handle.The visual drag indicator is decorative and doesn't convey meaningful content to assistive technologies.
♻️ Suggested improvement
- <div className="mx-auto mt-4 h-2 w-[100px] rounded-full bg-muted" /> + <div aria-hidden="true" className="mx-auto mt-4 h-2 w-[100px] rounded-full bg-muted" />samples/travel planner agent/frontend/src/components/SearchHero.tsx (1)
13-17: Pass a normalized query toonSearch.You’re trimming only for emptiness but still send the raw string. Consider sending the trimmed value for consistency.
♻️ Proposed refactor
const handleSearch = (e: React.FormEvent) => { e.preventDefault(); - if (!query.trim()) return; - onSearch?.(query); + const normalized = query.trim(); + if (!normalized) return; + onSearch?.(normalized); setQuery(""); };samples/travel planner agent/frontend/src/pages/not-found.tsx (1)
14-16: Consider a more user-friendly message for production use.The current message "Did you forget to add the page to the router?" is developer-oriented. For a sample application this is fine for educational purposes, but if this pattern is reused in production, consider a more user-friendly message like "The page you're looking for doesn't exist."
samples/travel planner agent/frontend/src/shared/schema.ts (2)
1-16: Consider using a consistent type forrating.The
ratingfield is typed asstring | number(line 6), which may lead to inconsistent handling in components (e.g., comparisons, formatting). If the backend can return either type, consider normalizing it at the API boundary, or document the expected formats.
28-36: Consider addingroomIdto the Booking type.The
Bookingtype referenceshotelIdbut not a specific room. If room selection is part of the booking flow (as suggested by theRoomtype), consider adding an optionalroomIdfield for completeness.export type Booking = { id: number; hotelId: number; + roomId?: number; guestName: string; checkIn: string; checkOut: string; guests: number; totalPrice: number; };samples/travel planner agent/frontend/src/components/ui/scroll-area.tsx (1)
6-22: Consider supporting horizontal scrollbars for content that may overflow horizontally.The
ScrollAreacomponent only renders a verticalScrollBar(line 18). If any consumers require horizontal scrolling (e.g., wide tables or carousels), they won't see a scrollbar. This is the standard shadcn/ui pattern, but you may want to add an optional horizontal scrollbar or document this limitation.♻️ Optional: Add horizontal scrollbar support
>(({ className, children, ...props }, ref) => ( <ScrollAreaPrimitive.Root ref={ref} className={cn("relative overflow-hidden", className)} {...props} > <ScrollAreaPrimitive.Viewport className="h-full w-full rounded-[inherit]"> {children} </ScrollAreaPrimitive.Viewport> <ScrollBar /> + <ScrollBar orientation="horizontal" /> <ScrollAreaPrimitive.Corner /> </ScrollAreaPrimitive.Root> ))samples/travel planner agent/frontend/tailwind.config.js (1)
26-50: Inconsistent color value format for border properties.Some border colors use
var(--xxx-border)directly (lines 29, 34, 39, 44, 49) while others usehsl(var(--xxx) / <alpha-value>)format (lines 14, 19, 24). This inconsistency means Tailwind's opacity modifiers (e.g.,border-primary-border/50) won't work on the non-hsl variants.If the CSS variables already contain full color values (e.g.,
hsl(...)orrgb(...)), this is fine. Otherwise, consider using the consistenthsl()wrapper pattern.samples/travel planner agent/frontend/src/components/ChatHotelResults.tsx (1)
145-149: Potential duplicate key issue with amenities.Using the amenity string directly as the key could cause React reconciliation issues if a hotel has duplicate amenity names in the array.
Suggested fix
- {amenities.map((amenity) => ( - <span key={amenity} className="amenity-tag"> + {amenities.map((amenity, index) => ( + <span key={`${amenity}-${index}`} className="amenity-tag"> {amenity} </span> ))}samples/travel planner agent/backend/agent/hotel/mock_dataset.json (1)
323-325: Zero price entries may cause display issues.Several hotels have
lowestPrice: 0.0(Galle Fort Hotel at line 323, Le Grand Galle at line 415, The Bartizan at line 461). The frontend'sformatPricefunction returns "N/A" for these, which is handled correctly. If this is intentional for "contact for pricing" scenarios, consider adding a flag likepriceOnRequest: trueto make the intent explicit.samples/travel planner agent/frontend/src/components/ui/card.tsx (1)
32-45: Consider using a semantic heading element for CardTitle.
CardTitlerenders a<div>but semantically represents a title/heading. For better accessibility and semantic HTML, consider rendering an<h3>or allowing the heading level to be configurable via anasprop.This is a common pattern in shadcn/ui where the default can remain a
<div>but the type annotation should match.samples/travel planner agent/frontend/src/lib/queryClient.ts (1)
31-34:queryKey.join("/")assumes all array elements are stringifiable.
QueryKeyisreadonly unknown[], so joining may produce unexpected URLs if non-string values are present (e.g., objects become[object Object]). Consider validating or mapping the key elements.Proposed defensive approach
async ({ queryKey }) => { - const res = await fetch(queryKey.join("/") as string, { + const url = queryKey.map(String).join("/"); + const res = await fetch(url, { credentials: "include", });samples/travel planner agent/frontend/src/components/ui/input-otp.tsx (1)
34-36: Missing bounds check onslots[index]access.If
indexis out of bounds,inputOTPContext.slots[index]will beundefined, causing destructuring to fail at runtime. Consider adding a defensive check.Proposed defensive approach
>(({ index, className, ...props }, ref) => { const inputOTPContext = React.useContext(OTPInputContext) - const { char, hasFakeCaret, isActive } = inputOTPContext.slots[index] + const slot = inputOTPContext.slots[index] + if (!slot) { + return null + } + const { char, hasFakeCaret, isActive } = slotsamples/travel planner agent/frontend/src/components/ui/carousel.tsx (1)
86-97: Keyboard navigation doesn't account for vertical orientation.The handler only responds to
ArrowLeft/ArrowRight. For vertical carousels, users expectArrowUp/ArrowDownto navigate slides.Proposed enhancement
const handleKeyDown = React.useCallback( (event: React.KeyboardEvent<HTMLDivElement>) => { - if (event.key === "ArrowLeft") { + const prevKeys = orientation === "horizontal" ? ["ArrowLeft"] : ["ArrowUp"] + const nextKeys = orientation === "horizontal" ? ["ArrowRight"] : ["ArrowDown"] + + if (prevKeys.includes(event.key)) { event.preventDefault() scrollPrev() - } else if (event.key === "ArrowRight") { + } else if (nextKeys.includes(event.key)) { event.preventDefault() scrollNext() } }, - [scrollPrev, scrollNext] + [orientation, scrollPrev, scrollNext] )samples/travel planner agent/frontend/src/components/ui/breadcrumb.tsx (1)
7-13:separatorprop is unused.
It’s declared but never read, so passing it has no effect. Either remove it or wire it into separator rendering to avoid a misleading API.♻️ Suggested cleanup
-const Breadcrumb = React.forwardRef< - HTMLElement, - React.ComponentPropsWithoutRef<"nav"> & { - separator?: React.ReactNode - } ->(({ ...props }, ref) => <nav ref={ref} aria-label="breadcrumb" {...props} />) +const Breadcrumb = React.forwardRef< + HTMLElement, + React.ComponentPropsWithoutRef<"nav"> +>(({ ...props }, ref) => <nav ref={ref} aria-label="breadcrumb" {...props} />)samples/travel planner agent/backend/agent/hotel/hotel_search.py (3)
148-167: Consider using Python's built-insorted()instead of bubble sort.The manual bubble sort implementation has O(n²) time complexity. While acceptable for small mock datasets, using Python's built-in
sorted()with a key function would be more idiomatic and efficient (O(n log n)).♻️ Suggested refactor
-def _sort_hotels_by_price(items: list[dict[str, Any]], ascending: bool) -> list[dict[str, Any]]: - sorted_hotels = items[:] - n = len(sorted_hotels) - for i in range(n - 1): - for j in range(n - i - 1): - left = sorted_hotels[j].get("lowestPrice", 0) - right = sorted_hotels[j + 1].get("lowestPrice", 0) - if (left > right and ascending) or (left < right and not ascending): - sorted_hotels[j], sorted_hotels[j + 1] = sorted_hotels[j + 1], sorted_hotels[j] - return sorted_hotels +def _sort_hotels_by_price(items: list[dict[str, Any]], ascending: bool) -> list[dict[str, Any]]: + return sorted(items, key=lambda h: h.get("lowestPrice", 0), reverse=not ascending) -def _sort_hotels_by_rating(items: list[dict[str, Any]]) -> list[dict[str, Any]]: - sorted_hotels = items[:] - n = len(sorted_hotels) - for i in range(n - 1): - for j in range(n - i - 1): - if sorted_hotels[j].get("rating", 0) < sorted_hotels[j + 1].get("rating", 0): - sorted_hotels[j], sorted_hotels[j + 1] = sorted_hotels[j + 1], sorted_hotels[j] - return sorted_hotels +def _sort_hotels_by_rating(items: list[dict[str, Any]]) -> list[dict[str, Any]]: + return sorted(items, key=lambda h: h.get("rating", 0), reverse=True)
68-73: Add error handling for dataset loading.If the mock dataset file is missing or contains invalid JSON, the exception will propagate up and potentially crash the application. Consider adding error handling to provide a graceful fallback or clearer error message.
♻️ Suggested improvement
def _load_dataset() -> dict[str, Any]: global _dataset_cache with _dataset_lock: if _dataset_cache is None: - _dataset_cache = json.loads(_dataset_path.read_text()) + try: + _dataset_cache = json.loads(_dataset_path.read_text()) + except FileNotFoundError: + logger.warning("Mock dataset file not found at %s", _dataset_path) + _dataset_cache = {} + except json.JSONDecodeError as e: + logger.error("Failed to parse mock dataset: %s", e) + _dataset_cache = {} return _dataset_cache or {}
301-345: Redundant cache lookup inget_hotel_details.On line 312,
get_cached_hotel(hotel_id)is called again even thoughcachedwas already retrieved on line 308. Thehotel = cached or get_cached_hotel(hotel_id)pattern is redundant sincecachedwould already contain the result.♻️ Suggested fix
def get_hotel_details( api_key: str | None, hotel_id: str, check_in_date: str | None = None, check_out_date: str | None = None, guests: int = 2, ) -> dict[str, Any]: cached = get_cached_hotel(hotel_id) if check_in_date and check_out_date: rates = _mock_rates_for_hotel(hotel_id) rooms_out = _build_rooms_from_rates(hotel_id, rates, guests) - hotel = cached or get_cached_hotel(hotel_id) + hotel = cached if not hotel: data = _load_dataset()samples/travel planner agent/resources/policy_pdfs/requirements.txt (1)
1-4: Consider pinning dependency versions for reproducibility.Unpinned dependencies can lead to inconsistent environments when packages release breaking changes.
Suggested fix
-requests -beautifulsoup4 -reportlab -python-dotenv +requests>=2.32.0,<3.0.0 +beautifulsoup4>=4.14.0,<5.0.0 +reportlab>=4.4.0,<5.0.0 +python-dotenv>=1.2.0,<2.0.0samples/travel planner agent/frontend/tsconfig.json (1)
9-9: Consider enabling strict mode for better type safety.
strict: falsedisables TypeScript's strict type checking, which can allow subtle bugs to slip through (e.g., implicitany, null/undefined issues). For sample code that serves as a reference implementation, enabling strict mode would demonstrate best practices.♻️ Suggested change
- "strict": false, + "strict": true,samples/travel planner agent/frontend/src/index.tsx (1)
7-21: Consider adding React.StrictMode and handling missing root element.Two suggestions for robustness:
- The missing root element case fails silently. Consider logging a warning or throwing an error.
- Wrapping with
React.StrictModehelps identify potential problems during development.♻️ Suggested improvements
import { createRoot } from "react-dom/client"; +import { StrictMode } from "react"; import { AsgardeoProvider } from "@asgardeo/react"; import App from "./App"; import authConfig from "./config/auth"; import "./index.css"; const root = document.getElementById("root"); -if (root) { +if (!root) { + throw new Error("Root element not found"); +} + - createRoot(root).render( - <AsgardeoProvider - clientId={authConfig.clientId} - baseUrl={authConfig.baseUrl} - scopes={authConfig.scopes} - afterSignInUrl={authConfig.afterSignInUrl} - afterSignOutUrl={authConfig.afterSignOutUrl} - storage={authConfig.storage} - > - <App /> - </AsgardeoProvider> - ); -} +createRoot(root).render( + <StrictMode> + <AsgardeoProvider + clientId={authConfig.clientId} + baseUrl={authConfig.baseUrl} + scopes={authConfig.scopes} + afterSignInUrl={authConfig.afterSignInUrl} + afterSignOutUrl={authConfig.afterSignOutUrl} + storage={authConfig.storage} + > + <App /> + </AsgardeoProvider> + </StrictMode> +);samples/travel planner agent/frontend/src/pages/Landing.tsx (1)
16-18: Consider a base-URL-safe asset reference for the hero image.
Using"/images/..."can break if the app is served from a subpath. Prefer the bundler’s base URL helper or import the asset.samples/travel planner agent/frontend/src/lib/mockData.ts (1)
3-79: Normalize ratings to numbers for consistent sorting/formatting.
Hotel.ratingallowsnumber; using numeric values avoids lexicographic comparisons.♻️ Example normalization
- rating: "4.9", + rating: 4.9, ... - rating: "4.7", + rating: 4.7, ... - rating: "4.8", + rating: 4.8, ... - rating: "4.5", + rating: 4.5, ... - rating: "4.9", + rating: 4.9, ... - rating: "4.6", + rating: 4.6,samples/travel planner agent/frontend/src/hooks/use-hotels.ts (1)
30-41: Fallback returns rooms from unrelated hotels.When no rooms match the
hotelId, the fallback returns the first two entries fromMOCK_ROOMS, which belong tohotelId: 1. This could display incorrect room data for other hotels.Consider returning an empty array or generating placeholder rooms that clearly indicate they're generic:
Suggested fix
// For mock purposes, just return some rooms if they match, or generic ones const rooms = MOCK_ROOMS.filter(r => r.hotelId === hotelId); - // Fallback to generic rooms if none specifically assigned in mock - return rooms.length > 0 ? rooms : MOCK_ROOMS.slice(0, 2); + return rooms; },samples/travel planner agent/frontend/src/pages/BookingSummary.tsx (2)
8-31: Local type duplicates and extends shared schema.A
Bookingtype exists inshared/schema.ts(lines 27-35) with a simpler structure. This local type is more comprehensive to match the API response. Consider either:
- Extending the shared type here
- Updating the shared schema to be the source of truth
This is acceptable for now but may cause maintenance overhead.
89-122: Missing cleanup for async effect.If the component unmounts while the fetch is in progress, the state updates (
setBookings,setLoading,setError) will attempt to run on an unmounted component. Consider adding an AbortController for cleanup:Suggested fix
useEffect(() => { + const controller = new AbortController(); const loadBookings = async () => { try { if (!isSignedIn) { setBookings([]); setLoading(false); return; } const userId = getOrCreateUserId(); const token = await getAccessToken(); const headers: Record<string, string> = { "x-user-id": userId }; if (token) { headers.Authorization = `Bearer ${token}`; } - const response = await fetch(`${API_BASE_URL}/bookings`, { headers }); + const response = await fetch(`${API_BASE_URL}/bookings`, { + headers, + signal: controller.signal + }); if (!response.ok) { throw new Error(`Failed to load bookings: ${response.status}`); } // ... rest of the code } catch (err) { if ((err as Error)?.name !== "AbortError") { setError("Unable to load bookings right now."); } } finally { - setLoading(false); + if (!controller.signal.aborted) { + setLoading(false); + } } }; loadBookings(); + return () => controller.abort(); }, [getAccessToken, isSignedIn]);samples/travel planner agent/resources/policy_pdfs/generate_policy_pdfs.py (1)
46-56: Consider adding error handling for file operations.The
_load_mock_hotelsfunction will raise an unhandled exception if the dataset file doesn't exist or contains invalid JSON. For a build-time script, adding a clear error message would improve the developer experience.Suggested improvement
def _load_mock_hotels() -> List[Dict[str, Any]]: + if not DATASET_PATH.exists(): + raise FileNotFoundError(f"Mock dataset not found at {DATASET_PATH}") - data = json.loads(DATASET_PATH.read_text()) + try: + data = json.loads(DATASET_PATH.read_text()) + except json.JSONDecodeError as e: + raise ValueError(f"Invalid JSON in {DATASET_PATH}: {e}") from e return data.get("hotels", [])[:MAX_HOTELS]samples/travel planner agent/backend/agent/graph.py (2)
1-17: Unused import:END.
ENDis imported fromlanggraph.graphbut never used directly. Thetools_conditionfunction handles returningENDinternally.Suggested fix
-from langgraph.graph import StateGraph, END +from langgraph.graph import StateGraph
76-86: Remove leftover comment.Line 76 contains what appears to be a development note (
#add in memory server) that should be cleaned up before merging.Suggested fix
- graph = StateGraph(AgentState) `#add` in memory server + graph = StateGraph(AgentState)samples/travel planner agent/backend/agent/app.py (3)
10-11: Unused imports.
Literalandreare imported but never used in this file.Suggested fix
from pydantic import BaseModel -from typing import Literal -import re
25-26: Module-level initialization may cause startup failures.
Settings.from_env()andbuild_graph(settings)run at import time. If required environment variables are missing, the application will fail to start without a clear error message in the FastAPI context. Consider deferring initialization or wrapping in a lifespan handler for cleaner error reporting.
77-83: Missing error handling for agent graph invocation.If
agent_graph.invoke()raises an exception (e.g., OpenAI API errors, recursion limit exceeded), the endpoint will return a 500 error without a user-friendly message. Consider wrapping this in a try/except to return a proper error response.Suggested fix
+ try: result = agent_graph.invoke( {"messages": [HumanMessage(content=wrapped_message)]}, config={ "recursion_limit": 50, "configurable": {"thread_id": thread_id}, }, ) + except Exception as e: + logging.getLogger(__name__).exception("Agent graph invocation failed") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to process your request. Please try again.", + )samples/travel planner agent/frontend/src/pages/Home.tsx (2)
318-319: Potential message ID collision.Using
(Date.now() + 1).toString()for the assistant message ID could still collide with subsequent user messages if requests complete quickly. Consider using a UUID or combining timestamp with a random suffix.Suggested fix
const assistantMessage: Message = { - id: (Date.now() + 1).toString(), + id: `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, role: 'assistant', content: data?.message || "Sorry, I couldn't generate a response.", };
333-343: Silent error swallowing.The catch block doesn't log the error, making debugging difficult. Consider adding
console.errorfor the caught exception.Suggested fix
} catch (error) { + console.error("Chat request failed:", error); const assistantMessage: Message = {samples/travel planner agent/resources/ingest/ingest.py (3)
128-130: Consider batching for large documents.Pinecone's upsert has a limit (typically 100 vectors per request for some tiers). For large policy documents that produce many chunks, this could fail. Consider batching the upsert.
Suggested fix
# Batch upsert to Pinecone if vectors: - self.index.upsert(vectors=vectors) + batch_size = 100 + for i in range(0, len(vectors), batch_size): + self.index.upsert(vectors=vectors[i:i + batch_size])
85-86: Exception chaining loses original traceback.Catching
Exceptionand raising a newExceptionloses the original traceback. Useraise ... from eto preserve the exception chain for debugging.Suggested fix
except Exception as e: - raise Exception(f"Error reading PDF {pdf_path}: {str(e)}") + raise Exception(f"Error reading PDF {pdf_path}: {str(e)}") from eApply the same pattern to lines 93-94 and 104-105.
166-168: Fragile path calculation.Using
parents[2]assumes a specific directory structure. If the script is moved or the structure changes, this will break silently. Consider using an environment variable or explicit configuration for the policies directory.
| fastapi==0.115.0 | ||
| uvicorn==0.30.6 | ||
| langgraph==0.2.34 | ||
| langchain==0.3.7 | ||
| langchain-openai==0.2.6 | ||
| pydantic==2.9.2 | ||
| requests==2.32.3 | ||
| pinecone-client==3.2.2 | ||
| python-dotenv==1.0.1 | ||
| psycopg[binary]==3.2.3 | ||
| PyJWT[crypto]==2.9.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions and security status of key packages
echo "=== Latest versions on PyPI ==="
for pkg in fastapi langchain langgraph langchain-openai starlette; do
version=$(curl -s "https://pypi.org/pypi/$pkg/json" 2>/dev/null | jq -r '.info.version' 2>/dev/null)
echo "$pkg: $version"
done
echo ""
echo "=== Checking FastAPI version and Starlette dependency ==="
curl -s "https://pypi.org/pypi/fastapi/0.115.0/json" 2>/dev/null | jq -r '.info.requires_dist[]' 2>/dev/null | grep -i starlette || echo "FastAPI 0.115.0 requires_dist not available"Repository: wso2/ai-agent-management-platform
Length of output: 286
🌐 Web query:
GHSA-6qv9-48xg-fc7f langchain-core template injection vulnerability
💡 Result:
GHSA-6qv9-48xg-fc7f (CVE-2025-65106) is a template injection issue in LangChain’s langchain-core prompt template system where an attacker who can supply untrusted template strings (the template structure, not just variable values) can use attribute access (.) and indexing ([]) in templates to traverse Python objects and potentially exfiltrate sensitive data (e.g., via objects provided through MessagesPlaceholder). Method calls () are not allowed, but traversal can still be dangerous depending on objects in scope. [1][2]
Affected versions
langchain-core≤ 0.3.79langchain-core1.0.0 – 1.0.6 [1][3]
Fixed versions
- Upgrade to 0.3.80+ or 1.0.7+ [1][3]
Key remediation guidance
- Do not accept template strings from untrusted users/sources; only accept user input as variable values. [1][2]
- After the fix, f-string templates validate variable names and reject attribute/index traversal patterns (e.g.,
{obj.attr}/{obj[0]}) at template creation time. [1] - Note: LangChain’s Jinja2 hardening introduced initially was later reverted as of
langchain-core1.1.3; Jinja2 should be treated as trusted-templates-only. [1]
References: GitHub Security Advisory (GHSA) [1], OSV entry [2], CVE summary/affected-fixed versions [3].
Sources: [1] [2] [3]
🌐 Web query:
GHSA-m42m-m8cr-8m58 langchain-text-splitters XXE vulnerability
💡 Result:
GHSA-m42m-m8cr-8m58 / CVE-2025-6985 (langchain-text-splitters XXE)
- What it is: An XML External Entity (XXE) / unsafe XSLT parsing issue in
langchain-text-splitters, in theHTMLSectionSplitterclass, caused by parsing attacker-controlled XSLT withlxml.etree.parse()/lxml.etree.XSLT()without hardening. This can enable arbitrary file reads (and in some configurations, outbound network fetches). [1][2] - Severity: High (CVSS 7.5). [1][3]
- Affected versions:
langchain-text-splitters< 0.3.9. [1][2] - Fixed version: 0.3.9. [1][2]
- Recommended remediation: Upgrade to
langchain-text-splitters>=0.3.9. [1][2]
Sources: GitHub Advisory Database [1], GitLab Advisory Database [2], OSV [3].
🌐 Web query:
GHSA-wwqv-p2pp-99h5 langgraph-checkpoint RCE JsonPlusSerializer
💡 Result:
GHSA-wwqv-p2pp-99h5 (CVE-2025-64439) is a remote code execution (RCE) vulnerability in langgraph-checkpoint caused by unsafe deserialization in JsonPlusSerializer when loading checkpoints that were saved/handled in "json" mode. An attacker who can get a malicious payload persisted and later deserialized can execute arbitrary Python code. [1][2]
- Affected:
langgraph-checkpoint< 3.0.0 (reported as “2.1.2 and below” in some advisories). [1][2] - Trigger / root cause: the serializer normally uses msgpack, but older versions could fallback to
"json"(e.g., when msgpack serialization fails due to illegal Unicode surrogate values); the"json"path supports a constructor-style format that can be abused for code execution during deserialization. [1][2] - Fixed in:
langgraph-checkpoint==3.0.0. The fix adds an allow-list for constructor deserialization and deprecates saving payloads in"json"format to remove the unsafe fallback path. [1] - Mitigation: Upgrade to
langgraph-checkpoint>=3.0.0. If you uselanggraph-api, upgrading to0.5+pulls in the patched checkpointer. [1][3]
Sources: [1] GitHub Advisory (GHSA-wwqv-p2pp-99h5) [2] INCIBE CVE entry [3] GitLab Advisory Database
🌐 Web query:
GHSA-2c2j-9gv5-cj73 starlette doS multipart forms vulnerability
💡 Result:
GHSA-2c2j-9gv5-cj73 (Starlette) — multipart form DoS
- Alias / CVE: CVE-2025-54121. [1]
- Issue: When Starlette parses
multipart/form-datauploads with large files (larger than the default spooled-file in‑memory threshold), it may block the main/event loop thread while rolling the upload to disk, preventing the server from accepting new connections (denial of service). The root cause is a bug inUploadFile.write()where the “in memory vs rolled to disk” decision doesn’t account for whether the next write will trigger rollover. [1][2] - Affected versions: Starlette < 0.47.2 (commonly summarized as 0.47.1 and below). [2][3]
- Fixed version: 0.47.2. [1][2]
- Severity: CVSS 3.1: 5.3 (Medium). [1][2]
- Recommended remediation: Upgrade Starlette to >= 0.47.2. [1][2]
Sources: OSV entry for GHSA-2c2j-9gv5-cj73 [1], GitLab Advisory Database (CVE-2025-54121) [2], OpenCVE summary for CVE-2025-54121 [3].
🌐 Web query:
GHSA-f96h-pmfr-66vw starlette vulnerability
💡 Result:
GHSA-f96h-pmfr-66vw (Starlette) = CVE-2024-47874: a Denial-of-Service (DoS) issue in Starlette’s handling of multipart/form-data. Parts without a filename are treated as text fields and were buffered with no size limit, letting an attacker send extremely large fields to drive excessive memory usage / OOM and degrade or stop the service. This can remain effective even if a reverse proxy enforces “reasonable” overall request-size limits. [1][2]
- Affected:
starlette< 0.40.0 [1][2] - Fixed: 0.40.0 (adds a
max_part_sizelimit inMultiPartParser) [1][3] - What to do: Upgrade to
starlette>=0.40.0(and redeploy). [1][2]
Sources:
[1] GitHub Advisory Database (GHSA-f96h-pmfr-66vw) (github.com)
[2] OSV entry for GHSA-f96h-pmfr-66vw (osv.dev)
[3] Safety CLI vulnerability record (notes max_part_size fix / commit) (data.safetycli.com)
Citations:
- 1: GHSA-f96h-pmfr-66vw
- 2: https://osv.dev/vulnerability/GHSA-f96h-pmfr-66vw?utm_source=openai
- 3: https://data.safetycli.com/vulnerabilities/CVE-2024-47874/73725?utm_source=openai
Update dependencies to address critical security vulnerabilities.
Multiple known public vulnerabilities affect the pinned versions:
- langchain-core (transitive via langchain==0.3.7): CVE-2025-65106 — template injection enabling attribute traversal and potential data exfiltration. Fixed in 0.3.80+.
- langchain-text-splitters (transitive via langchain==0.3.7): CVE-2025-6985 — XXE/unsafe XSLT parsing enabling arbitrary file reads. Fixed in 0.3.9+.
- langgraph-checkpoint (transitive via langgraph==0.2.34): CVE-2025-64439 — unsafe JSON deserialization enabling remote code execution. Fixed in 3.0.0+.
- Starlette (required by fastapi==0.115.0): CVE-2024-47874 and CVE-2025-54121 — denial-of-service via multipart form handling. Fixed in 0.40.0+ and 0.47.2+ respectively. Current constraint requires starlette<0.39.0, leaving the package vulnerable.
Upgrade all dependencies to latest stable versions before merging. Given that this is a sample others will copy, using outdated packages with public CVEs creates downstream security risks.
🧰 Tools
🪛 OSV Scanner (2.3.2)
[CRITICAL] 1-1: langchain-core 0.3.63: LangChain Vulnerable to Template Injection via Attribute Access in Prompt Templates
[CRITICAL] 1-1: langchain-core 0.3.63: LangChain serialization injection vulnerability enables secret extraction in dumps/loads APIs
[HIGH] 1-1: langchain-text-splitters 0.3.8: LangChain Text Splitters is vulnerable to XML External Entity (XXE) attacks due to unsafe XSLT parsing
[HIGH] 1-1: langgraph-checkpoint 2.1.2: LangGraph Checkpoint affected by RCE in "json" mode of JsonPlusSerializer
[HIGH] 1-1: starlette 0.38.6: Starlette has possible denial-of-service vector when parsing large files in multipart forms
[HIGH] 1-1: starlette 0.38.6: Starlette Denial of service (DoS) via multipart/form-data
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/agent/requirements.txt around lines 1 -
11, The requirements.txt pins vulnerable packages; update the pinned versions in
requirements.txt so fastapi, langchain, and langgraph (and any transitive
packages) are at safe releases: bump langchain to >=0.3.80 (addresses
langchain-core CVE), ensure langchain-text-splitters is >=0.3.9 (if directly
listed or via transitive constraints), bump langgraph to >=3.0.0 (fixes
langgraph-checkpoint), and upgrade fastapi to a release that allows Starlette
>=0.47.2 (so Starlette gets >=0.47.2 to mitigate CVE-2024/2025 issues); also
update related pins (langchain-openai, etc.) to compatible latest stable
releases, then regenerate your lock/install (pip-compile or pip install -U) and
run the test suite to verify compatibility before merging.
| response = requests.get(endpoint, params=params, timeout=30) | ||
| response.raise_for_status() | ||
| return response.text |
There was a problem hiding this comment.
Missing error handling for weather API call.
Unlike the booking tools which properly catch requests.RequestException, the weather tool will propagate exceptions directly if the API call fails or times out, causing the agent to crash.
Suggested fix
- response = requests.get(endpoint, params=params, timeout=30)
- response.raise_for_status()
- return response.text
+ try:
+ response = requests.get(endpoint, params=params, timeout=30)
+ response.raise_for_status()
+ return response.text
+ except requests.RequestException:
+ logger.exception("get_weather_forecast_tool failed")
+ return "Weather service unavailable."📝 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.
| response = requests.get(endpoint, params=params, timeout=30) | |
| response.raise_for_status() | |
| return response.text | |
| try: | |
| response = requests.get(endpoint, params=params, timeout=30) | |
| response.raise_for_status() | |
| return response.text | |
| except requests.RequestException: | |
| logger.exception("get_weather_forecast_tool failed") | |
| return "Weather service unavailable." |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/agent/tools.py around lines 486 - 488,
The weather API call (the requests.get / response.raise_for_status block in the
weather tool function in tools.py) lacks error handling; wrap the
requests.get(...) and response.raise_for_status() calls in a try/except that
catches requests.RequestException as e (same pattern used by the booking tools),
and handle it consistently (either return a clear error message or raise the
same ToolException/error type the booking tools use) so network errors and
timeouts don't propagate and crash the agent.
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["*"], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| max_age=86400, | ||
| ) |
There was a problem hiding this comment.
Security: Overly permissive CORS configuration.
Using allow_origins=["*"] with allow_credentials=True is insecure. While browsers typically block this combination, this configuration signals intent to allow any origin with credentials, which could lead to CSRF-style attacks if relaxed further.
For a sample/demo API this may be acceptable, but consider restricting origins to specific allowed domains for production use.
Suggested fix for production
app.add_middleware(
CORSMiddleware,
- allow_origins=["*"],
+ allow_origins=["http://localhost:3000"], # Restrict to known frontend origins
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
max_age=86400,
)📝 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.
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=["*"], | |
| allow_credentials=True, | |
| allow_methods=["*"], | |
| allow_headers=["*"], | |
| max_age=86400, | |
| ) | |
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=["http://localhost:3000"], # Restrict to known frontend origins | |
| allow_credentials=True, | |
| allow_methods=["*"], | |
| allow_headers=["*"], | |
| max_age=86400, | |
| ) |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/booking_api/booking_api.py around lines
26 - 33, The CORS setup in app.add_middleware using CORSMiddleware is overly
permissive (allow_origins=["*"] with allow_credentials=True); restrict it by
replacing the wildcard origin with an explicit allowlist (e.g., load a list of
allowed domains from config/env) and keep allow_credentials=True only when
origins are specific, or set allow_credentials=False if you must allow all
origins; update the CORSMiddleware call in booking_api.py to use the explicit
allow_origins list (and optionally allow_methods/allow_headers as needed) to
harden production deployments.
samples/travel planner agent/backend/booking_api/booking_api.py
Outdated
Show resolved
Hide resolved
| def _load_bookings() -> list[dict[str, Any]]: | ||
| if not DATA_PATH.exists(): | ||
| return [] | ||
| try: | ||
| return json.loads(DATA_PATH.read_text()) | ||
| except json.JSONDecodeError: | ||
| logger.warning("booking data corrupted; starting fresh") | ||
| return [] | ||
|
|
||
|
|
||
| def _save_bookings(bookings: list[dict[str, Any]]) -> None: | ||
| DATA_PATH.write_text(json.dumps(bookings, indent=2)) |
There was a problem hiding this comment.
Concurrency: File-based persistence without locking causes race conditions.
Multiple concurrent requests can read stale data and overwrite each other's changes. For example, two simultaneous create_booking calls could both load the same booking list, append their booking, and save—resulting in one booking being lost.
For a sample app this may be acceptable, but consider using file locking or a proper database for production.
Suggested fix using file locking
+import fcntl
+
+_bookings_lock = threading.Lock()
+
def _load_bookings() -> list[dict[str, Any]]:
- if not DATA_PATH.exists():
- return []
- try:
- return json.loads(DATA_PATH.read_text())
- except json.JSONDecodeError:
- logger.warning("booking data corrupted; starting fresh")
- return []
+ with _bookings_lock:
+ if not DATA_PATH.exists():
+ return []
+ try:
+ return json.loads(DATA_PATH.read_text())
+ except json.JSONDecodeError:
+ logger.warning("booking data corrupted; starting fresh")
+ return []
def _save_bookings(bookings: list[dict[str, Any]]) -> None:
- DATA_PATH.write_text(json.dumps(bookings, indent=2))
+ with _bookings_lock:
+ DATA_PATH.write_text(json.dumps(bookings, indent=2))🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/booking_api/booking_api.py around lines
203 - 214, The current file-based persistence in _load_bookings and
_save_bookings races under concurrent requests; fix by introducing an OS-level
file lock around read-modify-write sequences (use a library like portalocker or
fcntl) to ensure exclusive access to DATA_PATH, acquire the lock before calling
_load_bookings, perform updates (e.g. in create_booking/update/delete handlers),
and hold the lock until an atomic write is completed (write to a temp file and
os.replace to DATA_PATH) inside _save_bookings; update callers to use this
lock-protected sequence so no two operations can read stale data and overwrite
each other.
| const badgeVariants = cva( | ||
| // Whitespace-nowrap: Badges should never wrap. | ||
| "whitespace-nowrap inline-flex items-center rounded-md border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2" + | ||
| " hover-elevate " , | ||
| { | ||
| variants: { | ||
| variant: { | ||
| default: | ||
| "border-transparent bg-primary text-primary-foreground shadow-xs", | ||
| secondary: "border-transparent bg-secondary text-secondary-foreground", | ||
| destructive: | ||
| "border-transparent bg-destructive text-destructive-foreground shadow-xs", | ||
|
|
||
| outline: " border [border-color:var(--badge-outline)] shadow-xs", | ||
| }, | ||
| }, | ||
| defaultVariants: { | ||
| variant: "default", | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for hover-elevate class definition
echo "=== Searching for hover-elevate definition ==="
find . -type f \( -name "*.css" -o -name "*.scss" -o -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) ! -path "*/node_modules/*" -exec grep -l "hover-elevate" {} \;
echo ""
echo "=== Searching with rg for hover-elevate ==="
rg -i "hover-elevate" -g "!node_modules"
echo ""
echo "=== Searching for --badge-outline CSS variable ==="
rg --no-heading "badge-outline" -g "!node_modules"
echo ""
echo "=== Checking Tailwind config files ==="
find . -name "tailwind.config.*" ! -path "*/node_modules/*" -type fRepository: wso2/ai-agent-management-platform
Length of output: 788
🏁 Script executed:
#!/bin/bash
echo "=== Content of tailwind.config.js ==="
cat "./samples/travel planner agent/frontend/tailwind.config.js"
echo ""
echo "=== Searching for CSS/SCSS files ==="
find "./samples/travel planner agent/frontend" -type f \( -name "*.css" -o -name "*.scss" \) ! -path "*/node_modules/*"
echo ""
echo "=== Searching for globals.css or similar ==="
find "./samples/travel planner agent/frontend" -type f -name "globals.css" -o -name "global.css" -o -name "index.css"
echo ""
echo "=== Checking the app directory structure ==="
ls -la "./samples/travel planner agent/frontend/src/" 2>/dev/null || echo "src directory not found"Repository: wso2/ai-agent-management-platform
Length of output: 4989
🏁 Script executed:
#!/bin/bash
echo "=== Content of index.css ==="
cat "./samples/travel planner agent/frontend/src/index.css"Repository: wso2/ai-agent-management-platform
Length of output: 13909
Define the missing hover-elevate utility class and --badge-outline CSS variable.
The hover-elevate class (line 9) and --badge-outline CSS variable (line 19) are not defined in the codebase. Add them to src/index.css:
Example definitions
/* Add to :root in index.css */
--badge-outline: 214 32% 91%;
/* Add to `@layer` utilities in index.css */
.hover-elevate {
`@apply` transition-transform hover:translate-y-[-2px];
}🤖 Prompt for AI Agents
In `@samples/travel` planner agent/frontend/src/components/ui/badge.tsx around
lines 6 - 26, The badgeVariants utility uses an undefined CSS utility class
hover-elevate and CSS variable --badge-outline; add a global CSS variable
--badge-outline (in :root) with the intended HSL/RGB value and define a utility
class .hover-elevate in your main stylesheet's utility layer (e.g., `@layer`
utilities in src/index.css) that applies the desired transform/transition on
hover; update the file so badgeVariants (from badge.tsx) can rely on the new
.hover-elevate class and --badge-outline variable being present.
| const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => { | ||
| const colorConfig = Object.entries(config).filter( | ||
| ([, config]) => config.theme || config.color | ||
| ) | ||
|
|
||
| if (!colorConfig.length) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <style | ||
| dangerouslySetInnerHTML={{ | ||
| __html: Object.entries(THEMES) | ||
| .map( | ||
| ([theme, prefix]) => ` | ||
| ${prefix} [data-chart=${id}] { | ||
| ${colorConfig | ||
| .map(([key, itemConfig]) => { | ||
| const color = | ||
| itemConfig.theme?.[theme as keyof typeof itemConfig.theme] || | ||
| itemConfig.color | ||
| return color ? ` --color-${key}: ${color};` : null | ||
| }) | ||
| .join("\n")} | ||
| } | ||
| ` | ||
| ) | ||
| .join("\n"), | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "chart.tsx" | grep -E "samples|travel"Repository: wso2/ai-agent-management-platform
Length of output: 144
🏁 Script executed:
cat -n "./samples/travel planner agent/frontend/src/components/ui/chart.tsx"Repository: wso2/ai-agent-management-platform
Length of output: 13110
🏁 Script executed:
rg "ChartContainer" --type tsx --context 5 | head -80Repository: wso2/ai-agent-management-platform
Length of output: 104
🏁 Script executed:
rg "ChartContainer" -A 5 | head -100Repository: wso2/ai-agent-management-platform
Length of output: 2298
🏁 Script executed:
rg "ChartContainer|config.*=" --max-count 30 | grep -A 2 "config"Repository: wso2/ai-agent-management-platform
Length of output: 9240
🏁 Script executed:
rg "ChartContainer\s*\(" -B 3 -A 10 | head -150Repository: wso2/ai-agent-management-platform
Length of output: 59
🏁 Script executed:
find . -type f -name "*.tsx" -o -name "*.ts" -o -name "*.jsx" -o -name "*.js" | xargs grep -l "ChartContainer" 2>/dev/nullRepository: wso2/ai-agent-management-platform
Length of output: 61
🏁 Script executed:
find ./samples -type f \( -name "*.tsx" -o -name "*.ts" \) | head -20Repository: wso2/ai-agent-management-platform
Length of output: 1466
🏁 Script executed:
grep -r "ChartContainer" "./samples/travel planner agent/frontend/src" -A 5 -B 2Repository: wso2/ai-agent-management-platform
Length of output: 2790
🏁 Script executed:
find "./samples/travel planner agent/frontend" -type f -name "biome.json" -o -name ".biomeignore" -o -name "biome.jsonc"Repository: wso2/ai-agent-management-platform
Length of output: 59
🏁 Script executed:
find . -maxdepth 2 -type f -name "biome.json" -o -name "biome.jsonc" | head -5Repository: wso2/ai-agent-management-platform
Length of output: 89
🏁 Script executed:
cat ./biome.jsonRepository: wso2/ai-agent-management-platform
Length of output: 1851
🏁 Script executed:
grep -r "config\s*=" "./samples/travel planner agent/frontend" -B 2 -A 3 | head -100Repository: wso2/ai-agent-management-platform
Length of output: 681
🏁 Script executed:
grep -r "dangerouslySetInnerHTML" ./samples/travel\ planner\ agent/frontend -B 3 -A 3Repository: wso2/ai-agent-management-platform
Length of output: 698
🏁 Script executed:
python3 << 'EOF'
# Verify CSS.escape availability and safety of proposed fix
import json
# Check what CSS.escape is and where it comes from
css_escape_info = {
"source": "DOM API - available in all modern browsers",
"availability": "Available on window.CSS object in browser environments",
"in_react": "Can be used in React client components (marked 'use client')",
"what_it_does": "Escapes strings for safe use in CSS selectors and identifiers"
}
print(json.dumps(css_escape_info, indent=2))
# Show examples of what could go wrong without escaping
examples = {
"unescaped_key_with_spaces": {
"problem": "key = 'foo bar' produces '--color-foo bar: red;' which is invalid CSS variable syntax",
"with_escape": "--color-foo\\ bar: red;"
},
"unescaped_key_with_special_chars": {
"problem": "key = 'foo[bar]' produces '--color-foo[bar]: red;' which is invalid",
"with_escape": "--color-foo\\[bar\\]: red;"
},
"unescaped_id_in_selector": {
"problem": "id = ':r1:' produces [data-chart=:r1:] which breaks selector parsing",
"with_escape": "[data-chart=\":r1:\"]"
}
}
print("\nExamples of why escaping is needed:")
print(json.dumps(examples, indent=2))
EOFRepository: wso2/ai-agent-management-platform
Length of output: 163
🌐 Web query:
CSS.escape browser API React client component safe usage
💡 Result:
CSS.escape() is a browser API that escapes a string “mostly for use as part of a CSS selector” (e.g., when building #id / .class selectors for querySelector). It’s not HTML escaping. [1]
Safe usage in a React “client component” (esp. Next.js)
Even with "use client", the component is typically pre-rendered on the server and hydrated on the client, so don’t reference CSS / window at module top-level or in code paths that must run during SSR. [4]
Use a guard + run it only on the client (e.g., inside useEffect, event handlers), and avoid changing the first render output to prevent hydration mismatch issues. [5]
"use client";
import { useEffect, useState } from "react";
function cssEscape(value: string) {
if (typeof CSS !== "undefined" && typeof CSS.escape === "function") {
return CSS.escape(value);
}
return value; // or use a polyfill fallback below
}
export function AvatarById({ userId }: { userId: string }) {
const [selector, setSelector] = useState<string | null>(null);
useEffect(() => {
setSelector(`#${cssEscape(userId)} > img`);
}, [userId]);
if (!selector) return null; // ensures first render matches SSR HTML
// safe to use selector in the browser now
return <div data-selector={selector} />;
}Polyfill / fallback
If you need to support environments where CSS.escape might be missing, use a dedicated polyfill like css.escape (implements CSS.escape as defined in CSSOM). [3]
Remove dangerouslySetInnerHTML and escape CSS selector and variable name values.
This code is vulnerable to CSS injection: the unquoted id and unescaped config key values in the style template can break CSS syntax or inject unintended styles if they contain special characters. Use CSS.escape() to safely escape both the id (in the selector) and key (in the CSS variable name), and render the styles via a text node: <style>{css}</style>.
Note: Since this is a "use client" Next.js component, guard the CSS.escape() calls with a type check (e.g., typeof CSS !== 'undefined') to handle potential SSR/hydration edge cases, or use a dedicated CSS escaping utility if available.
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 80-80: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
[error] 81-81: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/frontend/src/components/ui/chart.tsx around
lines 70 - 99, The ChartStyle component currently injects CSS via
dangerouslySetInnerHTML using unescaped id and config keys (see ChartStyle, id,
config, THEMES, colorConfig, itemConfig, key), which risks CSS injection; remove
dangerouslySetInnerHTML, build the CSS string safely by escaping the selector id
and variable names with CSS.escape (guarded with typeof CSS !== 'undefined' or a
fallback escape utility), e.g. escape id when creating the [data-chart=...]
selector and escape key when creating --color-<key> variable names, then render
the stylesheet as a regular React child like <style>{css}</style> so the values
are escaped and not injected raw.
samples/hotel-booking-agent/frontend/src/components/ui/resizable.tsx
Outdated
Show resolved
Hide resolved
samples/travel planner agent/frontend/src/pages/HotelDetails.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We need instructions to setup and test this agent on the agent management platform, similar to https://github.com/wso2/ai-agent-management-platform/tree/main/samples/customer-support-agent
There was a problem hiding this comment.
this file seems to be empty
samples/travel planner agent/backend/booking_api/requirements.txt
Outdated
Show resolved
Hide resolved
samples/travel planner agent/backend/booking_api/booking_api.py
Outdated
Show resolved
Hide resolved
samples/travel planner agent/backend/agent/hotel/mock_dataset.json
Outdated
Show resolved
Hide resolved
| app = FastAPI(title="Travel Planner Agent") | ||
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["http://localhost:3001"], |
There was a problem hiding this comment.
What happens if someone wants to test only the backend. These cors policies can cause a issue in other scenarios ? @venumigihansa
There was a problem hiding this comment.
for someone testing the backend using curl requests curl won't be a problem right because it's enforced by the browser?
samples/travel planner agent/backend/booking_api/booking_api.py
Outdated
Show resolved
Hide resolved
| llm = ChatOpenAI( | ||
| model=settings.openai_model, | ||
| api_key=settings.openai_api_key, | ||
| temperature=0.3, |
nadheesh
left a comment
nadheesh
left a comment
There was a problem hiding this comment.
How do you initialize the vector DB?
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In `@samples/travel` planner agent/backend/agent/app.py:
- Around line 107-123: The chat handler builds thread_id as
f"{user_id}:{session_id}" which becomes "user_id:None" when sessionId is
missing; change construction in chat (and any callers relying on thread_id) to
avoid collapsing all no-session chats: either use a stable default (e.g., a
generated short id) or omit the suffix when request.sessionId is falsy — e.g.,
compute session_id = request.sessionId or None, then set thread_id =
f"{user_id}:{session_id}" only when session_id is truthy, otherwise thread_id =
user_id (or generate a unique id via uuid4() if uniqueness per chat is desired);
update imports if you choose uuid.
- Around line 43-52: _user_name and user_id may contain full email addresses and
should be masked before embedding in prompts; update the _wrap_user_message
function to detect email patterns in user_name and user_id, replace or obfuscate
the domain (e.g., keep local-part only or replace with "[redacted]" or
asterisks) and use those masked values (e.g., masked_user_name, masked_user_id)
when composing the "User Name" and "User Context" lines so no raw email is sent
to the LLM.
- Around line 71-80: The _decode_access_token function currently disables all
JWT checks; replace this by fetching the IdP JWKS (use ASGARDEO_BASE_URL to
build the /.well-known/jwks.json URL) and validate signature, issuer and
audience/expiry using the proper key via jwt.PyJWKClient (or equivalent),
passing ASGARDEO_CLIENT_ID as the expected audience and ASGARDEO_BASE_URL (or
the IdP issuer) as the expected issuer; ensure jwt.decode raises on invalid
tokens and propagate/handle errors accordingly in _decode_access_token so only
tokens with valid signature/iss/aud/exp are accepted.
In `@samples/travel` planner agent/backend/agent/config.py:
- Line 49: The default Weather API base URL currently uses plaintext HTTP;
update the default for weather_api_base_url (the os.getenv call supplying
"WEATHER_API_BASE_URL") to use the secure HTTPS endpoint
"https://api.weatherapi.com/v1" so API keys and traffic are encrypted by
default.
In `@samples/travel` planner agent/backend/agent/tools.py:
- Around line 126-157: The heuristic in _resolve_hotel_id treats any single‑word
hotel_id (e.g., "Hilton") as a valid ID causing failed downstream calls; replace
the simple `" " not in hotel_id` check with a strict ID-format validation (e.g.,
a regex that matches your real booking ID format such as a UUID, DB ObjectId, or
numeric ID) and only treat hotel_id as resolved when it matches that pattern;
otherwise call the search/resolve logic as currently implemented. Reference
symbols: _resolve_hotel_id, hotel_id, candidate_name, and the two API calls
_call_booking_api("/hotels/search", ...) and
_call_booking_api("/hotels/resolve", ...).
- Around line 329-371: create_booking_tool is missing the x-user-id header and
does not guard against non-JSON responses; update the function to add a headers
dict including "x-user-id": resolved_user_id (use the same resolved_user_id
variable) when calling requests.post, and wrap the final response.json() call in
a try/except catching ValueError (or JSONDecodeError) to return a consistent
{"error": "..."} payload on non-JSON responses (mirror the error handling
pattern used in the other booking tools such as
edit_booking_tool/cancel_booking_tool/list_bookings_tool).
In `@samples/travel` planner agent/backend/booking_api/hotel_search.py:
- Around line 306-333: The current branch in the function that handles requests
with check_in_date/check_out_date returns a placeholder "Unknown Hotel" when
get_cached_hotel/_load_dataset/_normalize_hotel find no match; instead, change
this path to raise HotelNotFoundError for unknown hotel_id (same behavior as the
no-dates path) so callers can detect invalid IDs reliably: after attempting
cached = get_cached_hotel(hotel_id), _rooms_for_hotel(hotel_id), and the dataset
lookup via _load_dataset/_normalize_hotel/_cache_hotels, if hotel is still falsy
raise HotelNotFoundError(hotel_id) rather than constructing the Unknown Hotel
dict; keep the rest of the successful return (hotel, rooms_out, recentReviews,
nearbyAttractions) unchanged.
- Around line 66-71: _wrap the file read and JSON parse in _load_dataset with a
try/except that catches OSError and json.JSONDecodeError, and convert them into
a HotelSearchError containing the original exception message so the API layer
can return a consistent error payload; keep the existing caching logic using
_dataset_cache and _dataset_lock but when reading _dataset_path.read_text() or
json.loads(...) fails, raise HotelSearchError(...) (preserving the original
exception as context) instead of letting the error propagate.
In `@samples/travel` planner agent/backend/booking_api/openapi.yaml:
- Around line 11-14: Add definitions for the two referenced security schemes
under components.securitySchemes: declare JWTAuth as an HTTP bearer scheme with
scheme: bearer and bearerFormat: JWT (used by the existing security: - JWTAuth)
and declare UserIdHeader as an apiKey type located in the header with a name
like X-User-Id (used by the existing security: - UserIdHeader); place these
entries under components.securitySchemes so the existing security block (JWTAuth
and UserIdHeader) resolves correctly.
In `@samples/travel` planner agent/backend/booking_api/requirements.txt:
- Line 2: Update the pinned uvicorn version from 0.30.6 to 0.40.0 in
requirements and add an explicit h11 constraint (h11 >= 0.16.0) to avoid
resolving to vulnerable h11 releases; modify the requirements entry that
currently lists "uvicorn==0.30.6" so it becomes the newer uvicorn pin and
include a separate "h11>=0.16.0" line to ensure the dependency resolver selects
a safe h11.
In `@samples/travel` planner agent/frontend/src/index.css:
- Around line 3-5: Add configuration so Biome's CSS linter recognizes Tailwind
directives or exclude this file from linting: update the Biome config (e.g., in
biome.json) by adding "allowAtRules": ["tailwind"] under css.linter.rules, or
add an exclusion for samples/travel planner agent/frontend/src/index.css (or its
folder) in .biomerc/build config so the `@tailwind` base/components/utilities
directives in index.css are not reported as unknown at-rules.
In `@samples/travel` planner agent/frontend/src/pages/Home.tsx:
- Around line 365-389: The delete control is a span with role="button" nested
inside the session <button>, which is invalid and not keyboard-focusable; fix by
making the inner control a real <button> (or moving it out of the outer
<button>) so it can receive keyboard focus and activation. Update the
sessions.map JSX: replace the inner <span role="button" ...> used for delete
with a proper <button aria-label="Delete chat" ...> (or restructure so the outer
element is not a <button> and the delete remains a separate <button>), keep
event.stopPropagation() and handleDeleteSession(s.id) to avoid toggling
setActiveSessionId, and preserve classes like "tp-chat-session-delete" and icons
(Trash2) for styling.
In `@samples/travel` planner agent/README.md:
- Around line 16-53: Replace the bold section titles that trigger MD036 with
proper Markdown headings: convert "**Agent Manager**", "**Agent environment
variables**", "**Expose the agent endpoint after deploy**", "**Booking API**",
and "**Pinecone policies (required)**" into appropriate heading levels (e.g.,
"## Agent Manager", "## Agent environment variables" with "### Required" and
"### Optional" for the variable lists) and adjust other bolded labels (like
"Required:" / "Optional (defaults are applied if unset):") to subheadings so the
README.md uses semantic headings instead of bold text and resolves the MD036
lint warnings.
In `@samples/travel` planner agent/resources/ingest/ingest.py:
- Around line 80-84: The loop over PDF pages calls page.extract_text() which may
return None and cause a TypeError when concatenated into text; update the code
that builds text (the reader/pages loop using variables reader, page, text and
pdf_path) to guard against None by replacing concatenation with something like t
= page.extract_text() and then text += (t or "") or only append when t is not
None so extraction safely skips None pages.
- Around line 16-52: Validate chunk_size and chunk_overlap to prevent infinite
loops: in the constructor (__init__) or at the start of chunk_text(), ensure
chunk_size is > 0 and 0 <= chunk_overlap < chunk_size; if these conditions are
violated raise a ValueError with a clear message referencing chunk_size and
chunk_overlap. This guards the chunk_text() loop (which uses start = end -
self.chunk_overlap) from never advancing and fails fast with a descriptive error
instead of hanging.
In `@samples/travel` planner agent/resources/ingest/README.md:
- Around line 25-42: The README documents a SINGLE_PDF_* ingestion flow that
isn’t implemented in ingest.py; add support in ingest.py to read the
SINGLE_PDF_PATH env var (when set) and parse the PDF into per-hotel documents
using headings/regexes like "Hotel Name - Hotel Policies" or "Hotel:\s*Name",
falling back to SINGLE_PDF_DEFAULT_HOTEL when no name is found on a page, then
feed those per-hotel texts into the existing chunking logic (respecting
CHUNK_SIZE and CHUNK_OVERLAP) and the same indexing/ingest functions used for
POLICIES_DIRS; alternatively, if you prefer the minimal change, remove the
SINGLE_PDF_PATH and SINGLE_PDF_DEFAULT_HOTEL mentions from the README to avoid
documenting an unsupported flow.
🧹 Nitpick comments (6)
samples/travel planner agent/resources/ingest/.env.example (1)
1-8: Align.env.exampleformatting with dotenv-linter rules.The linter flags quoted values and key ordering. If this repo enforces dotenv-linter, consider unquoting values and reordering keys to prevent warnings.
♻️ Suggested formatting adjustment
-PINECONE_SERVICE_URL="https://your-index-xxxxxx.svc.your-region.pinecone.io" -PINECONE_API_KEY="your-pinecone-api-key" -PINECONE_INDEX_NAME="hotel-policies" -PINECONE_NAMESPACE="" -OPENAI_API_KEY="your-openai-api-key" -OPENAI_EMBEDDING_MODEL="text-embedding-3-small" -CHUNK_SIZE=1000 -CHUNK_OVERLAP=200 +CHUNK_OVERLAP=200 +CHUNK_SIZE=1000 +OPENAI_API_KEY=your-openai-api-key +OPENAI_EMBEDDING_MODEL=text-embedding-3-small +PINECONE_API_KEY=your-pinecone-api-key +PINECONE_INDEX_NAME=hotel-policies +PINECONE_NAMESPACE= +PINECONE_SERVICE_URL=https://your-index-xxxxxx.svc.your-region.pinecone.iosamples/travel planner agent/backend/agent/.env.example (1)
1-14: Expose all supported config knobs in.env.example.
SettingssupportsOPENAI_EMBEDDING_MODEL,CORS_ALLOW_ORIGINS, andCORS_ALLOW_CREDENTIALS, but they’re missing from the sample env file. Adding them improves discoverability and avoids hidden defaults.🧩 Suggested additions
OPENAI_API_KEY= OPENAI_MODEL= +OPENAI_EMBEDDING_MODEL= @@ WEATHER_API_KEY= WEATHER_API_BASE_URL= + +CORS_ALLOW_ORIGINS= +CORS_ALLOW_CREDENTIALS=samples/travel planner agent/frontend/src/config/auth.ts (1)
1-10: Move auth config to environment variables.Client ID, base URL, and redirect URLs are tenant/environment-specific. Hardcoding them risks deploying the wrong tenant configuration or localhost URLs to production. Use environment variables per environment (e.g.,
.env.development,.env.productionin Vite withVITE_ASGARDEO_CLIENT_ID,VITE_ASGARDEO_BASE_URL,VITE_ASGARDEO_SIGN_IN_REDIRECT_URL,VITE_ASGARDEO_SIGN_OUT_REDIRECT_URL) as documented in the Asgardeo SDK configuration guide.samples/travel planner agent/frontend/src/pages/Home.tsx (1)
91-96: Avoid hard-coded chat API URL.
CHAT_API_URLis fixed to localhost. Consider reading this from the app’s config/env so deployments don’t require code changes (keep localhost as a fallback).🔧 Example (adapt to your build setup)
-const CHAT_API_URL = "http://localhost:9090/chat"; +const CHAT_API_URL = + (import.meta as any)?.env?.VITE_CHAT_API_URL ?? "http://localhost:9090/chat";samples/travel planner agent/backend/agent/app.py (1)
32-40: Prefer settings-driven CORS defaults.You already load CORS settings in
Settings; using them here avoids permissive defaults in non-dev environments.♻️ Proposed update
-app.add_middleware( - CORSMiddleware, - allow_origins=["*"], - allow_credentials=False, - allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], - allow_headers=["Content-Type", "Authorization", "Accept", "x-user-id"], - max_age=84900, -) +app.add_middleware( + CORSMiddleware, + allow_origins=configs.cors_allow_origins, + allow_credentials=configs.cors_allow_credentials, + allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], + allow_headers=["Content-Type", "Authorization", "Accept", "x-user-id"], + max_age=84900, +)samples/travel planner agent/backend/agent/tools.py (1)
291-299: Defaultroom_countto 1 to match API optionality.♻️ Suggested default
- room_count: int, + room_count: int = 1,
| def _wrap_user_message(user_message: str, user_id: str, user_name: str | None) -> str: | ||
| now = datetime.now(timezone.utc).isoformat() | ||
| resolved_user_id = user_id | ||
| resolved_user_name = user_name or "Traveler" | ||
| return ( | ||
| f"User Name: {resolved_user_name}\n" | ||
| f"User Context (non-hotel identifiers): {resolved_user_name} ({resolved_user_id})\n" | ||
| f"UTC Time now:\n{now}\n\n" | ||
| f"User Query:\n{user_message}" | ||
| ) |
There was a problem hiding this comment.
Mask email-derived names before sending to the LLM.
user_name can fall back to the email claim; avoid embedding emails in prompts to reduce PII exposure.
🛡️ Suggested masking
- resolved_user_name = user_name or "Traveler"
+ resolved_user_name = user_name or "Traveler"
+ if "@" in resolved_user_name:
+ resolved_user_name = "Traveler"🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/agent/app.py around lines 43 - 52,
_user_name and user_id may contain full email addresses and should be masked
before embedding in prompts; update the _wrap_user_message function to detect
email patterns in user_name and user_id, replace or obfuscate the domain (e.g.,
keep local-part only or replace with "[redacted]" or asterisks) and use those
masked values (e.g., masked_user_name, masked_user_id) when composing the "User
Name" and "User Context" lines so no raw email is sent to the LLM.
| def _decode_access_token(token: str) -> dict[str, Any]: | ||
| return jwt.decode( | ||
| token, | ||
| options={ | ||
| "verify_signature": False, | ||
| "verify_aud": False, | ||
| "verify_iss": False, | ||
| "verify_exp": False, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the specific file mentioned
find . -name "app.py" -path "*/travel*" -type fRepository: wso2/agent-manager
Length of output: 113
🏁 Script executed:
# Check the file structure around the mentioned lines
cat -n "samples/travel planner agent/backend/agent/app.py" | sed -n '65,85p'Repository: wso2/agent-manager
Length of output: 726
🏁 Script executed:
# Search for JWT-related imports and usage in the backend
rg -i "jwt|jwks|decode|token" "samples/travel planner agent/backend" -A 2 -B 2 --max-count=50Repository: wso2/agent-manager
Length of output: 8768
🏁 Script executed:
# Look for configuration files that might contain IdP settings
find "samples/travel planner agent/backend" -type f \( -name "*.py" -o -name "*.env*" -o -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) | head -20Repository: wso2/agent-manager
Length of output: 711
🌐 Web query:
PyJWT best practices for JWT verification with JWKS (PyJWKClient) including issuer and audience validation
💡 Result:
Best practices (PyJWT + JWKS via PyJWKClient)
-
Pin the expected algorithms (don’t trust
algin the JWT header).
Pass a fixed allow-list likealgorithms=["RS256"](or whatever your IdP uses) rather than deriving it from token content. PyJWT explicitly warns against attacker-influenced algorithm selection. [2] -
Always validate
issandaud(and pass the expected values explicitly).
Usejwt.decode(..., issuer=EXPECTED_ISSUER, audience=EXPECTED_AUDIENCE, ...)so PyJWT checks theissandaudclaims. [2] -
Require time-bound claims and verify them.
By default, claims likeexp/nbf/iatare only validated if present; if you need them to always exist, enforce them withoptions={"require": ["exp", ...]}. [2] -
Fetch the signing key from JWKS using
kid, then verify with that key.
Typical flow:jwks_client.get_signing_key_from_jwt(token)→jwt.decode(token, signing_key.key, ...). [1] -
Enable JWKS caching (and size it for your rotation).
PyJWKClientsupports JWKS-set caching (cache_jwk_set,lifespan) and optional per-key caching (cache_keys,max_cached_keys) to reduce HTTP calls and improve resilience. [3] -
Fail closed on verification errors.
Treat any decode / key-fetch failure as an invalid token (don’t fall back to “unverified” claims).
Reference implementation
import jwt
from jwt import PyJWKClient
JWKS_URL = "https://issuer.example.com/.well-known/jwks.json"
EXPECTED_ISSUER = "https://issuer.example.com/"
EXPECTED_AUDIENCE = "api://my-service"
ALGORITHMS = ["RS256"]
jwks_client = PyJWKClient(
JWKS_URL,
cache_jwk_set=True,
lifespan=300, # tune to your key-rotation + availability needs
cache_keys=True,
max_cached_keys=16,
)
def verify_jwt(token: str) -> dict:
signing_key = jwks_client.get_signing_key_from_jwt(token)
return jwt.decode(
token,
signing_key.key,
algorithms=ALGORITHMS,
issuer=EXPECTED_ISSUER,
audience=EXPECTED_AUDIENCE,
options={
"require": ["exp", "iss", "aud"],
# optionally: "require": ["exp", "iat", "nbf", "iss", "aud", "sub"]
},
leeway=30, # optional clock skew
)Notes:
get_signing_key_from_jwt()is the standard PyJWT approach for selecting the right JWKS key for the token. [1]issuer,audience,options, and the algorithm pinning are part of PyJWT’s verification API and guidance. [2]- Caching knobs shown (
cache_jwk_set,lifespan,cache_keys,max_cached_keys) come from thePyJWKClientimplementation. [3]
Sources: PyJWT usage examples for JWKS / OIDC flow [1], PyJWT API reference for decode() verification + algorithm warning [2], PyJWKClient implementation showing caching parameters [3].
🏁 Script executed:
# Check the config file for IdP settings
cat -n "samples/travel planner agent/backend/agent/config.py"Repository: wso2/agent-manager
Length of output: 2598
🏁 Script executed:
# Search for any other JWT validation or auth middleware in the codebase
rg -i "verify|jwks|issuer|audience|keycloak|asgardeo|idp" "samples/travel planner agent/backend" --type pyRepository: wso2/agent-manager
Length of output: 990
🏁 Script executed:
# Check README or documentation to understand if this is a demo
find "samples/travel planner agent" -type f \( -name "README*" -o -name "*.md" \) -exec head -30 {} +Repository: wso2/agent-manager
Length of output: 2870
JWT validation is fully disabled (auth bypass).
All signature/issuer/audience/expiry checks are disabled, so any token with a valid JSON structure and "sub" claim is accepted as valid. The required Asgardeo configuration (ASGARDEO_BASE_URL, ASGARDEO_CLIENT_ID) is present but unused for validation. Validate using the IdP JWKS endpoint with issuer/audience/expiry enforcement.
🔐 Example fix (adjust issuer/JWKS for your IdP)
-import jwt
+import jwt
+from jwt import PyJWKClient
+from agent.config import configs
@@
def _decode_access_token(token: str) -> dict[str, Any]:
- return jwt.decode(
- token,
- options={
- "verify_signature": False,
- "verify_aud": False,
- "verify_iss": False,
- "verify_exp": False,
- },
- )
+ jwks_client = PyJWKClient(f"{configs.asgardeo_base_url.rstrip('/')}/oauth2/jwks")
+ signing_key = jwks_client.get_signing_key_from_jwt(token)
+ return jwt.decode(
+ token,
+ signing_key.key,
+ algorithms=["RS256"],
+ audience=configs.asgardeo_client_id,
+ issuer=configs.asgardeo_base_url,
+ options={"verify_exp": True},
+ )📝 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.
| def _decode_access_token(token: str) -> dict[str, Any]: | |
| return jwt.decode( | |
| token, | |
| options={ | |
| "verify_signature": False, | |
| "verify_aud": False, | |
| "verify_iss": False, | |
| "verify_exp": False, | |
| }, | |
| ) | |
| from jwt import PyJWKClient | |
| from agent.config import configs | |
| def _decode_access_token(token: str) -> dict[str, Any]: | |
| jwks_client = PyJWKClient(f"{configs.asgardeo_base_url.rstrip('/')}/oauth2/jwks") | |
| signing_key = jwks_client.get_signing_key_from_jwt(token) | |
| return jwt.decode( | |
| token, | |
| signing_key.key, | |
| algorithms=["RS256"], | |
| audience=configs.asgardeo_client_id, | |
| issuer=configs.asgardeo_base_url, | |
| options={"verify_exp": True}, | |
| ) |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/agent/app.py around lines 71 - 80, The
_decode_access_token function currently disables all JWT checks; replace this by
fetching the IdP JWKS (use ASGARDEO_BASE_URL to build the /.well-known/jwks.json
URL) and validate signature, issuer and audience/expiry using the proper key via
jwt.PyJWKClient (or equivalent), passing ASGARDEO_CLIENT_ID as the expected
audience and ASGARDEO_BASE_URL (or the IdP issuer) as the expected issuer;
ensure jwt.decode raises on invalid tokens and propagate/handle errors
accordingly in _decode_access_token so only tokens with valid
signature/iss/aud/exp are accepted.
| pinecone_service_url=required("PINECONE_SERVICE_URL"), | ||
| pinecone_index_name=os.getenv("PINECONE_INDEX_NAME", "hotel-policies"), | ||
| weather_api_key=os.getenv("WEATHER_API_KEY"), | ||
| weather_api_base_url=os.getenv("WEATHER_API_BASE_URL", "http://api.weatherapi.com/v1"), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WeatherAPI official base URL HTTPS endpoint documentation
💡 Result:
WeatherAPI.com’s official API base URL is:
https://api.weatherapi.com/v1(same host/path as the docs’ base URL; HTTPS is supported) [1], [2]
Example HTTPS request:
GET https://api.weatherapi.com/v1/current.json?key=YOUR_API_KEY&q=London[1]
Sources:
[1] WeatherAPI.com Docs (Request / Base URL)
[2] weatherapicom/go package docs (All URIs relative to https://api.weatherapi.com/v1)
Use HTTPS for the Weather API default base URL.
The default uses HTTP, which exposes API keys over plaintext. WeatherAPI's official endpoint is https://api.weatherapi.com/v1 and should be used as the default.
🔧 Proposed fix
- weather_api_base_url=os.getenv("WEATHER_API_BASE_URL", "http://api.weatherapi.com/v1"),
+ weather_api_base_url=os.getenv("WEATHER_API_BASE_URL", "https://api.weatherapi.com/v1"),📝 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.
| weather_api_base_url=os.getenv("WEATHER_API_BASE_URL", "http://api.weatherapi.com/v1"), | |
| weather_api_base_url=os.getenv("WEATHER_API_BASE_URL", "https://api.weatherapi.com/v1"), |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/agent/config.py at line 49, The default
Weather API base URL currently uses plaintext HTTP; update the default for
weather_api_base_url (the os.getenv call supplying "WEATHER_API_BASE_URL") to
use the secure HTTPS endpoint "https://api.weatherapi.com/v1" so API keys and
traffic are encrypted by default.
| @tool(args_schema=BookingRequest) | ||
| def create_booking_tool( | ||
| userId: str, | ||
| hotelId: str, | ||
| rooms: list[RoomConfiguration], | ||
| checkInDate: str, | ||
| checkOutDate: str, | ||
| numberOfGuests: int, | ||
| numberOfRooms: int, | ||
| primaryGuest: GuestDetails, | ||
| specialRequests: SpecialRequests | None = None, | ||
| hotelName: str | None = None, | ||
| ) -> dict[str, Any]: | ||
| """Create a booking via the booking API.""" | ||
| resolved_user_id = userId | ||
| logger.info( | ||
| "create_booking_tool called: user_id=%s hotel_id=%s check_in_date=%s check_out_date=%s number_of_rooms=%s", | ||
| resolved_user_id, | ||
| hotelId, | ||
| checkInDate, | ||
| checkOutDate, | ||
| numberOfRooms, | ||
| ) | ||
| payload = { | ||
| "userId": resolved_user_id, | ||
| "hotelId": hotelId, | ||
| "hotelName": hotelName, | ||
| "rooms": [room.model_dump() for room in rooms], | ||
| "checkInDate": checkInDate, | ||
| "checkOutDate": checkOutDate, | ||
| "numberOfGuests": numberOfGuests, | ||
| "numberOfRooms": numberOfRooms, | ||
| "primaryGuest": primaryGuest.model_dump(), | ||
| "specialRequests": specialRequests.model_dump() if specialRequests else None, | ||
| } | ||
| endpoint = f"{settings.booking_api_base_url.rstrip('/')}/bookings" | ||
| try: | ||
| response = requests.post(endpoint, json=payload, timeout=30) | ||
| response.raise_for_status() | ||
| except requests.RequestException: | ||
| logger.exception("create_booking_tool failed calling booking API") | ||
| return {"error": "Booking API request failed."} | ||
| return response.json() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the file structure and find related booking tools
find . -type f -name "*.py" | grep -E "(tool|booking)" | head -20Repository: wso2/agent-manager
Length of output: 556
🏁 Script executed:
# Search for other booking tool implementations in the same file
rg -n "def.*booking.*tool" samples/travel\ planner\ agent/backend/agent/tools.pyRepository: wso2/agent-manager
Length of output: 323
🏁 Script executed:
# Search for x-user-id usage in the codebase
rg -n "x-user-id" --type pyRepository: wso2/agent-manager
Length of output: 605
🏁 Script executed:
# Look for booking API related code
rg -n "booking.*api\|booking_api" --type py -iRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
# Check the entire tools.py file to understand context
wc -l samples/travel\ planner\ agent/backend/agent/tools.pyRepository: wso2/agent-manager
Length of output: 117
🏁 Script executed:
# View the other booking tool implementations to see if they send x-user-id
sed -n '374,421p' samples/travel\ planner\ agent/backend/agent/tools.pyRepository: wso2/agent-manager
Length of output: 2126
🏁 Script executed:
# View cancel and list booking tools
sed -n '422,455p' samples/travel\ planner\ agent/backend/agent/tools.pyRepository: wso2/agent-manager
Length of output: 1694
🏁 Script executed:
# View booking_api.py to see how it uses x-user-id
sed -n '50,75p' samples/travel\ planner\ agent/backend/booking_api/booking_api.pyRepository: wso2/agent-manager
Length of output: 833
🏁 Script executed:
# Search for any error handling around response.json() in the codebase
rg -n "response\.json\(\)" -A 3 --type py samples/travel\ planner\ agent/Repository: wso2/agent-manager
Length of output: 2041
Add x-user-id header and guard non‑JSON responses for consistency.
The other three booking tools (edit_booking_tool, cancel_booking_tool, list_bookings_tool) all send the x-user-id header, and the booking API explicitly reads it via request.headers.get("x-user-id") for user attribution. create_booking_tool is missing this header, creating an inconsistency. Additionally, response.json() can raise ValueError on non-JSON responses; the codebase already handles this pattern elsewhere (see lines 119-121) but create_booking_tool, edit_booking_tool, cancel_booking_tool, and list_bookings_tool do not.
✅ Safer request handling
- try:
- response = requests.post(endpoint, json=payload, timeout=30)
+ headers = {"x-user-id": resolved_user_id} if resolved_user_id else {}
+ try:
+ response = requests.post(endpoint, json=payload, headers=headers, timeout=30)
response.raise_for_status()
except requests.RequestException:
logger.exception("create_booking_tool failed calling booking API")
return {"error": "Booking API request failed."}
- return response.json()
+ try:
+ return response.json()
+ except ValueError:
+ return {"error": "Booking API returned non-JSON response."}📝 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.
| @tool(args_schema=BookingRequest) | |
| def create_booking_tool( | |
| userId: str, | |
| hotelId: str, | |
| rooms: list[RoomConfiguration], | |
| checkInDate: str, | |
| checkOutDate: str, | |
| numberOfGuests: int, | |
| numberOfRooms: int, | |
| primaryGuest: GuestDetails, | |
| specialRequests: SpecialRequests | None = None, | |
| hotelName: str | None = None, | |
| ) -> dict[str, Any]: | |
| """Create a booking via the booking API.""" | |
| resolved_user_id = userId | |
| logger.info( | |
| "create_booking_tool called: user_id=%s hotel_id=%s check_in_date=%s check_out_date=%s number_of_rooms=%s", | |
| resolved_user_id, | |
| hotelId, | |
| checkInDate, | |
| checkOutDate, | |
| numberOfRooms, | |
| ) | |
| payload = { | |
| "userId": resolved_user_id, | |
| "hotelId": hotelId, | |
| "hotelName": hotelName, | |
| "rooms": [room.model_dump() for room in rooms], | |
| "checkInDate": checkInDate, | |
| "checkOutDate": checkOutDate, | |
| "numberOfGuests": numberOfGuests, | |
| "numberOfRooms": numberOfRooms, | |
| "primaryGuest": primaryGuest.model_dump(), | |
| "specialRequests": specialRequests.model_dump() if specialRequests else None, | |
| } | |
| endpoint = f"{settings.booking_api_base_url.rstrip('/')}/bookings" | |
| try: | |
| response = requests.post(endpoint, json=payload, timeout=30) | |
| response.raise_for_status() | |
| except requests.RequestException: | |
| logger.exception("create_booking_tool failed calling booking API") | |
| return {"error": "Booking API request failed."} | |
| return response.json() | |
| `@tool`(args_schema=BookingRequest) | |
| def create_booking_tool( | |
| userId: str, | |
| hotelId: str, | |
| rooms: list[RoomConfiguration], | |
| checkInDate: str, | |
| checkOutDate: str, | |
| numberOfGuests: int, | |
| numberOfRooms: int, | |
| primaryGuest: GuestDetails, | |
| specialRequests: SpecialRequests | None = None, | |
| hotelName: str | None = None, | |
| ) -> dict[str, Any]: | |
| """Create a booking via the booking API.""" | |
| resolved_user_id = userId | |
| logger.info( | |
| "create_booking_tool called: user_id=%s hotel_id=%s check_in_date=%s check_out_date=%s number_of_rooms=%s", | |
| resolved_user_id, | |
| hotelId, | |
| checkInDate, | |
| checkOutDate, | |
| numberOfRooms, | |
| ) | |
| payload = { | |
| "userId": resolved_user_id, | |
| "hotelId": hotelId, | |
| "hotelName": hotelName, | |
| "rooms": [room.model_dump() for room in rooms], | |
| "checkInDate": checkInDate, | |
| "checkOutDate": checkOutDate, | |
| "numberOfGuests": numberOfGuests, | |
| "numberOfRooms": numberOfRooms, | |
| "primaryGuest": primaryGuest.model_dump(), | |
| "specialRequests": specialRequests.model_dump() if specialRequests else None, | |
| } | |
| endpoint = f"{settings.booking_api_base_url.rstrip('/')}/bookings" | |
| headers = {"x-user-id": resolved_user_id} if resolved_user_id else {} | |
| try: | |
| response = requests.post(endpoint, json=payload, headers=headers, timeout=30) | |
| response.raise_for_status() | |
| except requests.RequestException: | |
| logger.exception("create_booking_tool failed calling booking API") | |
| return {"error": "Booking API request failed."} | |
| try: | |
| return response.json() | |
| except ValueError: | |
| return {"error": "Booking API returned non-JSON response."} |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/backend/agent/tools.py around lines 329 - 371,
create_booking_tool is missing the x-user-id header and does not guard against
non-JSON responses; update the function to add a headers dict including
"x-user-id": resolved_user_id (use the same resolved_user_id variable) when
calling requests.post, and wrap the final response.json() call in a try/except
catching ValueError (or JSONDecodeError) to return a consistent {"error": "..."}
payload on non-JSON responses (mirror the error handling pattern used in the
other booking tools such as
edit_booking_tool/cancel_booking_tool/list_bookings_tool).
| **Agent Manager** | ||
| - Repo URL: `https://github.com/wso2/agent-manager/tree/amp/v0/samples/travel_planner_agent` | ||
| - Language/runtime: Python 3.11 | ||
| - Run command: `uvicorn app:app --host 0.0.0.0 --port 9090` | ||
| - Agent type: Chat API Agent | ||
| - Schema path: `openapi.yaml` | ||
| - Port: `9090` | ||
|
|
||
| **Agent environment variables** | ||
| Required: | ||
| - `OPENAI_API_KEY` | ||
| - `ASGARDEO_BASE_URL` | ||
| - `ASGARDEO_CLIENT_ID` | ||
| - `PINECONE_API_KEY` | ||
| - `PINECONE_SERVICE_URL` | ||
|
|
||
| Optional (defaults are applied if unset): | ||
| - `OPENAI_MODEL` (default: `gpt-4o-mini`) | ||
| - `OPENAI_EMBEDDING_MODEL` (default: `text-embedding-3-small`) | ||
| - `WEATHER_API_KEY` | ||
| - `WEATHER_API_BASE_URL` (default: `http://api.weatherapi.com/v1`) | ||
| - `BOOKING_API_BASE_URL` (default: `http://localhost:9091`) | ||
|
|
||
| **Expose the agent endpoint after deploy** | ||
| Run this inside the WSO2-AMP dev container to expose the agent on `localhost:9090`: | ||
|
|
||
| ```bash | ||
| kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80 | ||
| ``` | ||
|
|
||
| **Booking API** | ||
| - Runs locally on `http://localhost:9091` when started via `uvicorn`. | ||
| - You can also deploy it to a cloud host; just point the agent configuration at the deployed base URL. | ||
|
|
||
| **Pinecone policies (required)** | ||
| - Create a Pinecone index using your preferred embedding model. | ||
| - Set the Pinecone and embedding configuration in `resources/ingest/.env`. | ||
| - Run the ingest to populate the index. |
There was a problem hiding this comment.
Use headings instead of bold text for section titles.
Markdownlint flags MD036 at Line 16/46/50; consider converting the bold labels into proper headings for structure.
📝 Example adjustment
-**Agent Manager**
+### Agent Manager
-**Booking API**
+### Booking API
-**Pinecone policies (required)**
+### Pinecone policies (required)📝 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.
| **Agent Manager** | |
| - Repo URL: `https://github.com/wso2/agent-manager/tree/amp/v0/samples/travel_planner_agent` | |
| - Language/runtime: Python 3.11 | |
| - Run command: `uvicorn app:app --host 0.0.0.0 --port 9090` | |
| - Agent type: Chat API Agent | |
| - Schema path: `openapi.yaml` | |
| - Port: `9090` | |
| **Agent environment variables** | |
| Required: | |
| - `OPENAI_API_KEY` | |
| - `ASGARDEO_BASE_URL` | |
| - `ASGARDEO_CLIENT_ID` | |
| - `PINECONE_API_KEY` | |
| - `PINECONE_SERVICE_URL` | |
| Optional (defaults are applied if unset): | |
| - `OPENAI_MODEL` (default: `gpt-4o-mini`) | |
| - `OPENAI_EMBEDDING_MODEL` (default: `text-embedding-3-small`) | |
| - `WEATHER_API_KEY` | |
| - `WEATHER_API_BASE_URL` (default: `http://api.weatherapi.com/v1`) | |
| - `BOOKING_API_BASE_URL` (default: `http://localhost:9091`) | |
| **Expose the agent endpoint after deploy** | |
| Run this inside the WSO2-AMP dev container to expose the agent on `localhost:9090`: | |
| ```bash | |
| kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80 | |
| ``` | |
| **Booking API** | |
| - Runs locally on `http://localhost:9091` when started via `uvicorn`. | |
| - You can also deploy it to a cloud host; just point the agent configuration at the deployed base URL. | |
| **Pinecone policies (required)** | |
| - Create a Pinecone index using your preferred embedding model. | |
| - Set the Pinecone and embedding configuration in `resources/ingest/.env`. | |
| - Run the ingest to populate the index. | |
| ### Agent Manager | |
| - Repo URL: `https://github.com/wso2/agent-manager/tree/amp/v0/samples/travel_planner_agent` | |
| - Language/runtime: Python 3.11 | |
| - Run command: `uvicorn app:app --host 0.0.0.0 --port 9090` | |
| - Agent type: Chat API Agent | |
| - Schema path: `openapi.yaml` | |
| - Port: `9090` | |
| **Agent environment variables** | |
| Required: | |
| - `OPENAI_API_KEY` | |
| - `ASGARDEO_BASE_URL` | |
| - `ASGARDEO_CLIENT_ID` | |
| - `PINECONE_API_KEY` | |
| - `PINECONE_SERVICE_URL` | |
| Optional (defaults are applied if unset): | |
| - `OPENAI_MODEL` (default: `gpt-4o-mini`) | |
| - `OPENAI_EMBEDDING_MODEL` (default: `text-embedding-3-small`) | |
| - `WEATHER_API_KEY` | |
| - `WEATHER_API_BASE_URL` (default: `http://api.weatherapi.com/v1`) | |
| - `BOOKING_API_BASE_URL` (default: `http://localhost:9091`) | |
| **Expose the agent endpoint after deploy** | |
| Run this inside the WSO2-AMP dev container to expose the agent on `localhost:9090`: | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 16-16: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
[warning] 46-46: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
[warning] 50-50: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/README.md around lines 16 - 53, Replace the
bold section titles that trigger MD036 with proper Markdown headings: convert
"**Agent Manager**", "**Agent environment variables**", "**Expose the agent
endpoint after deploy**", "**Booking API**", and "**Pinecone policies
(required)**" into appropriate heading levels (e.g., "## Agent Manager", "##
Agent environment variables" with "### Required" and "### Optional" for the
variable lists) and adjust other bolded labels (like "Required:" / "Optional
(defaults are applied if unset):") to subheadings so the README.md uses semantic
headings instead of bold text and resolves the MD036 lint warnings.
| def __init__(self, chunk_size: int = 1000, chunk_overlap: int = 200): | ||
| self.chunk_size = chunk_size | ||
| self.chunk_overlap = chunk_overlap | ||
|
|
||
| def chunk_text(self, text: str, metadata: Dict[str, Any]) -> List[Dict[str, Any]]: | ||
| """ | ||
| Recursively chunk text into smaller segments with overlap | ||
| """ | ||
| chunks = [] | ||
| text_length = len(text) | ||
|
|
||
| if text_length == 0: | ||
| return chunks | ||
|
|
||
| start = 0 | ||
| chunk_id = 0 | ||
|
|
||
| while start < text_length: | ||
| end = start + self.chunk_size | ||
| chunk_text = text[start:end] | ||
|
|
||
| chunks.append({ | ||
| "content": chunk_text.strip(), | ||
| "metadata": { | ||
| **metadata, | ||
| "chunk_id": chunk_id, | ||
| "start_index": start, | ||
| "end_index": min(end, text_length) | ||
| } | ||
| }) | ||
|
|
||
| chunk_id += 1 | ||
| start = end - self.chunk_overlap | ||
|
|
||
| # Prevent infinite loop | ||
| if start >= text_length: | ||
| break |
There was a problem hiding this comment.
Guard against invalid chunk overlap to prevent infinite loops.
If chunk_overlap >= chunk_size, start never advances and the chunking loop can hang. Validate inputs and fail fast.
🛡️ Proposed validation
def __init__(self, chunk_size: int = 1000, chunk_overlap: int = 200):
- self.chunk_size = chunk_size
- self.chunk_overlap = chunk_overlap
+ if chunk_size <= 0:
+ raise ValueError("chunk_size must be > 0")
+ if chunk_overlap < 0 or chunk_overlap >= chunk_size:
+ raise ValueError("chunk_overlap must be >= 0 and < chunk_size")
+ self.chunk_size = chunk_size
+ self.chunk_overlap = chunk_overlap📝 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.
| def __init__(self, chunk_size: int = 1000, chunk_overlap: int = 200): | |
| self.chunk_size = chunk_size | |
| self.chunk_overlap = chunk_overlap | |
| def chunk_text(self, text: str, metadata: Dict[str, Any]) -> List[Dict[str, Any]]: | |
| """ | |
| Recursively chunk text into smaller segments with overlap | |
| """ | |
| chunks = [] | |
| text_length = len(text) | |
| if text_length == 0: | |
| return chunks | |
| start = 0 | |
| chunk_id = 0 | |
| while start < text_length: | |
| end = start + self.chunk_size | |
| chunk_text = text[start:end] | |
| chunks.append({ | |
| "content": chunk_text.strip(), | |
| "metadata": { | |
| **metadata, | |
| "chunk_id": chunk_id, | |
| "start_index": start, | |
| "end_index": min(end, text_length) | |
| } | |
| }) | |
| chunk_id += 1 | |
| start = end - self.chunk_overlap | |
| # Prevent infinite loop | |
| if start >= text_length: | |
| break | |
| def __init__(self, chunk_size: int = 1000, chunk_overlap: int = 200): | |
| if chunk_size <= 0: | |
| raise ValueError("chunk_size must be > 0") | |
| if chunk_overlap < 0 or chunk_overlap >= chunk_size: | |
| raise ValueError("chunk_overlap must be >= 0 and < chunk_size") | |
| self.chunk_size = chunk_size | |
| self.chunk_overlap = chunk_overlap | |
| def chunk_text(self, text: str, metadata: Dict[str, Any]) -> List[Dict[str, Any]]: | |
| """ | |
| Recursively chunk text into smaller segments with overlap | |
| """ | |
| chunks = [] | |
| text_length = len(text) | |
| if text_length == 0: | |
| return chunks | |
| start = 0 | |
| chunk_id = 0 | |
| while start < text_length: | |
| end = start + self.chunk_size | |
| chunk_text = text[start:end] | |
| chunks.append({ | |
| "content": chunk_text.strip(), | |
| "metadata": { | |
| **metadata, | |
| "chunk_id": chunk_id, | |
| "start_index": start, | |
| "end_index": min(end, text_length) | |
| } | |
| }) | |
| chunk_id += 1 | |
| start = end - self.chunk_overlap | |
| # Prevent infinite loop | |
| if start >= text_length: | |
| break |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/resources/ingest/ingest.py around lines 16 -
52, Validate chunk_size and chunk_overlap to prevent infinite loops: in the
constructor (__init__) or at the start of chunk_text(), ensure chunk_size is > 0
and 0 <= chunk_overlap < chunk_size; if these conditions are violated raise a
ValueError with a clear message referencing chunk_size and chunk_overlap. This
guards the chunk_text() loop (which uses start = end - self.chunk_overlap) from
never advancing and fails fast with a descriptive error instead of hanging.
| reader = PdfReader(pdf_path) | ||
| text = "" | ||
| for page in reader.pages: | ||
| text += page.extract_text() + "\n" | ||
| return text |
There was a problem hiding this comment.
Handle None from extract_text() to avoid TypeError.
page.extract_text() can return None. Concatenating None into a string raises at runtime, causing ingestion to fail on some PDFs.
🔧 Proposed fix
- for page in reader.pages:
- text += page.extract_text() + "\n"
+ for page in reader.pages:
+ page_text = page.extract_text() or ""
+ text += page_text + "\n"📝 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.
| reader = PdfReader(pdf_path) | |
| text = "" | |
| for page in reader.pages: | |
| text += page.extract_text() + "\n" | |
| return text | |
| reader = PdfReader(pdf_path) | |
| text = "" | |
| for page in reader.pages: | |
| page_text = page.extract_text() or "" | |
| text += page_text + "\n" | |
| return text |
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/resources/ingest/ingest.py around lines 80 -
84, The loop over PDF pages calls page.extract_text() which may return None and
cause a TypeError when concatenated into text; update the code that builds text
(the reader/pages loop using variables reader, page, text and pdf_path) to guard
against None by replacing concatenation with something like t =
page.extract_text() and then text += (t or "") or only append when t is not None
so extraction safely skips None pages.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@samples/travel` planner agent/README.md:
- Line 14: Replace the placeholder text "(details to be added)" in the README
sentence "Deploy the agent in your Agent Manager environment (details to be
added)." with concrete deployment steps: list prerequisites (Agent Manager
version, cloud/container requirements), required supporting services (auth,
database, vector db, message queues) and their minimum configurations,
configuration steps (how to register the agent, required environment variables
and secrets), example CLI/Helm/Docker commands to deploy the agent, and a short
troubleshooting checklist and verification steps to confirm the agent is
running; ensure the instructions are concise, include example command snippets
and sample values, and label each subsection clearly (Prerequisites, Services,
Configuration, Deployment, Verification).
- Line 17: Update the broken Repo URL in the README entry that currently points
to
`https://github.com/wso2/agent-manager/tree/amp/v0/samples/travel_planner_agent`;
change it to point to the correct branch and path (e.g.,
`https://github.com/wso2/agent-manager/tree/main/samples/travel%20planner%20agent`)
so it uses the `main` branch and encodes the space in "travel planner agent" (or
substitute the correct release branch/tag if this doc targets a specific
release).
🧹 Nitpick comments (3)
samples/travel planner agent/README.md (3)
43-43: Clarify the kubectl namespace value.The namespace
dp-default-default-default-ccb66d74appears to be environment-specific or a placeholder. Consider adding a note that users should replace this with their actual Agent Manager namespace, or provide instructions for discovering the correct namespace.📝 Suggested clarification
-kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80 +# Replace <namespace> with your actual Agent Manager namespace +kubectl -n <namespace> port-forward svc/travel-planner-agent-is 9090:80Or add a note explaining how to find the namespace:
# Find your agent's namespace kubectl get namespaces | grep dp-default # Then use it in the port-forward command kubectl -n <your-namespace> port-forward svc/travel-planner-agent-is 9090:80
66-66: Minor phrasing improvement for clarity.The current phrasing "The frontend runs on
http://localhost:3000, then:" could be clearer.📝 Suggested rewording
-The frontend runs on `http://localhost:3000`, then: +The frontend will run on `http://localhost:3000`:or
-The frontend runs on `http://localhost:3000`, then: +To start the frontend (it will run on `http://localhost:3000`):
77-77: Consider formatting the environment variables list for readability.The long list of environment variables on a single line is harder to scan. Consider formatting it as a bulleted list.
📋 Suggested formatting
-`PINECONE_SERVICE_URL`, `PINECONE_API_KEY`, `PINECONE_INDEX_NAME`, `OPENAI_API_KEY`, `OPENAI_EMBEDDING_MODEL`, and optional chunk settings. +Required variables: +- `PINECONE_SERVICE_URL` +- `PINECONE_API_KEY` +- `PINECONE_INDEX_NAME` +- `OPENAI_API_KEY` +- `OPENAI_EMBEDDING_MODEL` +- Optional chunk settings as needed
| Deploy the agent in your Agent Manager environment (details to be added). The flow below covers the required supporting services: | ||
|
|
||
| **Agent Manager** | ||
| - Repo URL: `https://github.com/wso2/agent-manager/tree/amp/v0/samples/travel_planner_agent` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if amp/v0 branch exists and find path variations
# List all branches to check for amp/v0
echo "=== Checking for amp/v0 branch ==="
git branch -a | grep -i amp || echo "No amp branches found"
# Check for references to travel_planner_agent vs "travel planner agent"
echo -e "\n=== Searching for travel planner/agent references in markdown files ==="
rg -i "travel.?planner.?agent" --type md -C2
# Also check the actual file in question
echo -e "\n=== Content of the file in question ==="
cat -n "samples/travel planner agent/README.md" | head -30
# Check if external URL is valid by examining context
echo -e "\n=== Checking git history for the URL ==="
rg "wso2/agent-manager" -C2Repository: wso2/agent-manager
Length of output: 2791
🏁 Script executed:
#!/bin/bash
# Check for amp/v0 as a tag or other reference
echo "=== Checking for amp/v0 tag ==="
git tag | grep -i amp || echo "No amp tags found"
echo -e "\n=== Listing all tags ==="
git tag | head -20
echo -e "\n=== Checking current branch/default branch ==="
git symbolic-ref refs/remotes/origin/HEAD || echo "No symbolic ref"
git branch -a | grep "\*"
echo -e "\n=== Check remote branches ==="
git branch -r | head -10Repository: wso2/agent-manager
Length of output: 316
Fix the broken repository URL in the deployment documentation.
The URL references a non-existent branch amp/v0 and uses underscores (travel_planner_agent) instead of spaces. The repository has no amp/v0 branch or tag; the default branch is main. Update the URL to: https://github.com/wso2/agent-manager/tree/main/samples/travel%20planner%20agent (or use the appropriate branch/tag if this documentation targets a specific release).
🤖 Prompt for AI Agents
In `@samples/travel` planner agent/README.md at line 17, Update the broken Repo
URL in the README entry that currently points to
`https://github.com/wso2/agent-manager/tree/amp/v0/samples/travel_planner_agent`;
change it to point to the correct branch and path (e.g.,
`https://github.com/wso2/agent-manager/tree/main/samples/travel%20planner%20agent`)
so it uses the `main` branch and encodes the space in "travel planner agent" (or
substitute the correct release branch/tag if this doc targets a specific
release).
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@samples/hotel-booking-agent/frontend/src/components/ui/progress.tsx`:
- Around line 11-23: The Progress component destructures `value` but doesn't
pass it to ProgressPrimitive.Root and doesn't normalize it, breaking ARIA
semantics and allowing indicator overflow; fix by clamping/normalizing the
incoming `value` to the 0–100 range (e.g., Math.min(Math.max(Number(value) || 0,
0), 100)), pass this normalized value into ProgressPrimitive.Root as the `value`
prop, and use that same normalized value for the Indicator transform calculation
(reference ProgressPrimitive.Root, ProgressPrimitive.Indicator, and the `value`
prop).
🟡 Minor comments (21)
samples/hotel-booking-agent/frontend/src/components/ui/chart.tsx-241-245 (1)
241-245:⚠️ Potential issue | 🟡 MinorRender zero values in tooltips.
item.value &&uses a truthy check, filtering out zero values. Use explicit null/undefined checks instead to preserve zeros in chart data.✅ Suggested fix
- {item.value && ( + {item.value !== undefined && item.value !== null && ( <span className="font-mono font-medium tabular-nums text-foreground"> {item.value.toLocaleString()} </span> )}samples/hotel-booking-agent/resources/ingest/.env.example-1-8 (1)
1-8:⚠️ Potential issue | 🟡 MinorAlign
.env.examplewith dotenv-linter (quotes + ordering).Line 1–8: The linter flags quoted values and key ordering. If CI enforces this, it will fail. Consider removing quotes and ordering keys to satisfy the rule.
🛠️ Suggested diff
-PINECONE_SERVICE_URL="https://your-index-xxxxxx.svc.your-region.pinecone.io" -PINECONE_API_KEY="your-pinecone-api-key" -PINECONE_INDEX_NAME="hotel-policies" -PINECONE_NAMESPACE="" -OPENAI_API_KEY="your-openai-api-key" -OPENAI_EMBEDDING_MODEL="text-embedding-3-small" -CHUNK_SIZE=1000 -CHUNK_OVERLAP=200 +CHUNK_OVERLAP=200 +CHUNK_SIZE=1000 +OPENAI_API_KEY=your-openai-api-key +OPENAI_EMBEDDING_MODEL=text-embedding-3-small +PINECONE_API_KEY=your-pinecone-api-key +PINECONE_INDEX_NAME=hotel-policies +PINECONE_NAMESPACE= +PINECONE_SERVICE_URL=https://your-index-xxxxxx.svc.your-region.pinecone.iosamples/hotel-booking-agent/frontend/src/components/ui/alert.tsx-47-56 (1)
47-56:⚠️ Potential issue | 🟡 MinorRef type mismatch: typed as
HTMLParagraphElementbut renders<div>.The component renders a
<div>element, but the ref is typed asHTMLParagraphElement. This should beHTMLDivElementfor type correctness.🔧 Proposed fix
const AlertDescription = React.forwardRef< - HTMLParagraphElement, + HTMLDivElement, React.HTMLAttributes<HTMLParagraphElement> >(({ className, ...props }, ref) => (Also update the props type to match:
const AlertDescription = React.forwardRef< HTMLDivElement, - React.HTMLAttributes<HTMLParagraphElement> + React.HTMLAttributes<HTMLDivElement> >(({ className, ...props }, ref) => (samples/hotel-booking-agent/frontend/src/hooks/use-toast.ts-174-182 (1)
174-182:⚠️ Potential issue | 🟡 MinorIncorrect
useEffectdependency causes unnecessary listener re-registration.The effect depends on
state, but the effect body only registers/unregisters thesetStatelistener. This causes the listener to be removed and re-added on every state change, which is inefficient. The dependency should be an empty array[]sincesetStateis stable.🔧 Proposed fix
React.useEffect(() => { listeners.push(setState) return () => { const index = listeners.indexOf(setState) if (index > -1) { listeners.splice(index, 1) } } - }, [state]) + }, [])samples/hotel-booking-agent/frontend/src/components/ui/toggle-group.tsx-39-57 (1)
39-57:⚠️ Potential issue | 🟡 MinorItem-level
variantandsizeprops are effectively ignored.The context always has values (defaulting to
"default"), socontext.variant || variantwill always use the context value. Item-level overrides will never apply. If item-level overrides should take precedence, reverse the logic. If group-level should always win, this is fine but item props become misleading.🔧 Proposed fix if item-level overrides should take precedence
className={cn( toggleVariants({ - variant: context.variant || variant, - size: context.size || size, + variant: variant || context.variant, + size: size || context.size, }), className )}samples/hotel-booking-agent/frontend/src/lib/queryClient.ts-31-34 (1)
31-34:⚠️ Potential issue | 🟡 Minor
queryKey.join("/")assumes all elements are strings.
QueryKeycan contain non-string values (numbers, objects, etc.). Using.join("/")on mixed types could produce malformed URLs. Consider explicitly handling the query key construction or documenting the expected format.🔧 Proposed defensive fix
async ({ queryKey }) => { - const res = await fetch(queryKey.join("/") as string, { + const res = await fetch(queryKey.map(String).join("/"), { credentials: "include", });Or alternatively, filter to only string segments:
const url = queryKey.filter((k): k is string => typeof k === "string").join("/");samples/hotel-booking-agent/frontend/src/components/ui/alert.tsx-35-44 (1)
35-44:⚠️ Potential issue | 🟡 MinorRef type mismatch: typed as
HTMLParagraphElementbut renders<h5>.The component renders an
<h5>heading element, but the ref is typed asHTMLParagraphElement. This should beHTMLHeadingElementfor type correctness.🔧 Proposed fix
const AlertTitle = React.forwardRef< - HTMLParagraphElement, + HTMLHeadingElement, React.HTMLAttributes<HTMLHeadingElement> >(({ className, ...props }, ref) => (samples/hotel-booking-agent/frontend/src/components/ui/carousel.tsx-135-141 (1)
135-141:⚠️ Potential issue | 🟡 MinorMake the carousel wrapper focusable for arrow-key handling.
Line 137 attachesonKeyDownCapture, but the wrapper isn’t focusable, so the handler won’t fire unless a child has focus. AddtabIndex={0}(or allow callers to pass it).🧩 Proposed fix
<div ref={ref} onKeyDownCapture={handleKeyDown} + tabIndex={0} className={cn("relative", className)}samples/hotel-booking-agent/frontend/src/components/ui/carousel.tsx-107-118 (1)
107-118:⚠️ Potential issue | 🟡 MinorClean up the
reInitlistener on unmount.Line 113 registers a
reInitlistener, but the cleanup function only removesselect, allowing callbacks to accumulate after re-inits. Theoffmethod is available in Embla 8.6.0 and should also removereInit:🛠️ Proposed fix
return () => { api?.off("select", onSelect) + api?.off("reInit", onSelect) }samples/hotel-booking-agent/frontend/src/components/ui/breadcrumb.tsx-105-105 (1)
105-105:⚠️ Potential issue | 🟡 MinorTypo in displayName: "BreadcrumbElipssis".
The displayName has an extra 's' — should be
"BreadcrumbEllipsis"to match the component name.🐛 Proposed fix
-BreadcrumbEllipsis.displayName = "BreadcrumbElipssis" +BreadcrumbEllipsis.displayName = "BreadcrumbEllipsis"samples/hotel-booking-agent/frontend/src/components/ui/menubar.tsx-237-237 (1)
237-237:⚠️ Potential issue | 🟡 MinorTypo:
displaynameshould bedisplayName.The property name is lowercase which means React DevTools won't recognize it. All other components in this file use the correct casing.
🐛 Proposed fix
-MenubarShortcut.displayname = "MenubarShortcut" +MenubarShortcut.displayName = "MenubarShortcut"samples/hotel-booking-agent/README.md-43-43 (1)
43-43:⚠️ Potential issue | 🟡 Minorkubectl command references old "travel-planner-agent" naming.
The service name should be updated to match the new "hotel-booking-agent" naming convention.
📝 Proposed fix
-kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80 +kubectl -n dp-default-default-default-ccb66d74 port-forward svc/hotel-booking-agent-is 9090:80samples/hotel-booking-agent/README.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorInconsistent naming: "travel planner agent" should be "hotel booking agent".
The README title is "Hotel Booking Agent" but Line 3 still refers to "travel planner agent". This appears to be a leftover from the rename mentioned in the commit messages.
📝 Proposed fix
-Minimal Python + React stack for the travel planner agent. +Minimal Python + React stack for the hotel booking agent.samples/hotel-booking-agent/frontend/src/pages/Landing.tsx-31-38 (1)
31-38:⚠️ Potential issue | 🟡 Minor
group-hoveranimation won't trigger.The
group-hover:translate-x-1class on the ArrowRight icon (Line 37) requires a parent element with thegroupclass. The Button component doesn't have this class, so the hover animation won't work.🐛 Proposed fix
<Button size="lg" onClick={() => setLocation("/assistant")} - className="mt-10 bg-[`#ffb347`] hover:bg-[`#ff9f1c`] text-slate-900 font-semibold text-base md:text-lg px-8 py-4 rounded-full h-auto shadow-lg relative" + className="group mt-10 bg-[`#ffb347`] hover:bg-[`#ff9f1c`] text-slate-900 font-semibold text-base md:text-lg px-8 py-4 rounded-full h-auto shadow-lg relative" > Start Planning Now <ArrowRight className="ml-2 w-5 h-5 group-hover:translate-x-1 transition-transform" /> </Button>samples/hotel-booking-agent/frontend/src/pages/Landing.tsx-15-19 (1)
15-19:⚠️ Potential issue | 🟡 MinorMissing
relativepositioning on parent element.The gradient overlay div uses
absolute inset-0(Line 19), but its parentsectionelement lacksrelativepositioning. This may cause the overlay to position relative to an unintended ancestor.🐛 Proposed fix
<section - className="h-full flex items-center justify-center bg-cover bg-center" + className="relative h-full flex items-center justify-center bg-cover bg-center" style={{ backgroundImage: "url('/images/orchid-hotels.webp')" }} >samples/hotel-booking-agent/backend/booking_api/data/mock_dataset.json-1240-1304 (1)
1240-1304:⚠️ Potential issue | 🟡 MinorReview count mismatch with actual reviews.
Several hotels (e.g.,
mock-006-london,mock-012-tokyo,mock-013-newyork,mock-014-dubai,mock-015-sydney,mock-016-london) havereviewCount: 1in their hotel data but no corresponding entries in thereviewsarray. This inconsistency could cause issues if the UI displays "1 review" but shows an empty reviews section.samples/hotel-booking-agent/backend/booking_api/openapi.yaml-597-601 (1)
597-601:⚠️ Potential issue | 🟡 Minor
nightsminimum should be 1, not 0.A booking with 0 nights is semantically invalid. The minimum should be
1to ensure valid pricing calculations.nights: type: integer - minimum: 0 + minimum: 1 description: Number of nightssamples/hotel-booking-agent/frontend/src/components/ui/form.tsx-44-53 (1)
44-53:⚠️ Potential issue | 🟡 MinorCheck for
fieldContext.nameoccurs after it's already used.The validation
if (!fieldContext)at line 51 happens afterfieldContext.nameis accessed at line 49. IfuseFormFieldis called outside a<FormField>, the error will be a confusing undefined access rather than the intended helpful error message.const useFormField = () => { const fieldContext = React.useContext(FormFieldContext) const itemContext = React.useContext(FormItemContext) + + if (!fieldContext.name) { + throw new Error("useFormField should be used within <FormField>") + } + const { getFieldState, formState } = useFormContext() - const fieldState = getFieldState(fieldContext.name, formState) - - if (!fieldContext) { - throw new Error("useFormField should be used within <FormField>") - } const { id } = itemContextsamples/hotel-booking-agent/frontend/src/components/Navbar.tsx-128-130 (1)
128-130:⚠️ Potential issue | 🟡 MinorMobile menu button lacks onClick handler and associated menu implementation.
The hamburger menu button (lines 128-130) is rendered for small screens but has no
onClickhandler or mobile menu panel. Implement the mobile menu functionality or remove the button if not needed.samples/hotel-booking-agent/frontend/src/pages/Home.tsx-308-312 (1)
308-312:⚠️ Potential issue | 🟡 MinorFragile message ID generation could cause collisions.
Using
Date.now() + 1for the assistant message ID after the user message'sDate.now()could collide if operations complete within the same millisecond. Consider using a more robust approach.💡 Suggested fix
const assistantMessage: Message = { - id: (Date.now() + 1).toString(), + id: `${Date.now()}-assistant-${Math.random().toString(36).slice(2, 8)}`, role: 'assistant', content: data?.message || "Sorry, I couldn't generate a response.", };samples/hotel-booking-agent/backend/agent/tools.py-485-551 (1)
485-551:⚠️ Potential issue | 🟡 MinorPrevent duplicate resolution for “day after tomorrow”.
The phrase contains “tomorrow”, so both entries are added. Check the longer phrase first (or useelif) to keep results unambiguous.🛠️ Suggested fix
- if "tomorrow" in lowered: - _add("tomorrow", now + timedelta(days=1)) - if "day after tomorrow" in lowered: - _add("day_after_tomorrow", now + timedelta(days=2)) + if "day after tomorrow" in lowered: + _add("day_after_tomorrow", now + timedelta(days=2)) + elif "tomorrow" in lowered: + _add("tomorrow", now + timedelta(days=1))
🧹 Nitpick comments (32)
samples/hotel-booking-agent/frontend/src/hooks/use-mobile.tsx (1)
9-14: Consider usingmql.matchesfor consistency.The hook subscribes to the media query's change event but then reads
window.innerWidthdirectly instead of using theMediaQueryList.matchesproperty. Usingmql.matcheswould be more idiomatic and consistent with the media query approach.🔧 Suggested refactor
React.useEffect(() => { const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) } mql.addEventListener("change", onChange) - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) return () => mql.removeEventListener("change", onChange) }, [])samples/hotel-booking-agent/frontend/package.json (2)
2-2: Package name doesn't match project directory.The package is named
o2_trips_frontendbut resides inhotel-booking-agent/frontend. Based on the commit message indicating a rename from "travel planner agent" to "hotel-booking-agent", this appears to be a leftover. Consider updating for consistency.✏️ Suggested fix
- "name": "o2_trips_frontend", + "name": "hotel-booking-agent-frontend",
36-39: Move testing libraries to devDependencies.
@testing-library/*packages are development-only dependencies used for running tests. Placing them independenciesunnecessarily increases install size for production deployments.♻️ Suggested fix
Move these entries from
dependenciestodevDependencies:"dependencies": { ... - "@testing-library/dom": "^10.4.0", - "@testing-library/jest-dom": "^6.6.3", - "@testing-library/react": "^16.3.0", - "@testing-library/user-event": "^13.5.0", ... }, "devDependencies": { + "@testing-library/dom": "^10.4.0", + "@testing-library/jest-dom": "^6.6.3", + "@testing-library/react": "^16.3.0", + "@testing-library/user-event": "^13.5.0", ... }samples/hotel-booking-agent/frontend/src/components/SearchHero.tsx (2)
3-3: Remove unusedmotionimport.
motionfromframer-motionis imported but never used in this component.import { useState } from "react"; import { Sparkles, Send } from "lucide-react"; -import { motion } from "framer-motion";
56-58: Consider clearing the input after quick search to match form submission behavior.The quick search buttons set the query and call
onSearchdirectly, but unlikehandleSearch, they don't clear the input afterward. This creates an inconsistent UX—submitting via Enter clears the field, but clicking a suggestion leaves it populated.If the intent is to show users what was searched, this is fine. Otherwise, consider extracting the search logic to avoid duplication:
♻️ Optional refactor
+ const triggerSearch = (searchQuery: string) => { + if (!searchQuery.trim()) return; + onSearch?.(searchQuery); + setQuery(""); + }; + const handleSearch = (e: React.FormEvent) => { e.preventDefault(); - if (!query.trim()) return; - onSearch?.(query); - setQuery(""); + triggerSearch(query); }; // Then in the quick search buttons: - <button onClick={() => { setQuery("Romantic Paris getaway"); onSearch?.("Romantic Paris getaway"); }} ...> + <button onClick={() => triggerSearch("Romantic Paris getaway")} ...>samples/hotel-booking-agent/frontend/tsconfig.json (1)
3-4: UpdatetargettoES2022for React 18 alignment.Line 3 sets
targetto ES5, which is unnecessary for this project's modern browser support matrix (>0.2%, not dead) and conflicts with the modern React 18 ecosystem. Create React App applies the tsconfig target to actual output, so ES5 downleveling adds unnecessary build complexity. Updating toES2022aligns with modern TypeScript/React standards and improves type-checking accuracy.Proposed adjustment
- "target": "es5", + "target": "es2022",samples/hotel-booking-agent/frontend/src/shared/schema.ts (1)
1-16: NormalizeHotel.ratingto a single numeric type. Mixed types make comparisons and sorting error-prone; consider usingnumberonly and normalize inputs upstream.Proposed change
export type Hotel = { id: number; name: string; location: string; description: string; - rating: string | number; + rating: number; reviewCount: number; pricePerNight: number; imageUrl: string; website?: string;samples/hotel-booking-agent/frontend/src/config/auth.ts (1)
1-10: Make auth configuration environment-driven. Hardcoded tenant/client IDs and redirect URLs make non-local deployments brittle; consider sourcing from environment variables with safe defaults.samples/hotel-booking-agent/frontend/src/lib/mockData.ts (1)
3-80: Use numericratingvalues for consistency with a numeric schema. This avoids parsing and sorting pitfalls.Proposed change
- rating: "4.9", + rating: 4.9, ... - rating: "4.7", + rating: 4.7, ... - rating: "4.8", + rating: 4.8, ... - rating: "4.5", + rating: 4.5, ... - rating: "4.9", + rating: 4.9, ... - rating: "4.6", + rating: 4.6,samples/hotel-booking-agent/backend/agent/openapi.yaml (1)
7-21: Consider adding a top-levelsecuritysection. This avoids accidentally leaving new endpoints unauthenticated if added later.Proposed change
servers: - url: http://localhost:9090 description: Local development server - url: https://api.yourcompany.com description: Production server + +security: + - bearerAuth: [] paths: /chat: post:samples/hotel-booking-agent/frontend/src/components/ui/tooltip.tsx (1)
14-30: Wrap TooltipContent in Portal to prevent clipping in overflow containers.Radix UI's tooltip documentation recommends keeping
Tooltip.ContentinsideTooltip.Portalby default. This prevents clipping fromoverflow: hidden, z-index traps, and stacking-context issues.Proposed change
-const TooltipContent = React.forwardRef< +const TooltipContent = React.forwardRef< React.ElementRef<typeof TooltipPrimitive.Content>, React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content> >(({ className, sideOffset = 4, ...props }, ref) => ( - <TooltipPrimitive.Content - ref={ref} - sideOffset={sideOffset} - className={cn( - "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-tooltip-content-transform-origin]", - className - )} - {...props} - /> + <TooltipPrimitive.Portal> + <TooltipPrimitive.Content + ref={ref} + sideOffset={sideOffset} + className={cn( + "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 origin-[--radix-tooltip-content-transform-origin]", + className + )} + {...props} + /> + </TooltipPrimitive.Portal> ))samples/hotel-booking-agent/frontend/src/components/ChatHotelResults.tsx (1)
145-149: Consider using index-based keys for amenities to handle potential duplicates.Using
amenity(the string value) as the key could cause rendering issues if the amenities array contains duplicate values. While unlikely, it's safer to use a composite key.🔧 Proposed fix
- {amenities.map((amenity) => ( - <span key={amenity} className="amenity-tag"> + {amenities.map((amenity, index) => ( + <span key={`${amenity}-${index}`} className="amenity-tag"> {amenity} </span> ))}samples/hotel-booking-agent/frontend/src/components/ui/card.tsx (1)
32-45: Consider using a heading element for CardTitle for better semantics.
CardTitlerenders a<div>but semantically represents a title. For better accessibility and document structure, consider using a heading element or allowing the element type to be customized via anasprop.This is a minor accessibility consideration - screen readers and SEO benefit from proper heading hierarchy. However, the current implementation matches common shadcn/ui patterns and works fine in practice.
samples/hotel-booking-agent/frontend/src/hooks/use-hotels.ts (2)
18-28: Consider disabling the query whenidis invalid.If
useHotelis called with an invalid id (e.g.,0,undefined, orNaN), the query will still execute and throw an error. Consider adding anenabledoption to prevent unnecessary query execution.🔧 Proposed fix
export function useHotel(id: number) { return useQuery({ queryKey: ['/api/hotels', id], queryFn: async () => { await new Promise(resolve => setTimeout(resolve, 600)); const hotel = MOCK_HOTELS.find(h => h.id === id); if (!hotel) throw new Error("Hotel not found"); return hotel; }, + enabled: id > 0, }); }
30-41: Same consideration foruseHotelRooms- disable whenhotelIdis invalid.🔧 Proposed fix
export function useHotelRooms(hotelId: number) { return useQuery({ queryKey: ['/api/hotels', hotelId, 'rooms'], queryFn: async () => { await new Promise(resolve => setTimeout(resolve, 700)); const rooms = MOCK_ROOMS.filter(r => r.hotelId === hotelId); return rooms.length > 0 ? rooms : MOCK_ROOMS.slice(0, 2); }, + enabled: hotelId > 0, }); }samples/hotel-booking-agent/frontend/src/hooks/use-toast.ts (1)
90-114: Side effects in reducer are acknowledged but worth extracting.The comment on lines 93-94 acknowledges that
addToRemoveQueueis a side effect within the reducer. While this works, it couples the reducer to the timeout logic. Consider extracting this into a middleware or handling it in thedispatchfunction for cleaner separation of concerns.samples/hotel-booking-agent/frontend/src/lib/queryClient.ts (1)
44-57: Aggressive caching configuration noted.
staleTime: InfinitywithrefetchOnWindowFocus: falsemeans queries will never automatically refresh. This is acceptable for a sample application but ensure this aligns with the expected user experience for hotel search results that may change frequently.samples/hotel-booking-agent/frontend/src/components/ui/navigation-menu.tsx (1)
71-73: Minor: trailing space in className string.Line 72 has a trailing space before the closing quote. This is harmless but can be cleaned up.
🧹 Cleanup
className={cn( - "left-0 top-0 w-full data-[motion^=from-]:animate-in data-[motion^=to-]:animate-out data-[motion^=from-]:fade-in data-[motion^=to-]:fade-out data-[motion=from-end]:slide-in-from-right-52 data-[motion=from-start]:slide-in-from-left-52 data-[motion=to-end]:slide-out-to-right-52 data-[motion=to-start]:slide-out-to-left-52 md:absolute md:w-auto ", + "left-0 top-0 w-full data-[motion^=from-]:animate-in data-[motion^=to-]:animate-out data-[motion^=from-]:fade-in data-[motion^=to-]:fade-out data-[motion=from-end]:slide-in-from-right-52 data-[motion=from-start]:slide-in-from-left-52 data-[motion=to-end]:slide-out-to-right-52 data-[motion=to-start]:slide-out-to-left-52 md:absolute md:w-auto", className )}samples/hotel-booking-agent/frontend/src/components/ui/breadcrumb.tsx (1)
7-13: Unusedseparatorprop in Breadcrumb component.The
separatorprop is declared in the component's type definition but is never used in the implementation. Either implement the separator functionality or remove the prop from the type.♻️ Option 1: Remove unused prop
const Breadcrumb = React.forwardRef< HTMLElement, - React.ComponentPropsWithoutRef<"nav"> & { - separator?: React.ReactNode - } + React.ComponentPropsWithoutRef<"nav"> >(({ ...props }, ref) => <nav ref={ref} aria-label="breadcrumb" {...props} />)samples/hotel-booking-agent/frontend/src/index.tsx (1)
7-21: Consider adding React.StrictMode and handling missing root element.The setup looks correct. Two optional improvements:
- Wrapping with
React.StrictModehelps catch potential issues during development.- The missing root element case fails silently - consider logging an error.
💡 Optional enhancement
const root = document.getElementById("root"); if (root) { createRoot(root).render( + <React.StrictMode> <AsgardeoProvider clientId={authConfig.clientId} baseUrl={authConfig.baseUrl} scopes={authConfig.scopes} afterSignInUrl={authConfig.afterSignInUrl} afterSignOutUrl={authConfig.afterSignOutUrl} storage={authConfig.storage} > <App /> </AsgardeoProvider> + </React.StrictMode> ); +} else { + console.error("Root element not found"); }samples/hotel-booking-agent/frontend/src/App.tsx (1)
26-28: Consider redirecting instead of rendering SignIn inline.When an unauthenticated user accesses
/assistant, the SignIn component renders while the URL remains/assistant. This can confuse users and affect browser history. Consider using Wouter'sRedirectcomponent to navigate to/signin.♻️ Proposed refactor
+import { Switch, Route, useLocation, Redirect } from "wouter"; ... if (!isSignedIn && requiresAuth) { - return <SignIn />; + return <Redirect to="/signin" />; }samples/hotel-booking-agent/frontend/tailwind.config.js (2)
26-50: Inconsistent border color definitions.Some border colors use raw
var()references (Lines 29, 34, 39, 44, 49) while the mainbordercolor useshsl()wrapper. This prevents using Tailwind's opacity modifiers on these border colors.💡 For consistency, consider wrapping all borders with hsl()
primary: { DEFAULT: "hsl(var(--primary) / <alpha-value>)", foreground: "hsl(var(--primary-foreground) / <alpha-value>)", - border: "var(--primary-border)", + border: "hsl(var(--primary-border) / <alpha-value>)", },Apply similar changes to secondary, muted, accent, and destructive border definitions.
75-80: Status colors use hardcoded RGB values.Status colors are hardcoded (
rgb(34 197 94), etc.) rather than using CSS variables like other colors. This reduces theming flexibility if dark mode or custom themes need different status colors.💡 Consider using CSS variables for consistency
status: { - online: "rgb(34 197 94)", - away: "rgb(245 158 11)", - busy: "rgb(239 68 68)", - offline: "rgb(156 163 175)", + online: "hsl(var(--status-online) / <alpha-value>)", + away: "hsl(var(--status-away) / <alpha-value>)", + busy: "hsl(var(--status-busy) / <alpha-value>)", + offline: "hsl(var(--status-offline) / <alpha-value>)", },Then define
--status-*CSS variables in yourindex.cssfor both light and dark modes.samples/hotel-booking-agent/backend/booking_api/data/mock_dataset.json (1)
1010-1238: Nearby attractions are missing for several hotels.Hotels
mock-002-tokyo,mock-005-sydney,mock-012-tokyo,mock-013-newyork,mock-014-dubai,mock-015-sydney, andmock-016-londonhave no entries in thenearby_attractionsarray, while they exist in thehotelsarray. If this is intentional for the mock dataset, consider adding a comment or ensure the consuming code handles missing attractions gracefully.Additionally, all attraction
locationobjects use placeholder coordinates (latitude: 0.0,longitude: 0.0). If map rendering is planned, these should be populated with realistic values.samples/hotel-booking-agent/backend/booking_api/openapi.yaml (1)
238-242: Consider addingmaxItemsconstraint to array responses.The
/bookingsendpoint returns an unbounded array. For production APIs, addingmaxItemsprevents excessively large responses and aligns with API security best practices. This also applies to other array responses likehotelsinHotelSearchResponse.🛡️ Example fix
schema: type: array + maxItems: 100 items: $ref: '#/components/schemas/Booking'samples/hotel-booking-agent/frontend/src/components/Navbar.tsx (1)
95-98: Potential contrast issue in non-themed variant.The signed-in user container uses
border-white/20 bg-white/10for both themed and non-themed variants. On the non-themed navbar (bg-white/80), this creates a nearly invisible white-on-white container. Consider using a different background for the default variant.className={`flex items-center gap-4 rounded-full px-4 py-2 ${ - isThemed ? "border border-white/20 bg-white/10" : "border border-white/20 bg-white/10 backdrop-blur-md" + isThemed ? "border border-white/20 bg-white/10" : "border border-border bg-muted/50" }`}samples/hotel-booking-agent/backend/agent/graph.py (3)
8-8: Remove unused importEND.
ENDis imported fromlanggraph.graphbut never used in the code. Thetools_conditionfunction handles the END transition automatically.-from langgraph.graph import StateGraph, END +from langgraph.graph import StateGraph
70-70: Remove leftover debug comment.The inline comment
#add in memory serverappears to be a development note that should be cleaned up.- graph = StateGraph(AgentState) `#add` in memory server + graph = StateGraph(AgentState)
79-80:InMemorySaverstate is not persisted across restarts.The in-memory checkpointer loses all conversation state when the server restarts. For a sample application this is acceptable, but for production use consider a persistent backend like Redis or a database-backed saver.
samples/hotel-booking-agent/frontend/src/components/ui/input-otp.tsx (1)
34-36: Missing bounds check onslots[index]access.If an invalid
indexis passed,inputOTPContext.slots[index]will beundefined, causing destructuring to fail. Consider adding a defensive check.>(({ index, className, ...props }, ref) => { const inputOTPContext = React.useContext(OTPInputContext) - const { char, hasFakeCaret, isActive } = inputOTPContext.slots[index] + const slot = inputOTPContext.slots[index] + if (!slot) { + return null + } + const { char, hasFakeCaret, isActive } = slotsamples/hotel-booking-agent/frontend/src/pages/Home.tsx (1)
91-95: Hardcoded API URLs reduce deployment flexibility.Consider extracting these to environment variables or a configuration module to support different deployment environments.
-const CHAT_API_URL = "http://localhost:9090/chat"; -const CHAT_SESSIONS_URL = `${CHAT_API_URL}/sessions`; +const CHAT_API_URL = import.meta.env.VITE_CHAT_API_URL || "http://localhost:9090/chat"; +const CHAT_SESSIONS_URL = `${CHAT_API_URL}/sessions`;samples/hotel-booking-agent/resources/ingest/ingest.py (1)
108-131: Unbatched upserts may hit Pinecone limits for large documents.All vectors are accumulated and upserted in a single call. Pinecone has limits on upsert batch sizes (typically 100 vectors). For large policy documents with many chunks, this could fail.
💡 Suggested batching approach
def ingest_chunks(self, chunks: List[Dict[str, Any]], source_folder: str): """Ingest document chunks into Pinecone""" vectors = [] + BATCH_SIZE = 100 for i, chunk in enumerate(chunks): embedding = self.generate_embedding(chunk["content"]) chunk_id = f"{source_folder.replace('/', '_')}_{i}" vectors.append({ "id": chunk_id, "values": embedding, "metadata": { "content": chunk["content"], **chunk["metadata"] } }) + + if len(vectors) >= BATCH_SIZE: + self.index.upsert(vectors=vectors) + vectors = [] - if vectors: - self.index.upsert(vectors=vectors) + if vectors: # Remaining vectors + self.index.upsert(vectors=vectors)
| >(({ className, value, ...props }, ref) => ( | ||
| <ProgressPrimitive.Root | ||
| ref={ref} | ||
| className={cn( | ||
| "relative h-4 w-full overflow-hidden rounded-full bg-secondary", | ||
| className | ||
| )} | ||
| {...props} | ||
| > | ||
| <ProgressPrimitive.Indicator | ||
| className="h-full w-full flex-1 bg-primary transition-all" | ||
| style={{ transform: `translateX(-${100 - (value || 0)}%)` }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the file in question
find . -name "progress.tsx" -type f 2>/dev/null | head -20Repository: wso2/agent-manager
Length of output: 131
🏁 Script executed:
# Look for the actual file and its content
cat -n samples/hotel-booking-agent/frontend/src/components/ui/progress.tsx 2>/dev/null || echo "File not found in expected location"Repository: wso2/agent-manager
Length of output: 1046
🌐 Web query:
Radix UI Progress component Root value prop ARIA semantics documentation
💡 Result:
Radix UI’s Progress.Root value prop is documented as number | null, and Radix states the component “adheres to the progressbar role requirements.” (radix-ui.com)
What that implies for ARIA semantics (per progressbar requirements)
- Determinate progress (
valueis a number):- The element with
role="progressbar"should expose the current numeric value viaaria-valuenow. (developer.mozilla.org) - The range is defined by
aria-valueminandaria-valuemax; forprogressbar, defaults are 0 and 100 when not provided/invalid (so if you don’t setmax, the ARIA max is effectively 100 by the pattern). (developer.mozilla.org)
- The element with
- Indeterminate progress (
value={null}):- When there is no known current value, don’t set
aria-valuenow(i.e., it should be absent). (developer.mozilla.org)
- When there is no known current value, don’t set
Where Radix documents the props
Progress.Rootprops include:value: number | nullmax: numbergetValueLabel: function(radix-ui.com)
Citations:
- 1: https://www.radix-ui.com/primitives/docs/components/progress
- 2: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/progressbar_role?utm_source=openai
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/progressbar_role?utm_source=openai
- 4: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-valuenow?utm_source=openai
- 5: https://www.radix-ui.com/primitives/docs/components/progress
Pass value to ProgressPrimitive.Root to maintain ARIA semantics and prevent out-of-range indicator overflow.
The value prop is destructured but not forwarded to the Root component, causing two issues:
- ARIA semantics break: Without
valueon Root, Radix UI doesn't setaria-valuenow, making the progress appear indeterminate to assistive technologies. - Visual overflow: Values exceeding 100 push the indicator beyond the track container (e.g.,
value=150producestranslateX(50%)).
Normalize value once and pass it to both Root and the transform calculation.
Suggested fix
->(({ className, value, ...props }, ref) => (
- <ProgressPrimitive.Root
- ref={ref}
- className={cn(
- "relative h-4 w-full overflow-hidden rounded-full bg-secondary",
- className
- )}
- {...props}
- >
- <ProgressPrimitive.Indicator
- className="h-full w-full flex-1 bg-primary transition-all"
- style={{ transform: `translateX(-${100 - (value || 0)}%)` }}
- />
- </ProgressPrimitive.Root>
-))
+>(({ className, value, ...props }, ref) => {
+ const clamped = Math.min(100, Math.max(0, value ?? 0))
+ return (
+ <ProgressPrimitive.Root
+ ref={ref}
+ value={clamped}
+ className={cn(
+ "relative h-4 w-full overflow-hidden rounded-full bg-secondary",
+ className
+ )}
+ {...props}
+ >
+ <ProgressPrimitive.Indicator
+ className="h-full w-full flex-1 bg-primary transition-all"
+ style={{ transform: `translateX(-${100 - clamped}%)` }}
+ />
+ </ProgressPrimitive.Root>
+ )
+})📝 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.
| >(({ className, value, ...props }, ref) => ( | |
| <ProgressPrimitive.Root | |
| ref={ref} | |
| className={cn( | |
| "relative h-4 w-full overflow-hidden rounded-full bg-secondary", | |
| className | |
| )} | |
| {...props} | |
| > | |
| <ProgressPrimitive.Indicator | |
| className="h-full w-full flex-1 bg-primary transition-all" | |
| style={{ transform: `translateX(-${100 - (value || 0)}%)` }} | |
| /> | |
| >(({ className, value, ...props }, ref) => { | |
| const clamped = Math.min(100, Math.max(0, value ?? 0)) | |
| return ( | |
| <ProgressPrimitive.Root | |
| ref={ref} | |
| value={clamped} | |
| className={cn( | |
| "relative h-4 w-full overflow-hidden rounded-full bg-secondary", | |
| className | |
| )} | |
| {...props} | |
| > | |
| <ProgressPrimitive.Indicator | |
| className="h-full w-full flex-1 bg-primary transition-all" | |
| style={{ transform: `translateX(-${100 - clamped}%)` }} | |
| /> | |
| </ProgressPrimitive.Root> | |
| ) | |
| }) |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/frontend/src/components/ui/progress.tsx` around
lines 11 - 23, The Progress component destructures `value` but doesn't pass it
to ProgressPrimitive.Root and doesn't normalize it, breaking ARIA semantics and
allowing indicator overflow; fix by clamping/normalizing the incoming `value` to
the 0–100 range (e.g., Math.min(Math.max(Number(value) || 0, 0), 100)), pass
this normalized value into ProgressPrimitive.Root as the `value` prop, and use
that same normalized value for the Indicator transform calculation (reference
ProgressPrimitive.Root, ProgressPrimitive.Indicator, and the `value` prop).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@samples/hotel-booking-agent/agent/tools.py`:
- Around line 494-500: The current checks on variable lowered add both
"tomorrow" and "day_after_tomorrow" when the string contains "day after
tomorrow"; update the branching in the block that calls _add (using lowered,
now, and timedelta) to avoid duplicate resolutions by either checking the longer
phrase first (check "day after tomorrow" before "tomorrow") or converting the
successive ifs into an if/elif chain so once "day after tomorrow" matches it
won't also match "tomorrow".
In `@samples/hotel-booking-agent/services/hotel_api/booking.py`:
- Around line 139-141: The handlers currently return the error dict directly
(via _error_response) which results in a 200 OK; change get_booking,
update_booking, and cancel_booking to return proper 404 HTTP responses by either
raising fastapi.HTTPException(status_code=404, detail=_error_response("Booking
not found", "BOOKING_NOT_FOUND")) or by returning a Response/JSONResponse with
status_code=404 and the error payload; update calls that use _error_response to
construct the detail/body rather than returning the dict directly so clients
receive an actual 404 status.
In `@samples/hotel-booking-agent/services/hotel_api/service.py`:
- Around line 10-17: The CORS configuration uses app.add_middleware with
CORSMiddleware and currently sets allow_origins=["*"] together with
allow_credentials=True which is invalid for credentialed requests; either remove
allow_credentials=True from that call (if wildcard origins are desired) or
replace allow_origins=["*"] with an explicit list of allowed origins (e.g.,
["https://example.com"]) while keeping allow_credentials=True; update the
CORSMiddleware call in service.py accordingly (referencing app.add_middleware,
CORSMiddleware, allow_origins, allow_credentials) so the server responds with a
specific Access-Control-Allow-Origin when credentials are allowed.
🧹 Nitpick comments (3)
samples/hotel-booking-agent/services/hotel_api/booking.py (1)
20-25: Consider unifying_error_responsewithsearch.py.This module's
_error_responseincludes atimestampfield, whilesearch.py(Lines 306-310) does not. For consistency across the API, consider extracting a shared helper or aligning the response structures.samples/hotel-booking-agent/services/hotel_api/openapi.yaml (1)
240-242: Consider addingmaxItemsto array schemas for defensive specification.The booking array response (and similar arrays throughout) lacks a
maxItemsconstraint. While not critical for a sample app, adding reasonable limits improves API documentation and can help prevent abuse.samples/hotel-booking-agent/agent/openapi.yaml (1)
7-21: Declare global bearerAuth to secure future endpoints by default.
This prevents new paths from accidentally being unauthenticated.🔧 Suggested update
servers: - url: http://localhost:9090 description: Local development server - url: https://api.yourcompany.com description: Production server +security: + - bearerAuth: [] paths: /chat: post:
| if "today" in lowered: | ||
| _add("today", now) | ||
| if "tomorrow" in lowered: | ||
| _add("tomorrow", now + timedelta(days=1)) | ||
| if "day after tomorrow" in lowered: | ||
| _add("day_after_tomorrow", now + timedelta(days=2)) | ||
|
|
There was a problem hiding this comment.
"day after tomorrow" currently resolves both tomorrow and day‑after.
Check the longer phrase first (or use elif) to avoid duplicate resolutions.
🔧 Suggested fix
- if "today" in lowered:
- _add("today", now)
- if "tomorrow" in lowered:
- _add("tomorrow", now + timedelta(days=1))
- if "day after tomorrow" in lowered:
- _add("day_after_tomorrow", now + timedelta(days=2))
+ if "today" in lowered:
+ _add("today", now)
+ if "day after tomorrow" in lowered:
+ _add("day_after_tomorrow", now + timedelta(days=2))
+ elif "tomorrow" in lowered:
+ _add("tomorrow", now + timedelta(days=1))📝 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.
| if "today" in lowered: | |
| _add("today", now) | |
| if "tomorrow" in lowered: | |
| _add("tomorrow", now + timedelta(days=1)) | |
| if "day after tomorrow" in lowered: | |
| _add("day_after_tomorrow", now + timedelta(days=2)) | |
| if "today" in lowered: | |
| _add("today", now) | |
| if "day after tomorrow" in lowered: | |
| _add("day_after_tomorrow", now + timedelta(days=2)) | |
| elif "tomorrow" in lowered: | |
| _add("tomorrow", now + timedelta(days=1)) | |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/tools.py` around lines 494 - 500, The
current checks on variable lowered add both "tomorrow" and "day_after_tomorrow"
when the string contains "day after tomorrow"; update the branching in the block
that calls _add (using lowered, now, and timedelta) to avoid duplicate
resolutions by either checking the longer phrase first (check "day after
tomorrow" before "tomorrow") or converting the successive ifs into an if/elif
chain so once "day after tomorrow" matches it won't also match "tomorrow".
| if not booking: | ||
| return _error_response("Booking not found", "BOOKING_NOT_FOUND") | ||
| return booking |
There was a problem hiding this comment.
Return proper HTTP status codes for error responses.
The error dict is returned but FastAPI will send it with a 200 status code. Use Response or HTTPException to return the appropriate 404 status.
Suggested fix
+from fastapi import APIRouter, Request, HTTPException
+
`@router.get`("/bookings/{booking_id}")
def get_booking(booking_id: str, request: Request):
user_id = _resolve_user_id(request)
try:
bookings = _load_bookings()
booking = next(
(
item
for item in bookings
if item.get("bookingId") == booking_id and item.get("userId") == user_id
),
None,
)
if not booking:
- return _error_response("Booking not found", "BOOKING_NOT_FOUND")
+ raise HTTPException(status_code=404, detail=_error_response("Booking not found", "BOOKING_NOT_FOUND"))
return bookingThe same issue applies to update_booking (Line 161) and cancel_booking (Line 201).
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/booking.py` around lines 139 -
141, The handlers currently return the error dict directly (via _error_response)
which results in a 200 OK; change get_booking, update_booking, and
cancel_booking to return proper 404 HTTP responses by either raising
fastapi.HTTPException(status_code=404, detail=_error_response("Booking not
found", "BOOKING_NOT_FOUND")) or by returning a Response/JSONResponse with
status_code=404 and the error payload; update calls that use _error_response to
construct the detail/body rather than returning the dict directly so clients
receive an actual 404 status.
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["*"], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| max_age=86400, | ||
| ) |
There was a problem hiding this comment.
CORS: Wildcard origin with credentials is insecure and may not work as expected.
allow_origins=["*"] combined with allow_credentials=True is problematic:
- Browsers block credentialed requests when the server responds with
Access-Control-Allow-Origin: * - This configuration creates a false sense of security while not actually working for authenticated requests
For a sample app, either remove allow_credentials=True or specify explicit origins.
Suggested fix (option 1: remove credentials)
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
- allow_credentials=True,
+ allow_credentials=False,
allow_methods=["*"],
allow_headers=["*"],
max_age=86400,
)📝 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.
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=["*"], | |
| allow_credentials=True, | |
| allow_methods=["*"], | |
| allow_headers=["*"], | |
| max_age=86400, | |
| ) | |
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=["*"], | |
| allow_credentials=False, | |
| allow_methods=["*"], | |
| allow_headers=["*"], | |
| max_age=86400, | |
| ) |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/service.py` around lines 10 -
17, The CORS configuration uses app.add_middleware with CORSMiddleware and
currently sets allow_origins=["*"] together with allow_credentials=True which is
invalid for credentialed requests; either remove allow_credentials=True from
that call (if wildcard origins are desired) or replace allow_origins=["*"] with
an explicit list of allowed origins (e.g., ["https://example.com"]) while
keeping allow_credentials=True; update the CORSMiddleware call in service.py
accordingly (referencing app.add_middleware, CORSMiddleware, allow_origins,
allow_credentials) so the server responds with a specific
Access-Control-Allow-Origin when credentials are allowed.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@samples/hotel-booking-agent/agent/openapi.yaml`:
- Around line 1-71: The OpenAPI spec is missing a security scheme and security
requirement for the /chat endpoint; add a Bearer JWT scheme under
components.securitySchemes (e.g., name it BearerAuth) and add a global security
requirement (security: - BearerAuth: []) and/or apply it specifically to the
/chat POST operation (operationId: chat_chat_post) so the endpoint requires the
bearer token for production use.
In `@samples/hotel-booking-agent/README.md`:
- Line 3: Update the README description line that currently reads "Minimal
Python stack for the travel planner agent, plus optional frontend." to
accurately describe the project as the Hotel Booking Agent (for example:
"Minimal Python stack for the Hotel Booking Agent, plus optional frontend.");
locate and edit the top-level description string in README.md to ensure the
agent name matches the repository content.
In `@samples/hotel-booking-agent/services/hotel_api/ingest.py`:
- Around line 56-58: Replace the corrupted line containing backticks and garbage
with a valid call to add the prepared chunks to the vector store: ensure you
have the statement vectorstore.add_documents(chunks) (or await
vectorstore.add_documents(chunks) if add_documents is async) and then keep the
existing print(f"✓ Ingested {folder.name}"); remove any stray characters so the
block reads cleanly and references the existing variables vectorstore,
add_documents, chunks and folder.name.
- Line 45: Replace the unsafe eval usage when reading metadata by parsing the
file as JSON: in the code that assigns metadata (the line using metadata =
eval(metadata_path.read_text())), import the json module and use json.load (or
json.loads) to deserialize the content from metadata_path safely, ensuring you
open/read the file or use metadata_path.read_text() passed to json.loads; keep
the variable name metadata and preserve metadata_path usage to locate the
change.
In `@samples/hotel-booking-agent/services/hotel_api/openapi.yaml`:
- Around line 23-86: The booking endpoints (/bookings and
/bookings/{booking_id}) currently accept a caller-controlled userId query
parameter and an optional x-user-id header; remove the userId query parameter
from the OpenAPI operations (and any other booking-related operations) and make
caller identity come from a trusted source — either mark x-user-id as required
only if it is injected by your auth layer or replace it with your standardized
auth header/identity mechanism (e.g., Authorization or an internal-required
header populated by the server). Update the schemas/references that expect
caller-supplied userId (e.g., Booking, BookingCreateRequest,
BookingCreateResponse) to rely on server-side identity resolution instead, and
apply the same change to all booking routes in this spec.
- Around line 306-394: The schemas Booking, BookingCreateRequest, and
BookingUpdateRequest are missing the numberOfRooms field; add a numberOfRooms
property (type: integer) to each of these definitions and include numberOfRooms
in the BookingCreateRequest.required array so generated clients will send it;
update the properties for Booking (Booking.numberOfRooms), BookingCreateRequest
(BookingCreateRequest.numberOfRooms) and BookingUpdateRequest
(BookingUpdateRequest.numberOfRooms) accordingly.
In `@samples/hotel-booking-agent/services/hotel_api/service.py`:
- Around line 76-82: The code calls a non-existent PolicyIngestion class; update
the block to use the functions exported by ingest.py instead: replace the use of
ingest_module.PolicyIngestion() and its ingest_all_policies method with a direct
call to ingest_module.ingest_all(policies_dir=policies_dir) (or
ingest_module.ingest_policy_folder if you intend to ingest a single folder),
keeping the existing policies_dir variable and logger/exception handling intact
so the call matches the ingest.py API.
🧹 Nitpick comments (5)
samples/hotel-booking-agent/services/hotel_api/ingest.py (1)
26-31: Module-level Pinecone initialization may fail at import time.The
vectorstoreis initialized globally when the module loads. If environment variables are missing or Pinecone is unreachable, import will fail with an unhandled exception. Consider lazy initialization or wrapping in a function.samples/hotel-booking-agent/agent/openapi.yaml (1)
10-11: Update placeholder production server URL.The production server URL
https://api.yourcompany.comis a placeholder. Update it to the actual deployment URL or remove it if not applicable for a sample.samples/hotel-booking-agent/agent/app.py (1)
65-84: Consider adding error handling for agent invocation.The
agent_graph.invoke()call and result access (result["messages"][-1]) have no try/except. If the graph fails or returns an unexpected structure, the endpoint will return a 500 with no useful context.💡 Example error handling
+from fastapi.responses import JSONResponse + `@app.post`("/chat", response_model=ChatResponse) def chat(request: ChatRequest, http_request: Request) -> ChatResponse: session_id = request.sessionId user_id, user_name = _extract_user_from_payload(request) wrapped_message = _wrap_user_message( request.message, user_id, user_name, ) thread_id = f"{user_id}:{session_id}" - result = agent_graph.invoke( - {"messages": [HumanMessage(content=wrapped_message)]}, - config={ - "recursion_limit": 50, - "configurable": {"thread_id": thread_id}, - }, - ) - - last_message = result["messages"][-1] - return ChatResponse(message=last_message.content) + try: + result = agent_graph.invoke( + {"messages": [HumanMessage(content=wrapped_message)]}, + config={ + "recursion_limit": 50, + "configurable": {"thread_id": thread_id}, + }, + ) + last_message = result["messages"][-1] + return ChatResponse(message=last_message.content) + except Exception as e: + logging.exception("Agent invocation failed") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to process chat request", + )samples/hotel-booking-agent/services/hotel_api/service.py (1)
94-96: Replace deprecated@app.on_event("startup")with FastAPI'slifespancontext manager.
@app.on_event("startup")is deprecated in FastAPI. Use thelifespanpattern instead:💡 Recommended migration
+from contextlib import asynccontextmanager + +@asynccontextmanager +async def lifespan(app: FastAPI): + _ensure_policy_index() + yield + -app = FastAPI(title="Hotel Booking API") +app = FastAPI(title="Hotel Booking API", lifespan=lifespan) -@app.on_event("startup") -def bootstrap_policy_index() -> None: - _ensure_policy_index()samples/hotel-booking-agent/services/hotel_api/openapi.yaml (1)
148-158: Add explicitformat: dateto date query params.This improves validation and client generation consistency across search/details/availability endpoints.
📅 Example (replicate for all date fields)
- name: checkInDate in: query required: false schema: type: string + format: dateAlso applies to: 243-253, 275-285
| openapi: 3.1.0 | ||
| info: | ||
| title: Hotel Booking Agent | ||
| version: 0.1.0 | ||
| description: AI-powered travel planning agent that provides personalized travel recommendations and itineraries | ||
|
|
||
| servers: | ||
| - url: http://localhost:9090 | ||
| description: Local development server | ||
| - url: https://api.yourcompany.com | ||
| description: Production server | ||
|
|
||
| paths: | ||
| /chat: | ||
| post: | ||
| summary: Send Message to Agent | ||
| description: Send a chat message to the AI agent and receive travel recommendations, itineraries, or answers to travel-related questions | ||
| operationId: chat_chat_post | ||
| requestBody: | ||
| required: true | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ChatRequest" | ||
| examples: | ||
| simple_query: | ||
| summary: Simple travel query | ||
| value: | ||
| message: "Plan a 5-day trip to Tokyo" | ||
| userId: "user_123" | ||
| userName: "Traveler" | ||
| with_session: | ||
| summary: Continuing conversation | ||
| value: | ||
| message: "What about hotels near Shibuya?" | ||
| sessionId: "session_abc123" | ||
| userId: "user_123" | ||
| userName: "Traveler" | ||
| responses: | ||
| '200': | ||
| description: Successfully processed the message and generated a response | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/ChatResponse" | ||
| examples: | ||
| itinerary_response: | ||
| summary: Travel itinerary response | ||
| value: | ||
| message: "Here's a suggested 5-day Tokyo itinerary: Day 1: Explore Shibuya and Harajuku..." | ||
| hotel_response: | ||
| summary: Hotel recommendations | ||
| value: | ||
| message: "Here are some great hotels near Shibuya: 1. Shibuya Excel Hotel..." | ||
| '422': | ||
| description: Validation error - invalid request format | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/HTTPValidationError" | ||
| '500': | ||
| description: Internal server error | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| properties: | ||
| error: | ||
| type: string | ||
| example: "An unexpected error occurred" | ||
|
|
There was a problem hiding this comment.
Add security scheme for the API.
The OpenAPI spec defines no security scheme, yet the API accepts userId and handles user sessions. For production readiness, define a security scheme (e.g., Bearer token) and apply it to the /chat endpoint.
📝 Example security scheme addition
components:
securitySchemes:
BearerAuth:
type: http
scheme: bearer
bearerFormat: JWT
security:
- BearerAuth: []🧰 Tools
🪛 Checkov (3.2.334)
[high] 1-155: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-155: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/openapi.yaml` around lines 1 - 71, The
OpenAPI spec is missing a security scheme and security requirement for the /chat
endpoint; add a Bearer JWT scheme under components.securitySchemes (e.g., name
it BearerAuth) and add a global security requirement (security: - BearerAuth:
[]) and/or apply it specifically to the /chat POST operation (operationId:
chat_chat_post) so the endpoint requires the bearer token for production use.
| @@ -0,0 +1,82 @@ | |||
| # Hotel Booking Agent | |||
|
|
|||
| Minimal Python stack for the travel planner agent, plus optional frontend. | |||
There was a problem hiding this comment.
Inconsistent description.
The README states "Minimal Python stack for the travel planner agent" but this is the Hotel Booking Agent. Update the description to match.
📝 Suggested fix
-Minimal Python stack for the travel planner agent, plus optional frontend.
+Minimal Python stack for the hotel booking agent, plus optional frontend.📝 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.
| Minimal Python stack for the travel planner agent, plus optional frontend. | |
| Minimal Python stack for the hotel booking agent, plus optional frontend. |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/README.md` at line 3, Update the README
description line that currently reads "Minimal Python stack for the travel
planner agent, plus optional frontend." to accurately describe the project as
the Hotel Booking Agent (for example: "Minimal Python stack for the Hotel
Booking Agent, plus optional frontend."); locate and edit the top-level
description string in README.md to ensure the agent name matches the repository
content.
| /bookings: | ||
| get: | ||
| summary: List bookings for the caller | ||
| parameters: | ||
| - name: x-user-id | ||
| in: header | ||
| required: false | ||
| schema: | ||
| type: string | ||
| - name: userId | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: string | ||
| responses: | ||
| "200": | ||
| description: List of bookings | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: array | ||
| items: | ||
| $ref: "#/components/schemas/Booking" | ||
| post: | ||
| summary: Create a booking | ||
| parameters: | ||
| - name: x-user-id | ||
| in: header | ||
| required: false | ||
| schema: | ||
| type: string | ||
| requestBody: | ||
| required: true | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/BookingCreateRequest" | ||
| responses: | ||
| "201": | ||
| description: Booking created | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/BookingCreateResponse" | ||
| /bookings/{booking_id}: | ||
| get: | ||
| summary: Get a booking by id | ||
| parameters: | ||
| - name: booking_id | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| - name: x-user-id | ||
| in: header | ||
| required: false | ||
| schema: | ||
| type: string | ||
| - name: userId | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: string | ||
| responses: |
There was a problem hiding this comment.
Avoid client‑controlled user identity on booking endpoints.
userId is caller‑supplied and the x-user-id header is optional, which makes it easy to spoof user identity for booking data. If this API is user-facing, identity should come from a trusted context (e.g., required header from auth or server-side session), and the userId query param should be removed or clearly marked internal.
🔒 Suggested tightening (apply similarly to other booking operations)
get:
summary: List bookings for the caller
parameters:
- name: x-user-id
in: header
- required: false
+ required: true
schema:
type: string
- - name: userId
- in: query
- required: false
- schema:
- type: stringAlso applies to: 67-138
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 43-47: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/openapi.yaml` around lines 23
- 86, The booking endpoints (/bookings and /bookings/{booking_id}) currently
accept a caller-controlled userId query parameter and an optional x-user-id
header; remove the userId query parameter from the OpenAPI operations (and any
other booking-related operations) and make caller identity come from a trusted
source — either mark x-user-id as required only if it is injected by your auth
layer or replace it with your standardized auth header/identity mechanism (e.g.,
Authorization or an internal-required header populated by the server). Update
the schemas/references that expect caller-supplied userId (e.g., Booking,
BookingCreateRequest, BookingCreateResponse) to rely on server-side identity
resolution instead, and apply the same change to all booking routes in this
spec.
| Booking: | ||
| type: object | ||
| properties: | ||
| bookingId: | ||
| type: string | ||
| hotelId: | ||
| type: string | ||
| hotelName: | ||
| type: string | ||
| rooms: | ||
| type: array | ||
| items: | ||
| type: object | ||
| userId: | ||
| type: string | ||
| checkInDate: | ||
| type: string | ||
| checkOutDate: | ||
| type: string | ||
| numberOfGuests: | ||
| type: integer | ||
| primaryGuest: | ||
| type: object | ||
| pricing: | ||
| type: array | ||
| items: | ||
| type: object | ||
| bookingStatus: | ||
| type: string | ||
| bookingDate: | ||
| type: string | ||
| confirmationNumber: | ||
| type: string | ||
| specialRequests: | ||
| type: string | ||
| updatedAt: | ||
| type: string | ||
| cancelledAt: | ||
| type: string | ||
| BookingCreateRequest: | ||
| type: object | ||
| properties: | ||
| hotelId: | ||
| type: string | ||
| hotelName: | ||
| type: string | ||
| rooms: | ||
| type: array | ||
| items: | ||
| type: object | ||
| checkInDate: | ||
| type: string | ||
| checkOutDate: | ||
| type: string | ||
| numberOfGuests: | ||
| type: integer | ||
| primaryGuest: | ||
| type: object | ||
| specialRequests: | ||
| type: string | ||
| required: | ||
| - hotelId | ||
| - hotelName | ||
| - rooms | ||
| - checkInDate | ||
| - checkOutDate | ||
| - numberOfGuests | ||
| - primaryGuest | ||
| BookingUpdateRequest: | ||
| type: object | ||
| properties: | ||
| hotelId: | ||
| type: string | ||
| hotelName: | ||
| type: string | ||
| rooms: | ||
| type: array | ||
| items: | ||
| type: object | ||
| checkInDate: | ||
| type: string | ||
| checkOutDate: | ||
| type: string | ||
| numberOfGuests: | ||
| type: integer | ||
| primaryGuest: | ||
| type: object | ||
| specialRequests: | ||
| type: string |
There was a problem hiding this comment.
Add numberOfRooms to booking schemas to match tooling payloads.
Booking creation/update payloads include numberOfRooms, but the schema omits it, so generated clients won’t send it. Align the contract with the payload shape.
🧾 Suggested schema alignment
Booking:
type: object
properties:
numberOfGuests:
type: integer
+ numberOfRooms:
+ type: integer
@@
BookingCreateRequest:
type: object
properties:
numberOfGuests:
type: integer
+ numberOfRooms:
+ type: integer
required:
- numberOfGuests
+ - numberOfRooms
@@
BookingUpdateRequest:
type: object
properties:
numberOfGuests:
type: integer
+ numberOfRooms:
+ type: integer📝 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.
| Booking: | |
| type: object | |
| properties: | |
| bookingId: | |
| type: string | |
| hotelId: | |
| type: string | |
| hotelName: | |
| type: string | |
| rooms: | |
| type: array | |
| items: | |
| type: object | |
| userId: | |
| type: string | |
| checkInDate: | |
| type: string | |
| checkOutDate: | |
| type: string | |
| numberOfGuests: | |
| type: integer | |
| primaryGuest: | |
| type: object | |
| pricing: | |
| type: array | |
| items: | |
| type: object | |
| bookingStatus: | |
| type: string | |
| bookingDate: | |
| type: string | |
| confirmationNumber: | |
| type: string | |
| specialRequests: | |
| type: string | |
| updatedAt: | |
| type: string | |
| cancelledAt: | |
| type: string | |
| BookingCreateRequest: | |
| type: object | |
| properties: | |
| hotelId: | |
| type: string | |
| hotelName: | |
| type: string | |
| rooms: | |
| type: array | |
| items: | |
| type: object | |
| checkInDate: | |
| type: string | |
| checkOutDate: | |
| type: string | |
| numberOfGuests: | |
| type: integer | |
| primaryGuest: | |
| type: object | |
| specialRequests: | |
| type: string | |
| required: | |
| - hotelId | |
| - hotelName | |
| - rooms | |
| - checkInDate | |
| - checkOutDate | |
| - numberOfGuests | |
| - primaryGuest | |
| BookingUpdateRequest: | |
| type: object | |
| properties: | |
| hotelId: | |
| type: string | |
| hotelName: | |
| type: string | |
| rooms: | |
| type: array | |
| items: | |
| type: object | |
| checkInDate: | |
| type: string | |
| checkOutDate: | |
| type: string | |
| numberOfGuests: | |
| type: integer | |
| primaryGuest: | |
| type: object | |
| specialRequests: | |
| type: string | |
| Booking: | |
| type: object | |
| properties: | |
| bookingId: | |
| type: string | |
| hotelId: | |
| type: string | |
| hotelName: | |
| type: string | |
| rooms: | |
| type: array | |
| items: | |
| type: object | |
| userId: | |
| type: string | |
| checkInDate: | |
| type: string | |
| checkOutDate: | |
| type: string | |
| numberOfGuests: | |
| type: integer | |
| numberOfRooms: | |
| type: integer | |
| primaryGuest: | |
| type: object | |
| pricing: | |
| type: array | |
| items: | |
| type: object | |
| bookingStatus: | |
| type: string | |
| bookingDate: | |
| type: string | |
| confirmationNumber: | |
| type: string | |
| specialRequests: | |
| type: string | |
| updatedAt: | |
| type: string | |
| cancelledAt: | |
| type: string | |
| BookingCreateRequest: | |
| type: object | |
| properties: | |
| hotelId: | |
| type: string | |
| hotelName: | |
| type: string | |
| rooms: | |
| type: array | |
| items: | |
| type: object | |
| checkInDate: | |
| type: string | |
| checkOutDate: | |
| type: string | |
| numberOfGuests: | |
| type: integer | |
| numberOfRooms: | |
| type: integer | |
| primaryGuest: | |
| type: object | |
| specialRequests: | |
| type: string | |
| required: | |
| - hotelId | |
| - hotelName | |
| - rooms | |
| - checkInDate | |
| - checkOutDate | |
| - numberOfGuests | |
| - numberOfRooms | |
| - primaryGuest | |
| BookingUpdateRequest: | |
| type: object | |
| properties: | |
| hotelId: | |
| type: string | |
| hotelName: | |
| type: string | |
| rooms: | |
| type: array | |
| items: | |
| type: object | |
| checkInDate: | |
| type: string | |
| checkOutDate: | |
| type: string | |
| numberOfGuests: | |
| type: integer | |
| numberOfRooms: | |
| type: integer | |
| primaryGuest: | |
| type: object | |
| specialRequests: | |
| type: string |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/openapi.yaml` around lines 306
- 394, The schemas Booking, BookingCreateRequest, and BookingUpdateRequest are
missing the numberOfRooms field; add a numberOfRooms property (type: integer) to
each of these definitions and include numberOfRooms in the
BookingCreateRequest.required array so generated clients will send it; update
the properties for Booking (Booking.numberOfRooms), BookingCreateRequest
(BookingCreateRequest.numberOfRooms) and BookingUpdateRequest
(BookingUpdateRequest.numberOfRooms) accordingly.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@samples/hotel-booking-agent/README.md`:
- Line 42: The kubectl port-forward command in README references the wrong
service name "travel-planner-agent-is"; update that service name to the Hotel
Booking Agent service (e.g., replace "travel-planner-agent-is" with
"hotel-booking-agent-is") so the line reads the correct kubectl port-forward
invocation using svc/hotel-booking-agent-is; ensure the port-forward command
still maps 9090:80 and keep the namespace unchanged.
In `@samples/hotel-booking-agent/services/hotel_api/.env.example`:
- Around line 1-6: The .env.example contains quoted values and an order that
trips dotenv-linter; remove the surrounding quotes from values (e.g.,
PINECONE_SERVICE_URL, PINECONE_API_KEY, PINECONE_INDEX_NAME, PINECONE_NAMESPACE,
OPENAI_API_KEY, OPENAI_EMBEDDING_MODEL) and reorder the keys into a consistent,
logical order (group Pinecone keys together then OpenAI keys) so dotenv-linter
accepts the file; ensure no trailing spaces or comments break parsing and keep
empty values unquoted.
In `@samples/hotel-booking-agent/services/hotel_api/ingest.py`:
- Around line 66-89: ensure_policy_index currently skips creation and will call
PolicyIngestion.ingest_all_policies() even when the Pinecone index is missing;
update ensure_policy_index to explicitly create the Pinecone index when
index_name is not present by calling the Pinecone client create_index (use the
correct embedding dimension for your embedding model) before instantiating
PolicyIngestion, e.g., check index_names = pc.list_indexes().names(), and if
index_name not in index_names call pc.create_index(name=index_name,
dimension=<EMBEDDING_DIM>) (handle and log any creation errors via
logger.exception), then proceed to ingestion; reference ensure_policy_index,
Pinecone (pc), index_name, and PolicyIngestion.ingest_all_policies when making
the change.
In `@samples/hotel-booking-agent/services/hotel_api/openapi.yaml`:
- Around line 1-7: The OpenAPI spec currently has no security definitions; add a
components.securitySchemes entry (e.g., an ApiKeyAuth or BearerAuth scheme) and
then apply a global security requirement by adding a top-level security: -
<schemeName>: [] (or declare per-operation under paths/* if needed). Update
components.securitySchemes with the chosen scheme name and type (for example
"BearerAuth" type: http scheme: bearer) and reference that exact scheme name in
the security requirement so clients/tooling enforce the actual auth layer.
🧹 Nitpick comments (5)
samples/hotel-booking-agent/README.md (1)
77-77: Sample request doesn't align with hotel booking use case.The sample message "Plan a 3-day trip to Tokyo" is more appropriate for a general travel planner than a hotel booking agent. Consider using a hotel-specific example query.
📝 Suggested fix
- -d '{"message":"Plan a 3-day trip to Tokyo","sessionId":"session_abc123","userId":"user_123","userName":"Traveler"}' + -d '{"message":"Find hotels in Tokyo for 3 nights starting March 15","sessionId":"session_abc123","userId":"user_123","userName":"Traveler"}'samples/hotel-booking-agent/services/hotel_api/ingest.py (3)
6-6: Unused import:Any.The
Anytype fromtypingis imported but never used in the module.♻️ Proposed fix
-from typing import Any
29-37: Consider validating required environment variables.
OpenAIEmbeddingsandPineconeVectorStorereceiveos.getenv()values that could beNoneif the environment variables are unset. Whileensure_policy_index()checksPINECONE_API_KEY, the other required variables (OPENAI_EMBEDDING_MODEL,PINECONE_INDEX_NAME,PINECONE_SERVICE_URL) are not validated before use. For a sample app this is acceptable, but consider adding validation or informative error messages.
51-64: Inconsistent output:print()vslogger.The module defines a
logger(line 17) but usesprint()for output in_ingest_policy_folder(). Consider usinglogger.info()for consistency, which also enables log level control.♻️ Proposed fix
if not pdf_path.exists() or not metadata_path.exists(): - print(f"Skipping {folder.name}: missing files") + logger.info("Skipping %s: missing files", folder.name) returnself._vectorstore.add_documents(chunks) - print(f"✓ Ingested {folder.name}") + logger.info("Ingested %s", folder.name)samples/hotel-booking-agent/services/hotel_api/service.py (1)
39-41:@app.on_event("startup")is deprecated. FastAPI recommends using thelifespancontext manager instead. While this code works, the project is on FastAPI 0.115.0, which fully supports the modern pattern.♻️ Proposed fix using lifespan
+from contextlib import asynccontextmanager + +@asynccontextmanager +async def lifespan(app: FastAPI): + ensure_policy_index() + yield + -app = FastAPI(title="Hotel Booking API") +app = FastAPI(title="Hotel Booking API", lifespan=lifespan)Then remove the
@app.on_event("startup")decorated function.
| Run this inside the WSO2-AMP dev container to expose the agent on `localhost:9090`: | ||
|
|
||
| ```bash | ||
| kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80 |
There was a problem hiding this comment.
Update the service name to match the hotel booking agent.
The kubectl command references travel-planner-agent-is, but this README is for the Hotel Booking Agent. Update the service name to match the actual deployed service name.
📝 Suggested fix
-kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80
+kubectl -n dp-default-default-default-ccb66d74 port-forward svc/hotel-booking-agent-is 9090:80📝 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.
| kubectl -n dp-default-default-default-ccb66d74 port-forward svc/travel-planner-agent-is 9090:80 | |
| kubectl -n dp-default-default-default-ccb66d74 port-forward svc/hotel-booking-agent-is 9090:80 |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/README.md` at line 42, The kubectl port-forward
command in README references the wrong service name "travel-planner-agent-is";
update that service name to the Hotel Booking Agent service (e.g., replace
"travel-planner-agent-is" with "hotel-booking-agent-is") so the line reads the
correct kubectl port-forward invocation using svc/hotel-booking-agent-is; ensure
the port-forward command still maps 9090:80 and keep the namespace unchanged.
| PINECONE_SERVICE_URL="https://your-index-xxxxxx.svc.your-region.pinecone.io" | ||
| PINECONE_API_KEY="your-pinecone-api-key" | ||
| PINECONE_INDEX_NAME="hotel-policies" | ||
| PINECONE_NAMESPACE="" | ||
| OPENAI_API_KEY="your-openai-api-key" | ||
| OPENAI_EMBEDDING_MODEL="text-embedding-3-small" |
There was a problem hiding this comment.
Normalize .env.example values to avoid dotenv-linter failures.
The linter flags quoted values and key order; if it runs in CI, this will fail. Consider unquoting and ordering keys.
🧩 Suggested normalization
-PINECONE_SERVICE_URL="https://your-index-xxxxxx.svc.your-region.pinecone.io"
-PINECONE_API_KEY="your-pinecone-api-key"
-PINECONE_INDEX_NAME="hotel-policies"
-PINECONE_NAMESPACE=""
-OPENAI_API_KEY="your-openai-api-key"
-OPENAI_EMBEDDING_MODEL="text-embedding-3-small"
+OPENAI_API_KEY=your-openai-api-key
+OPENAI_EMBEDDING_MODEL=text-embedding-3-small
+PINECONE_API_KEY=your-pinecone-api-key
+PINECONE_INDEX_NAME=hotel-policies
+PINECONE_NAMESPACE=
+PINECONE_SERVICE_URL=https://your-index-xxxxxx.svc.your-region.pinecone.io📝 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.
| PINECONE_SERVICE_URL="https://your-index-xxxxxx.svc.your-region.pinecone.io" | |
| PINECONE_API_KEY="your-pinecone-api-key" | |
| PINECONE_INDEX_NAME="hotel-policies" | |
| PINECONE_NAMESPACE="" | |
| OPENAI_API_KEY="your-openai-api-key" | |
| OPENAI_EMBEDDING_MODEL="text-embedding-3-small" | |
| OPENAI_API_KEY=your-openai-api-key | |
| OPENAI_EMBEDDING_MODEL=text-embedding-3-small | |
| PINECONE_API_KEY=your-pinecone-api-key | |
| PINECONE_INDEX_NAME=hotel-policies | |
| PINECONE_NAMESPACE= | |
| PINECONE_SERVICE_URL=https://your-index-xxxxxx.svc.your-region.pinecone.io |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The PINECONE_API_KEY key should go before the PINECONE_SERVICE_URL key
(UnorderedKey)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 3-3: [UnorderedKey] The PINECONE_INDEX_NAME key should go before the PINECONE_SERVICE_URL key
(UnorderedKey)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [UnorderedKey] The PINECONE_NAMESPACE key should go before the PINECONE_SERVICE_URL key
(UnorderedKey)
[warning] 5-5: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 5-5: [UnorderedKey] The OPENAI_API_KEY key should go before the PINECONE_API_KEY key
(UnorderedKey)
[warning] 6-6: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 6-6: [UnorderedKey] The OPENAI_EMBEDDING_MODEL key should go before the PINECONE_API_KEY key
(UnorderedKey)
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/.env.example` around lines 1 -
6, The .env.example contains quoted values and an order that trips
dotenv-linter; remove the surrounding quotes from values (e.g.,
PINECONE_SERVICE_URL, PINECONE_API_KEY, PINECONE_INDEX_NAME, PINECONE_NAMESPACE,
OPENAI_API_KEY, OPENAI_EMBEDDING_MODEL) and reorder the keys into a consistent,
logical order (group Pinecone keys together then OpenAI keys) so dotenv-linter
accepts the file; ensure no trailing spaces or comments break parsing and keep
empty values unquoted.
| def ensure_policy_index() -> None: | ||
| pinecone_api_key = os.getenv("PINECONE_API_KEY") | ||
| if not pinecone_api_key: | ||
| logger.info("PINECONE_API_KEY not set; skipping policy ingest") | ||
| return | ||
|
|
||
| index_name = os.getenv("PINECONE_INDEX_NAME", "hotelbookingdb") | ||
| try: | ||
| pc = Pinecone(api_key=pinecone_api_key) | ||
| index_names = pc.list_indexes().names() | ||
| if index_name in index_names: | ||
| logger.info("policy index '%s' already exists; skipping ingest", index_name) | ||
| return | ||
| except Exception: | ||
| logger.exception("failed to check Pinecone index; skipping policy ingest") | ||
| return | ||
|
|
||
| policies_dir = os.getenv("POLICIES_DIRS") or str(DEFAULT_POLICIES_DIR) | ||
| try: | ||
| ingestion = PolicyIngestion() | ||
| ingestion.ingest_all_policies(policies_dir=policies_dir) | ||
| logger.info("policy ingest completed") | ||
| except Exception: | ||
| logger.exception("policy ingest failed") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
LangChain PineconeVectorStore auto-create index add_documents
💡 Result:
LangChain’s PineconeVectorStore does not auto-create a Pinecone index for you. The LangChain + Pinecone docs explicitly say you must pass the name of an existing index to PineconeVectorStore(...), from_documents(...), or from_texts(...), and then add_documents(...) / add_texts(...) will embed + upsert records into that existing index. (docs.pinecone.io)
What to do instead (create index if missing, then add_documents)
import os, time
from pinecone import Pinecone, ServerlessSpec
from langchain_openai import OpenAIEmbeddings
from langchain_pinecone import PineconeVectorStore
pc = Pinecone(api_key=os.environ["PINECONE_API_KEY"])
index_name = "my-index"
dimension = 1536 # must match your embedding model output dimension
if not pc.has_index(index_name):
pc.create_index(
name=index_name,
dimension=dimension,
metric="cosine",
spec=ServerlessSpec(cloud="aws", region="us-east-1"),
)
while not pc.describe_index(index_name).status["ready"]:
time.sleep(1)
embeddings = OpenAIEmbeddings()
vs = PineconeVectorStore(index_name=index_name, embedding=embeddings)
# docs is a list[langchain_core.documents.Document]
vs.add_documents(docs)This “create if missing” pattern is shown in LangChain’s Pinecone integration docs / API reference. (python.langchain.com)
Notes that commonly trip people up
PineconeVectorStore.from_documents(...)/.from_texts(...)also require an existing index; they add records and return a vectorstore, but they still don’t create the index itself. (docs.pinecone.io)add_documents(...)supports explicit IDs (optional):(python.langchain.com)ids = ["1","2","3"] vs.add_documents(documents=docs, ids=ids)
Citations:
- 1: https://docs.pinecone.io/integrations/langchain?utm_source=openai
- 2: https://python.langchain.com/docs/integrations/vectorstores/pinecone/?utm_source=openai
- 3: https://docs.pinecone.io/integrations/langchain?utm_source=openai
- 4: https://python.langchain.com/api_reference/pinecone/vectorstores/langchain_pinecone.vectorstores.PineconeVectorStore.html?utm_source=openai
🏁 Script executed:
# Find PolicyIngestion implementation to understand how indexes are created
find . -type f -name "*.py" | xargs rg -l "class PolicyIngestion" | head -5Repository: wso2/agent-manager
Length of output: 120
🏁 Script executed:
# Look at the ingest.py file to understand the full context
cat -n samples/hotel-booking-agent/services/hotel_api/ingest.pyRepository: wso2/agent-manager
Length of output: 3755
Fix: ensure_policy_index() does not create the index; it will fail on fresh setups.
The function checks if the index exists and skips if it does, but if the index is missing, it proceeds to ingest via PolicyIngestion() without creating it first. Since PineconeVectorStore requires an existing index and does not auto-create, calling add_documents() (line 63) will fail when the index is absent.
To fix, the function must explicitly create the index (with dimensions matching the embedding model) if it doesn't exist, before proceeding to ingest. Without this, any fresh deployment will fail silently due to the broad except Exception handler.
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/ingest.py` around lines 66 -
89, ensure_policy_index currently skips creation and will call
PolicyIngestion.ingest_all_policies() even when the Pinecone index is missing;
update ensure_policy_index to explicitly create the Pinecone index when
index_name is not present by calling the Pinecone client create_index (use the
correct embedding dimension for your embedding model) before instantiating
PolicyIngestion, e.g., check index_names = pc.list_indexes().names(), and if
index_name not in index_names call pc.create_index(name=index_name,
dimension=<EMBEDDING_DIM>) (handle and log any creation errors via
logger.exception), then proceed to ingestion; reference ensure_policy_index,
Pinecone (pc), index_name, and PolicyIngestion.ingest_all_policies when making
the change.
| openapi: 3.0.3 | ||
| info: | ||
| title: Hotel Booking API | ||
| version: "1.0.0" | ||
| servers: | ||
| - url: http://localhost:9091 | ||
| paths: |
There was a problem hiding this comment.
Define and apply API security requirements.
The spec documents no security at all, so clients and tooling will treat these endpoints as public. Add a global (or per‑operation) security requirement and declare the scheme under components.securitySchemes to match the actual auth layer.
🔐 Example skeleton (adjust to your real auth)
openapi: 3.0.3
info:
title: Hotel Booking API
version: "1.0.0"
servers:
- url: http://localhost:9091
+security:
+ - bearerAuth: []
@@
components:
+ securitySchemes:
+ bearerAuth:
+ type: http
+ scheme: bearer
+ bearerFormat: JWT
schemas:Also applies to: 320-321
🧰 Tools
🪛 Checkov (3.2.334)
[high] 1-487: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-487: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/openapi.yaml` around lines 1 -
7, The OpenAPI spec currently has no security definitions; add a
components.securitySchemes entry (e.g., an ApiKeyAuth or BearerAuth scheme) and
then apply a global security requirement by adding a top-level security: -
<schemeName>: [] (or declare per-operation under paths/* if needed). Update
components.securitySchemes with the chosen scheme name and type (for example
"BearerAuth" type: http scheme: bearer) and reference that exact scheme name in
the security requirement so clients/tooling enforce the actual auth layer.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@samples/hotel-booking-agent/agent/.env.example`:
- Around line 1-11: Add the missing OPENAI_EMBEDDING_MODEL variable to the .env
example so users can configure the embedding model used by the app;
specifically, add a line for OPENAI_EMBEDDING_MODEL= (set to the same default
value used in config.py) so it matches the embedding model lookup in config.py
(the OPENAI_EMBEDDING_MODEL setting) and makes the option discoverable.
In `@samples/hotel-booking-agent/README.md`:
- Around line 23-33: README currently marks many env vars as "Required" though
config.py provides defaults; update the README's "Agent environment variables"
section to list only OPENAI_API_KEY, PINECONE_API_KEY, and PINECONE_SERVICE_URL
as Required and mark OPENAI_MODEL, OPENAI_EMBEDDING_MODEL, WEATHER_API_BASE_URL,
and HOTEL_API_BASE_URL as Optional with their default values per config.py
(OPENAI_MODEL="gpt-4o-mini", OPENAI_EMBEDDING_MODEL="text-embedding-3-small",
HOTEL_API_BASE_URL="http://localhost:9091"); also change the documented default
for WEATHER_API_BASE_URL to use HTTPS (https://api.weatherapi.com/v1) to match
the security guidance in config.py. Ensure variable names exactly match those in
config.py so readers can cross-reference.
| OPENAI_API_KEY= | ||
| OPENAI_MODEL= | ||
|
|
||
| HOTEL_API_BASE_URL= | ||
|
|
||
| PINECONE_API_KEY= | ||
| PINECONE_SERVICE_URL= | ||
| PINECONE_INDEX_NAME= | ||
|
|
||
| WEATHER_API_KEY= | ||
| WEATHER_API_BASE_URL= |
There was a problem hiding this comment.
Missing OPENAI_EMBEDDING_MODEL variable.
The template is missing OPENAI_EMBEDDING_MODEL which is used in config.py (Line 38). While it has a default value, including it in the template helps users discover all configurable options.
📝 Proposed fix
OPENAI_API_KEY=
OPENAI_MODEL=
+OPENAI_EMBEDDING_MODEL=
HOTEL_API_BASE_URL=📝 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.
| OPENAI_API_KEY= | |
| OPENAI_MODEL= | |
| HOTEL_API_BASE_URL= | |
| PINECONE_API_KEY= | |
| PINECONE_SERVICE_URL= | |
| PINECONE_INDEX_NAME= | |
| WEATHER_API_KEY= | |
| WEATHER_API_BASE_URL= | |
| OPENAI_API_KEY= | |
| OPENAI_MODEL= | |
| OPENAI_EMBEDDING_MODEL= | |
| HOTEL_API_BASE_URL= | |
| PINECONE_API_KEY= | |
| PINECONE_SERVICE_URL= | |
| PINECONE_INDEX_NAME= | |
| WEATHER_API_KEY= | |
| WEATHER_API_BASE_URL= |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 8-8: [UnorderedKey] The PINECONE_INDEX_NAME key should go before the PINECONE_SERVICE_URL key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The WEATHER_API_BASE_URL key should go before the WEATHER_API_KEY key
(UnorderedKey)
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/.env.example` around lines 1 - 11, Add the
missing OPENAI_EMBEDDING_MODEL variable to the .env example so users can
configure the embedding model used by the app; specifically, add a line for
OPENAI_EMBEDDING_MODEL= (set to the same default value used in config.py) so it
matches the embedding model lookup in config.py (the OPENAI_EMBEDDING_MODEL
setting) and makes the option discoverable.
| **Agent environment variables** | ||
| Required: | ||
| - `OPENAI_API_KEY` | ||
| - `PINECONE_API_KEY` | ||
| - `PINECONE_SERVICE_URL` | ||
| - `OPENAI_MODEL` | ||
| - `OPENAI_EMBEDDING_MODEL` | ||
| - `WEATHER_API_KEY` | ||
| - `WEATHER_API_BASE_URL` (default: `http://api.weatherapi.com/v1`) | ||
| - `HOTEL_API_BASE_URL` (default: `http://localhost:9091`) | ||
|
|
There was a problem hiding this comment.
Correct the "Required" vs "Optional" labeling for environment variables.
Several variables listed as "Required" actually have defaults in config.py:
OPENAI_MODELdefaults to"gpt-4o-mini"OPENAI_EMBEDDING_MODELdefaults to"text-embedding-3-small"WEATHER_API_BASE_URLdefaults to the WeatherAPI endpointHOTEL_API_BASE_URLdefaults to"http://localhost:9091"
Only OPENAI_API_KEY, PINECONE_API_KEY, and PINECONE_SERVICE_URL are truly required (they raise ValueError if missing).
Additionally, WEATHER_API_BASE_URL default should use HTTPS for consistency with the security recommendation in config.py.
📝 Suggested fix
**Agent environment variables**
Required:
- `OPENAI_API_KEY`
- `PINECONE_API_KEY`
- `PINECONE_SERVICE_URL`
-- `OPENAI_MODEL`
-- `OPENAI_EMBEDDING_MODEL`
-- `WEATHER_API_KEY`
-- `WEATHER_API_BASE_URL` (default: `http://api.weatherapi.com/v1`)
-- `HOTEL_API_BASE_URL` (default: `http://localhost:9091`)
+
+Optional (defaults applied if unset):
+- `OPENAI_MODEL` (default: `gpt-4o-mini`)
+- `OPENAI_EMBEDDING_MODEL` (default: `text-embedding-3-small`)
+- `PINECONE_INDEX_NAME` (default: `hotel-policies`)
+- `WEATHER_API_KEY` (enables weather tool if set)
+- `WEATHER_API_BASE_URL` (default: `https://api.weatherapi.com/v1`)
+- `HOTEL_API_BASE_URL` (default: `http://localhost:9091`)🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/README.md` around lines 23 - 33, README currently
marks many env vars as "Required" though config.py provides defaults; update the
README's "Agent environment variables" section to list only OPENAI_API_KEY,
PINECONE_API_KEY, and PINECONE_SERVICE_URL as Required and mark OPENAI_MODEL,
OPENAI_EMBEDDING_MODEL, WEATHER_API_BASE_URL, and HOTEL_API_BASE_URL as Optional
with their default values per config.py (OPENAI_MODEL="gpt-4o-mini",
OPENAI_EMBEDDING_MODEL="text-embedding-3-small",
HOTEL_API_BASE_URL="http://localhost:9091"); also change the documented default
for WEATHER_API_BASE_URL to use HTTPS (https://api.weatherapi.com/v1) to match
the security guidance in config.py. Ensure variable names exactly match those in
config.py so readers can cross-reference.
Clarified deployment instructions for the agent and cloud hosting.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@samples/hotel-booking-agent/agent/app.py`:
- Around line 33-40: The CORS middleware is using a hardcoded
allow_origins=["*"] which bypasses the configured settings; change the
app.add_middleware call to read origins from
Settings.from_env().cors_allow_origins (or the Settings instance you create) and
pass that list into CORSMiddleware's allow_origins; ensure you still set
allow_credentials, allow_methods, allow_headers, and max_age as before and
handle the case where cors_allow_origins may be None or empty by falling back to
the default list from Settings.cors_allow_origins.
- Around line 66-82: The chat endpoint builds a thread_id using sessionId which
can be an empty string; update validation to ensure sessionId (and userId) are
non-empty before constructing thread_id in the chat function (or add a safe
fallback) — e.g., enforce non-empty via Pydantic validators on ChatRequest
(validate sessionId and userId with a field_validator/method) or add an explicit
check in chat after extracting session_id and user_id to raise an error or
substitute a fallback value before forming thread_id.
In `@samples/hotel-booking-agent/agent/tools.py`:
- Around line 516-521: The "this weekend" branch incorrectly calls
_next_weekday(5, now) for Sundays; update the logic so Saturday/Sunday represent
the current weekend: if now.weekday() == 5 (Saturday) set saturday = now and
sunday = now + timedelta(days=1); if now.weekday() == 6 (Sunday) set saturday =
now - timedelta(days=1) and sunday = now; otherwise (Mon–Fri) compute upcoming
Saturday with _next_weekday(5, now) or now + timedelta(days=(5 - now.weekday()))
and then set sunday = saturday + timedelta(days=1); keep uses of
_add("this_weekend_start", saturday) and _add("this_weekend_end", sunday).
- Around line 165-169: The current whitespace-based heuristic (clean_id and " "
not in clean_id) lets single-word hotel names bypass resolution; replace that
with a robust format check by implementing and using an is_valid_hotel_id helper
(or inline validation) that matches the real hotel ID patterns (e.g., required
prefix like "hotel_", UUID regex, or numeric ID rules) and use it when deciding
to set resolved_id vs call _resolve_hotel_id; update the logic in the current
snippet and the other sites mentioned (get_hotel_info_tool,
check_hotel_availability_tool, and query_hotel_policy_tool) to call the same
validator so single-word names are routed to _resolve_hotel_id while only true
IDs are accepted directly.
In `@samples/hotel-booking-agent/services/hotel_api/booking.py`:
- Around line 89-95: Exception handlers in booking persistence currently return
error dictionaries but keep HTTP 200/201 status codes; change them to raise
HTTPException (from fastapi) with an appropriate 5xx status (500 or 503) and
include a descriptive detail message and the error info. For example, inside
create_booking (and the other affected handlers: get_booking, update_booking,
delete_booking, list_bookings) catch Exception, call logger.exception(...) as
before, then raise HTTPException(status_code=500, detail="Booking persistence
failed") (or 503 if you prefer transient errors) instead of returning
_error_response; ensure imports include fastapi.HTTPException and remove/replace
the old return paths so the correct HTTP status is sent.
- Around line 64-87: Add a FastAPI dependency (e.g., get_current_user) that
extracts and validates the authenticated user identity from the request
(JWT/OAuth2 token or request context) and raise HTTPException(401) on failure;
then replace all uses of client-supplied userId with the authenticated user's id
by injecting this dependency into endpoints: create_booking (replace
payload.get("userId", "guest") in create_booking), the GET /bookings handler
that reads query param userId, GET /bookings/{booking_id} that reads query param
userId, update handler that uses payload.get("userId", "guest") (e.g., in
update_booking), and DELETE /bookings/{booking_id} that reads query param
userId; ensure each endpoint signature includes current_user: dict =
Depends(get_current_user) (or typed user model) and use current_user["id"] (or
current_user.id) to authorize access to and modification of bookings, returning
403 if the authenticated user is not the owner.
In `@samples/hotel-booking-agent/services/hotel_api/storage/bookings.json`:
- Around line 17-22: The sample bookings file contains PII in fields like
primaryGuest.firstName, primaryGuest.lastName, primaryGuest.email,
primaryGuest.phoneNumber and nationality across multiple entries; update each
booking record (e.g., the object under "primaryGuest" and any top-level
"nationality") to use clearly fictitious placeholders (e.g., firstName/lastName
-> "Guest", email -> "[email protected]", phoneNumber -> "555-0100") or
remove those fields entirely, and apply the same replacement to the other
occurrences noted (around the other entries at the commented ranges).
- Around line 27-55: The sample booking objects use inconsistent shapes for
specialRequests and primaryGuest which will break typed clients; update all
booking entries (e.g., the object with "bookingId": "BK99E89980" and the entries
around lines 71-79) to match the OpenAPI schema by making specialRequests an
object (or null) with the defined optional fields (e.g., otherRequests,
accessibility, etc.) instead of a raw string, and ensure primaryGuest is always
an object with consistent keys (firstName, lastName, email?, phone? as per
contract) or null; normalize every booking record in bookings.json to that shape
so the JSON schema is stable across records.
🧹 Nitpick comments (3)
samples/hotel-booking-agent/agent/app.py (1)
84-85: Consider defensive check for empty messages.
result["messages"][-1]assumes the graph always returns at least one message. While typically true, an empty list would cause anIndexError(500 error).🛡️ Defensive approach
- last_message = result["messages"][-1] - return ChatResponse(message=last_message.content) + messages = result.get("messages", []) + if not messages: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Agent returned no response.", + ) + return ChatResponse(message=messages[-1].content)samples/hotel-booking-agent/agent/tools.py (2)
20-90: Consider adding email validation and unifying type annotation style.The data models are well-structured. Two optional improvements:
GuestDetails.email(line 31) lacks format validation—consider usingpydantic.EmailStrto catch malformed emails early.- The file mixes
Optional[str]withstr | None—unifying on one style improves readability.♻️ Example for email validation
+from pydantic import EmailStr + class GuestDetails(BaseModel): firstName: str = Field(..., description="Primary guest first name.") lastName: str = Field(..., description="Primary guest last name.") - email: str = Field(..., description="Primary guest email address.") + email: EmailStr = Field(..., description="Primary guest email address.") phoneNumber: str = Field(..., description="Primary guest phone number.")
262-265: Simplify redundant return logic.Lines 263-265 can be simplified since both branches return
response.♻️ Simplified return
response = _call_hotel_api("GET", "/hotels/search", params=params) - if isinstance(response, dict) and response.get("error"): - return response return response
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["*"], | ||
| allow_credentials=False, | ||
| allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], | ||
| allow_headers=["Content-Type", "Authorization", "Accept", "x-user-id"], | ||
| max_age=84900, | ||
| ) |
There was a problem hiding this comment.
Use configured CORS origins instead of hardcoded wildcard.
Settings.from_env() populates cors_allow_origins from environment (defaulting to ["http://localhost:3001"]), but this is ignored in favor of hardcoded allow_origins=["*"]. This bypasses the intended security configuration.
🔐 Suggested fix
app = FastAPI(title="Hotel Booking Agent")
app.add_middleware(
CORSMiddleware,
- allow_origins=["*"],
- allow_credentials=False,
+ allow_origins=configs.cors_allow_origins,
+ allow_credentials=configs.cors_allow_credentials,
allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"],
allow_headers=["Content-Type", "Authorization", "Accept", "x-user-id"],
max_age=84900,
)📝 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.
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=["*"], | |
| allow_credentials=False, | |
| allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], | |
| allow_headers=["Content-Type", "Authorization", "Accept", "x-user-id"], | |
| max_age=84900, | |
| ) | |
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=configs.cors_allow_origins, | |
| allow_credentials=configs.cors_allow_credentials, | |
| allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], | |
| allow_headers=["Content-Type", "Authorization", "Accept", "x-user-id"], | |
| max_age=84900, | |
| ) |
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/app.py` around lines 33 - 40, The CORS
middleware is using a hardcoded allow_origins=["*"] which bypasses the
configured settings; change the app.add_middleware call to read origins from
Settings.from_env().cors_allow_origins (or the Settings instance you create) and
pass that list into CORSMiddleware's allow_origins; ensure you still set
allow_credentials, allow_methods, allow_headers, and max_age as before and
handle the case where cors_allow_origins may be None or empty by falling back to
the default list from Settings.cors_allow_origins.
| @app.post("/chat", response_model=ChatResponse) | ||
| def chat(request: ChatRequest, http_request: Request) -> ChatResponse: | ||
| session_id = request.sessionId | ||
| user_id, user_name = _extract_user_from_payload(request) | ||
| wrapped_message = _wrap_user_message( | ||
| request.message, | ||
| user_id, | ||
| user_name, | ||
| ) | ||
| thread_id = f"{user_id}:{session_id}" | ||
| result = agent_graph.invoke( | ||
| {"messages": [HumanMessage(content=wrapped_message)]}, | ||
| config={ | ||
| "recursion_limit": 50, | ||
| "configurable": {"thread_id": thread_id}, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Validate sessionId is non-empty to avoid malformed thread IDs.
While sessionId is required by Pydantic and won't be None, an empty string "" passes validation, resulting in a thread ID like "user_id:". Consider validating non-empty or using a fallback.
💡 Suggested validation
`@app.post`("/chat", response_model=ChatResponse)
def chat(request: ChatRequest, http_request: Request) -> ChatResponse:
session_id = request.sessionId
+ if not session_id:
+ raise HTTPException(
+ status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+ detail="Missing sessionId in request payload.",
+ )
user_id, user_name = _extract_user_from_payload(request)Alternatively, add Pydantic field validation:
from pydantic import field_validator
class ChatRequest(BaseModel):
message: str
sessionId: str
userId: str
userName: str | None = None
`@field_validator`("sessionId", "userId")
`@classmethod`
def must_be_non_empty(cls, v: str) -> str:
if not v.strip():
raise ValueError("must be non-empty")
return v🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/app.py` around lines 66 - 82, The chat
endpoint builds a thread_id using sessionId which can be an empty string; update
validation to ensure sessionId (and userId) are non-empty before constructing
thread_id in the chat function (or add a safe fallback) — e.g., enforce
non-empty via Pydantic validators on ChatRequest (validate sessionId and userId
with a field_validator/method) or add an explicit check in chat after extracting
session_id and user_id to raise an error or substitute a fallback value before
forming thread_id.
| clean_id = (hotel_id or "").strip() | ||
| if clean_id and " " not in clean_id: | ||
| resolved_id = clean_id | ||
| else: | ||
| resolved_id = _resolve_hotel_id(hotel_name or hotel_id) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual hotel ID format used in the hotel API
rg -n "hotelId" samples/hotel-booking-agent/ --type py -A 2 -B 2 | head -80Repository: wso2/agent-manager
Length of output: 6167
🏁 Script executed:
# Also search for hotel ID examples or patterns in the codebase
rg -n "hotel_id\|hotel-id\|hotel_code" samples/hotel-booking-agent/ --type py -A 1 | head -100Repository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
# Let's look at the structure of tools.py to verify the functions mentioned
cat -n samples/hotel-booking-agent/agent/tools.py | head -250Repository: wso2/agent-manager
Length of output: 12072
🏁 Script executed:
# Find the hotel dataset to see actual ID examples
fd -n "hotel" samples/hotel-booking-agent/ --type f | grep -E "\.(json|py)" | head -20Repository: wso2/agent-manager
Length of output: 292
🏁 Script executed:
# Check the search.py file to see how IDs are used and what format they have
cat -n samples/hotel-booking-agent/services/hotel_api/search.pyRepository: wso2/agent-manager
Length of output: 13915
🏁 Script executed:
# Check lines 270-310 in tools.py to verify the other functions mentioned
sed -n '270,310p' samples/hotel-booking-agent/agent/tools.pyRepository: wso2/agent-manager
Length of output: 1747
Fragile ID validation heuristic allows names to bypass resolution.
The " " not in clean_id check at line 166 assumes any whitespace-free input is a valid hotel ID, but this allows single-word hotel names (e.g., "Hilton", "Marriott") to bypass _resolve_hotel_id() and fail downstream with "Hotel not found" instead of being resolved by name. The same vulnerable pattern appears in get_hotel_info_tool (lines 274–277) and check_hotel_availability_tool (lines 297–300).
Additionally, the validation is inconsistent—get_hotel_info_tool includes a user_ prefix check (line 272), but query_hotel_policy_tool lacks any safeguard.
Validate against the actual hotel ID format (e.g., prefix, UUID pattern, numeric) rather than relying on whitespace detection.
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/tools.py` around lines 165 - 169, The
current whitespace-based heuristic (clean_id and " " not in clean_id) lets
single-word hotel names bypass resolution; replace that with a robust format
check by implementing and using an is_valid_hotel_id helper (or inline
validation) that matches the real hotel ID patterns (e.g., required prefix like
"hotel_", UUID regex, or numeric ID rules) and use it when deciding to set
resolved_id vs call _resolve_hotel_id; update the logic in the current snippet
and the other sites mentioned (get_hotel_info_tool,
check_hotel_availability_tool, and query_hotel_policy_tool) to call the same
validator so single-word names are routed to _resolve_hotel_id while only true
IDs are accepted directly.
| if "this weekend" in lowered: | ||
| # Upcoming Saturday/Sunday based on current week. | ||
| saturday = _next_weekday(5, now) if now.weekday() > 5 else now + timedelta(days=(5 - now.weekday())) | ||
| sunday = saturday + timedelta(days=1) | ||
| _add("this_weekend_start", saturday) | ||
| _add("this_weekend_end", sunday) |
There was a problem hiding this comment.
"this weekend" logic is incorrect when today is Saturday.
Line 518's condition now.weekday() > 5 is never true since weekday() returns 0–6 (Monday=0, Sunday=6). If today is Saturday (5), the else branch computes 5 - 5 = 0, returning today correctly. However, if today is Sunday (6), weekday() > 5 is 6 > 5 = True, so it calls _next_weekday(5, now) which returns next Saturday—likely not the intended "this weekend" behavior.
🔧 Suggested fix
if "this weekend" in lowered:
- # Upcoming Saturday/Sunday based on current week.
- saturday = _next_weekday(5, now) if now.weekday() > 5 else now + timedelta(days=(5 - now.weekday()))
+ # If today is Sat or Sun, use this weekend; otherwise find upcoming Saturday
+ if now.weekday() == 5: # Saturday
+ saturday = now
+ elif now.weekday() == 6: # Sunday
+ saturday = now - timedelta(days=1)
+ else:
+ saturday = now + timedelta(days=(5 - now.weekday()))
sunday = saturday + timedelta(days=1)🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/agent/tools.py` around lines 516 - 521, The "this
weekend" branch incorrectly calls _next_weekday(5, now) for Sundays; update the
logic so Saturday/Sunday represent the current weekend: if now.weekday() == 5
(Saturday) set saturday = now and sunday = now + timedelta(days=1); if
now.weekday() == 6 (Sunday) set saturday = now - timedelta(days=1) and sunday =
now; otherwise (Mon–Fri) compute upcoming Saturday with _next_weekday(5, now) or
now + timedelta(days=(5 - now.weekday())) and then set sunday = saturday +
timedelta(days=1); keep uses of _add("this_weekend_start", saturday) and
_add("this_weekend_end", sunday).
| @router.post("/bookings", status_code=201) | ||
| def create_booking(payload: dict[str, Any]): | ||
| user_id = payload.get("userId", "guest") | ||
| pricing: list[dict[str, Any]] = [] | ||
|
|
||
| booking_id = _generate_booking_id() | ||
| confirmation_number = _generate_confirmation_number() | ||
|
|
||
| new_booking = { | ||
| "bookingId": booking_id, | ||
| "hotelId": payload.get("hotelId"), | ||
| "hotelName": payload.get("hotelName"), | ||
| "rooms": payload.get("rooms"), | ||
| "userId": user_id, | ||
| "checkInDate": payload.get("checkInDate"), | ||
| "checkOutDate": payload.get("checkOutDate"), | ||
| "numberOfGuests": payload.get("numberOfGuests"), | ||
| "primaryGuest": payload.get("primaryGuest"), | ||
| "pricing": pricing, | ||
| "bookingStatus": "CONFIRMED", | ||
| "bookingDate": _get_current_timestamp(), | ||
| "confirmationNumber": confirmation_number, | ||
| "specialRequests": payload.get("specialRequests"), | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*/hotel-booking-agent/services/hotel_api/booking.py" -type fRepository: wso2/agent-manager
Length of output: 121
🏁 Script executed:
find . -path "*/hotel-booking-agent/*" -type f | head -20Repository: wso2/agent-manager
Length of output: 1410
🏁 Script executed:
git ls-files | grep -i hotel | head -20Repository: wso2/agent-manager
Length of output: 1311
🏁 Script executed:
wc -l ./samples/hotel-booking-agent/services/hotel_api/booking.pyRepository: wso2/agent-manager
Length of output: 125
🏁 Script executed:
cat -n ./samples/hotel-booking-agent/services/hotel_api/booking.pyRepository: wso2/agent-manager
Length of output: 8337
🏁 Script executed:
find ./samples/hotel-booking-agent/services/hotel_api -name "*.py" -type fRepository: wso2/agent-manager
Length of output: 299
🏁 Script executed:
cat -n ./samples/hotel-booking-agent/services/hotel_api/ingest.pyRepository: wso2/agent-manager
Length of output: 3755
🏁 Script executed:
head -100 ./samples/hotel-booking-agent/services/hotel_api/.env.exampleRepository: wso2/agent-manager
Length of output: 323
🏁 Script executed:
cat -n ./samples/hotel-booking-agent/services/hotel_api/service.pyRepository: wso2/agent-manager
Length of output: 1238
🏁 Script executed:
cat -n ./samples/hotel-booking-agent/services/hotel_api/search.pyRepository: wso2/agent-manager
Length of output: 13915
Implement authentication to resolve IDOR vulnerability affecting user bookings.
The service has no authentication mechanism. All booking endpoints accept userId directly from client-supplied input (payload or query parameters):
/bookings(POST): line 66 -payload.get("userId", "guest")/bookings(GET): line 106 - query parameteruserId/bookings/{booking_id}(GET): line 117 - query parameteruserId/bookings/{booking_id}(PUT): line 139 -payload.get("userId", "guest")/bookings/{booking_id}(DELETE): line 178 - query parameteruserId
Any caller can access or modify any user's bookings by supplying an arbitrary userId. The service.py file contains only CORS middleware with no authentication configured.
Implement FastAPI dependency injection to extract authenticated user identity (e.g., from JWT, OAuth2, or request context) and use that instead of client-supplied values.
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/booking.py` around lines 64 -
87, Add a FastAPI dependency (e.g., get_current_user) that extracts and
validates the authenticated user identity from the request (JWT/OAuth2 token or
request context) and raise HTTPException(401) on failure; then replace all uses
of client-supplied userId with the authenticated user's id by injecting this
dependency into endpoints: create_booking (replace payload.get("userId",
"guest") in create_booking), the GET /bookings handler that reads query param
userId, GET /bookings/{booking_id} that reads query param userId, update handler
that uses payload.get("userId", "guest") (e.g., in update_booking), and DELETE
/bookings/{booking_id} that reads query param userId; ensure each endpoint
signature includes current_user: dict = Depends(get_current_user) (or typed user
model) and use current_user["id"] (or current_user.id) to authorize access to
and modification of bookings, returning 403 if the authenticated user is not the
owner.
| try: | ||
| bookings = _load_bookings() | ||
| bookings.append(new_booking) | ||
| _save_bookings(bookings) | ||
| except Exception: | ||
| logger.exception("create_booking: failed to persist booking") | ||
| return _error_response("Booking persistence failed", "BOOKING_PERSIST_FAILED") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "booking.py" -type fRepository: wso2/agent-manager
Length of output: 121
🏁 Script executed:
wc -l ./samples/hotel-booking-agent/services/hotel_api/booking.pyRepository: wso2/agent-manager
Length of output: 125
🏁 Script executed:
cat -n ./samples/hotel-booking-agent/services/hotel_api/booking.pyRepository: wso2/agent-manager
Length of output: 8337
Storage failures currently return HTTP 200/201 instead of error status codes.
When persistence fails, the API returns error payloads with success status codes (201 for create_booking, 200 for others), misleading clients and breaking standard HTTP retry logic. Use HTTPException with status_code 500 or 503 instead of returning dictionaries from exception handlers.
Affected locations: 89-95, 108-114, 132-134, 172-174, 206-208
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/booking.py` around lines 89 -
95, Exception handlers in booking persistence currently return error
dictionaries but keep HTTP 200/201 status codes; change them to raise
HTTPException (from fastapi) with an appropriate 5xx status (500 or 503) and
include a descriptive detail message and the error info. For example, inside
create_booking (and the other affected handlers: get_booking, update_booking,
delete_booking, list_bookings) catch Exception, call logger.exception(...) as
before, then raise HTTPException(status_code=500, detail="Booking persistence
failed") (or 503 if you prefer transient errors) instead of returning
_error_response; ensure imports include fastapi.HTTPException and remove/replace
the old return paths so the correct HTTP status is sent.
| "primaryGuest": { | ||
| "firstName": "Ava", | ||
| "lastName": "Lee", | ||
| "email": "[email protected]", | ||
| "phoneNumber": "+1234567890" | ||
| }, |
There was a problem hiding this comment.
Anonymize PII in sample bookings.
This dataset includes real-looking names, emails, phone numbers, and nationality fields. Keeping PII-like values in a public repo is a compliance/privacy risk. Please replace with clearly fictitious placeholders (e.g., [email protected], 555-0100) or remove these fields entirely from sample data.
Also applies to: 96-101, 124-129, 152-157
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/storage/bookings.json` around
lines 17 - 22, The sample bookings file contains PII in fields like
primaryGuest.firstName, primaryGuest.lastName, primaryGuest.email,
primaryGuest.phoneNumber and nationality across multiple entries; update each
booking record (e.g., the object under "primaryGuest" and any top-level
"nationality") to use clearly fictitious placeholders (e.g., firstName/lastName
-> "Guest", email -> "[email protected]", phoneNumber -> "555-0100") or
remove those fields entirely, and apply the same replacement to the other
occurrences noted (around the other entries at the commented ranges).
| "specialRequests": { | ||
| "otherRequests": "Late check-in" | ||
| } | ||
| }, | ||
| { | ||
| "bookingId": "BK99E89980", | ||
| "hotelId": "h1", | ||
| "hotelName": "Hotel One", | ||
| "rooms": [ | ||
| { | ||
| "roomType": "Queen", | ||
| "quantity": 1 | ||
| } | ||
| ], | ||
| "userId": "user-a", | ||
| "checkInDate": "2026-02-10", | ||
| "checkOutDate": "2026-02-12", | ||
| "numberOfGuests": 2, | ||
| "primaryGuest": { | ||
| "firstName": "Ava", | ||
| "lastName": "Lee" | ||
| }, | ||
| "pricing": [], | ||
| "bookingStatus": "CANCELLED", | ||
| "bookingDate": "2026-02-03T06:57:01.415099+00:00", | ||
| "confirmationNumber": "CONF033715bf-9a7c-4892-b57a-949cdad808a5", | ||
| "specialRequests": "High floor", | ||
| "updatedAt": "2026-02-03T07:05:46.954947+00:00", | ||
| "cancelledAt": "2026-02-03T07:05:51.245974+00:00" |
There was a problem hiding this comment.
Normalize specialRequests and primaryGuest shapes.
specialRequests alternates between object/string/null and primaryGuest fields are inconsistent across records. This will break clients that rely on a stable schema (especially typed UIs). Align the sample data with the OpenAPI contract and use a consistent shape (e.g., object with optional fields or null).
Also applies to: 71-79
🤖 Prompt for AI Agents
In `@samples/hotel-booking-agent/services/hotel_api/storage/bookings.json` around
lines 27 - 55, The sample booking objects use inconsistent shapes for
specialRequests and primaryGuest which will break typed clients; update all
booking entries (e.g., the object with "bookingId": "BK99E89980" and the entries
around lines 71-79) to match the OpenAPI schema by making specialRequests an
object (or null) with the defined optional fields (e.g., otherRequests,
accessibility, etc.) instead of a raw string, and ensure primaryGuest is always
an object with consistent keys (firstName, lastName, email?, phone? as per
contract) or null; normalize every booking record in bookings.json to that shape
so the JSON schema is stable across records.
4 participants
New Sample: Hotel Booking Agent
Summary by CodeRabbit
New Features
Documentation
Chores