feat(platform): add AI image generation integration with approval flow#1114
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR introduces a complete example AI image generation integration (ai-image) with OpenAI-compatible API support, alongside infrastructure and UI changes to support integration feedback workflows. The ai-image example includes an agent configuration, integration metadata, and a connector implementation supporting both image generation and editing. Chat UI components are extended to thread an Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/platform/convex/approvals/helpers.ts (1)
262-289:⚠️ Potential issue | 🟠 MajorKeep executing approvals in the conflict check.
This query only reads
status = 'pending', so theexisting.status === 'executing'branch below is dead code. An approval that is already executing on the same thread/resource type will now be ignored and a second active approval can be created on top of it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/approvals/helpers.ts` around lines 262 - 289, The conflict check only queries approvals with status='pending' so the branch checking existing.status === 'executing' is never reached; update the query built in ctx.db.query('approvals').withIndex('by_threadId_status_resourceType', (q) => ...) to include both pending and executing statuses (e.g. use q.in('status', ['pending','executing']) or otherwise remove the status eq and filter for both statuses after fetching) so that the loop over existing approvals will detect executing approvals and the subsequent existing.status === 'executing' / existing.status === 'pending' logic in the block will work as intended.services/platform/convex/agent_tools/integrations/internal_actions.ts (1)
230-257:⚠️ Potential issue | 🟠 MajorMake thread notification best-effort.
Both branches persist the execution result before awaiting
notifyThread(). If the follow-up save/schedule fails, this action throws even though the integration already ran and the approval is already completed/rejected; on the error path it can also hide the original execution failure. Wrap notification in a localtry/catchand preserve the primary outcome.♻️ Proposed fix
- await notifyThread( - ctx, - approval, - buildCompletionMessage(metadata, result), - ); + try { + await notifyThread( + ctx, + approval, + buildCompletionMessage(metadata, result), + ); + } catch (notifyError) { + console.error( + '[executeApprovedOperation] Failed to notify thread after success:', + notifyError, + ); + } @@ - await notifyThread( - ctx, - approval, - buildErrorMessage(metadata, errorMessage), - ); + try { + await notifyThread( + ctx, + approval, + buildErrorMessage(metadata, errorMessage), + ); + } catch (notifyError) { + console.error( + '[executeApprovedOperation] Failed to notify thread after failure:', + notifyError, + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/agent_tools/integrations/internal_actions.ts` around lines 230 - 257, The notifyThread calls should be best-effort so they never mask or change the primary outcome: wrap both notifyThread(..., buildCompletionMessage(...)) and notifyThread(..., buildErrorMessage(...)) in their own try/catch blocks and log any notify errors without rethrowing; keep the existing ctx.runMutation calls to internal.agent_tools.integrations.internal_mutations.updateApprovalWithResult (and the conversion toConvexJsonValue(result)) as the authoritative persisted outcome, and ensure the original errorMessage/result is returned/propagated unchanged if notification fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/integrations/ai-image/config.json`:
- Line 3: The description field overclaims by saying "other compatible
providers" while the allowedHosts array only permits api.together.xyz and
api.openai.com; either change the "description" string(s) (including the other
description occurrences at the same section) to explicitly list only OpenAI and
Together AI, or expand the allowedHosts array to include the other intended
providers' hostnames so the text matches runtime enforcement; update the
"description" properties and/or the "allowedHosts" array accordingly and ensure
the two remain consistent.
In `@examples/integrations/ai-image/connector.ts`:
- Around line 399-404: The response payload currently sets model to params.model
|| 'default' but the connector actually uses the connection-level
configuredModel; replace occurrences where the payload/model is set from params
(e.g., the data object using imageRefs, params.prompt, params.model ||
'default') to use the actual configuredModel value used at runtime (e.g.,
configuredModel or connection.configuredModel), and apply the same change to the
other identical block around lines 503-507 so the returned result reports the
real model used rather than 'default'.
- Around line 300-330: processImageItems currently silently skips items when the
FilesApi (files) is missing or when an item isn't one of the handled shapes;
change it to fail fast and surface errors: if files is undefined, throw a
descriptive Error (e.g., "Files API unavailable: cannot store generated images")
before the loop; during processing, if an item cannot be converted to a stored
FileReference (neither b64_json nor url handled), throw or accumulate an error
and finally throw if no images were stored (e.g., "No images could be stored
from API response"); ensure the same defensive checks and error behavior are
applied to the other image-processing blocks with the same logic (the other
functions/blocks handling image items at the indicated locations) so callers
receive a failure instead of success: true when count === 0.
In `@services/platform/app/features/chat/components/chat-interface.tsx`:
- Around line 336-341: handleSendMessageDirect currently fires sendMessage
without guarding against concurrent calls; add a local guard (e.g., isSendingRef
or isSending state) in the chat-interface component to return early if a send is
already in progress, set the guard before calling sendMessage and clear it after
the send finishes (use finally). If sendMessage currently does not return a
promise, update the wrapper to await or consume its promise so the guard can be
cleared reliably; reference handleSendMessageDirect, sendMessage,
useSendMessage/useChatLoadingState and clearChatState when making the change.
In
`@services/platform/app/features/chat/components/integration-approval-card.tsx`:
- Around line 337-345: The rejection flow currently always sets
triggerAgentResponse: true when calling updateApprovalStatus, but onSendMessage
is optional so that causes the client to skip the server fallback; change the
updateApprovalStatus call in the rejection handler (the block using
updateApprovalStatus, approvalId, feedback, setFeedbackText, setShowFeedback,
onSendMessage) to derive triggerAgentResponse from the presence of onSendMessage
(e.g. triggerAgentResponse: Boolean(onSendMessage) or !!onSendMessage) so that
if onSendMessage is not provided the backend rejection path runs, and keep the
existing calls to setFeedbackText('')/setShowFeedback(false) and
onSendMessage?.(feedback) unchanged.
- Around line 38-57: Replace the raw <img> in ParameterImagePreview with the app
Image component (import from "@/app/components/ui/data-display/image") and only
render the preview/button when isImageUrl(url) returns true for first-party
storage URLs (require the URL contains "/api/storage/"); update isImageUrl to be
a TypeScript type predicate (e.g., function isImageUrl(v): v is ImageUrl) so you
can avoid unsafe casts where it’s used. Additionally, change the feedback flow
around triggerAgentResponse and onSendMessage so feedback is never dropped: if
onSendMessage is provided use it, otherwise call the existing backend feedback
mutation/fallback submit function (or set triggerAgentResponse=false to let the
backend emit the rejection system message) so that feedback is always delivered
to the thread instead of being silently lost.
In
`@services/platform/app/features/chat/components/message-bubble/markdown-renderer.tsx`:
- Around line 107-113: The markdownImageComponent is currently destructuring and
discarding width/height (named _width/_height) so authored intrinsic sizes are
lost; change the signature to capture width and height (e.g., width, height) and
forward parsed numeric values to <MarkdownImage /> instead of stripping
them—convert string values like "100" or "100px" to numbers (or gracefully
handle non-numeric by leaving undefined) and pass them as numeric width/height
props to MarkdownImage so authored dimensions are preserved; reference
markdownImageComponent and MarkdownImage when applying this fix.
In
`@services/platform/app/features/settings/integrations/components/integration-manage/integration-credentials-form.tsx`:
- Around line 112-114: The code redundantly asserts integration.setupGuide as
string when a prior typeof check already narrows it; remove the unnecessary "as
string" type assertion and pass integration.setupGuide directly to ReactMarkdown
(locate the JSX using ReactMarkdown in IntegrationCredentialsForm /
integration-credentials-form.tsx and the typeof check around line 100).
- Line 18: Remove the unused import isRecord from the top of the file
(integration-credentials-form.tsx); locate the import statement that reads
"import { isRecord } ..." and delete it so ESLint no-unused-vars is resolved and
the file only imports actually used symbols.
In
`@services/platform/convex/agent_tools/integrations/create_bound_integration_tool.ts`:
- Around line 94-101: Replace the manual typeof/rawProp checks and the "as
Record<string, unknown>" cast by using the existing isRecord(rawProp) guard
before treating rawProp as an object (so paramEntries.push([...
jsonTypeToZod(...)]) only runs when isRecord(rawProp) is true); and for handling
variants (where you already validated variants.length >= 2), remove the tuple
cast and instead destructure the first two elements with a length guard (e.g.,
const [v1, v2] = variants; after confirming length) to build the tuple safely
without assertions. Ensure references are made to properties, paramEntries,
jsonTypeToZod, isRecord, and variants.
- Around line 84-101: The loop in create_bound_integration_tool.ts that builds
paramsSchema currently checks prop.required (wrong) and uses unsafe casts;
change it to read the required field names from parametersSchema.required (an
array of strings) to determine whether each property is required when calling
jsonTypeToZod, e.g., compute a Set of required names from
op.parametersSchema?.required and use requiredSet.has(name) instead of
prop.required; also remove the unsafe "as" assertions by adding proper
runtime/type guards for rawProp/prop (narrowing checks for objects and string
fields like prop.type and prop.description) rather than using "as" casts so
jsonTypeToZod receives correctly typed arguments.
In
`@services/platform/convex/agent_tools/integrations/fetch_operations_summary.ts`:
- Around line 105-145: The code casts integration.metadata and uses typeof
connectionConfig === 'object' which can be true for arrays; update
resolveMetadataVariables to accept metadata as Record<string, unknown> (remove
the unnecessary cast of integration.metadata) and replace the typeof check with
the project's isRecord narrowing (e.g., use isRecord(connectionConfig) to create
vars as a Record<string, unknown>), keeping the rest of the logic the same so
arrays are excluded and proper typing is preserved for resolveMetadataVariables,
integration.metadata, and connectionConfig.
In `@services/platform/convex/agent_tools/integrations/internal_mutations.ts`:
- Around line 87-111: Both new internal mutations saveSystemMessage and
triggerIntegrationCompletionResponse are missing explicit returns validators;
add returns: v.null() to each internalMutation declaration so the runtime schema
and generated docs reflect Promise<void> responses. Update the saveSystemMessage
internalMutation to include returns: v.null() alongside args, and do the same
for triggerIntegrationCompletionResponse (the handler remains Promise<void> and
unchanged). Ensure you import or reference the same validator namespace (v)
already used for args.
In
`@services/platform/convex/agent_tools/integrations/trigger_completion_action.ts`:
- Around line 25-31: The code is hardcoding DEFAULT_ORG_SLUG when calling
internal.agents.file_actions.resolveAgentConfig; change the call to pass the
actual org slug from the incoming args (use args.orgSlug) and only fall back to
DEFAULT_ORG_SLUG if args.orgSlug is missing so org-scoped/custom agents are
resolved correctly; update the ctx.runAction call that sets agentConfig (and any
nearby references) to use args.orgSlug instead of the hardcoded DEFAULT_ORG_SLUG
while keeping args.agentSlug and args.organizationId as before.
In `@services/platform/convex/approvals/mutations.ts`:
- Around line 4-5: The file imports unused symbols causing lint failures; remove
the unused imports by deleting isRecord from the first import and internal from
the second import, leaving only the used import(s) (e.g., keep components) so
the import lines only bring in referenced identifiers (update the import
statements that currently reference isRecord and internal in this file).
In `@services/platform/convex/integrations/build_test_secrets.ts`:
- Around line 20-27: The code iterates over integration.connectionConfig without
ensuring it's actually an object; add a defensive object type guard before the
loop (e.g., check integration.connectionConfig !== null && typeof
integration.connectionConfig === 'object' and optionally
!Array.isArray(integration.connectionConfig')) so Object.entries(...) is only
called on real objects; update the block around the for (const [key, value] of
Object.entries(integration.connectionConfig)) loop to require that guard and
otherwise skip populating secrets.
In `@services/platform/convex/integrations/credentials_schema.ts`:
- Around line 70-72: The schema currently makes connectionConfig permissive by
using v.optional(v.any()), which allows primitives/null/arrays and shifts errors
to runtime; change the field to an object-shaped validator (e.g., use
jsonRecordValidator or a v.record/v.object validator) so connectionConfig
remains a record. Update the credentials schema entry named connectionConfig in
credentials_schema.ts to use v.optional(jsonRecordValidator()) (or an equivalent
record/object validator) so downstream code that expects an object
(Object.entries/spread) always receives a validated record.
In `@services/platform/convex/integrations/load_integration.ts`:
- Around line 259-264: The merge for connectionConfig (and likewise metadata)
must guard runtime v.optional(v.any()) values before spreading; update the code
that builds connectionConfig in load_integration.ts to first test each source
(config.connectionConfig and credentials.connectionConfig) is a plain object
(e.g., value !== null && typeof value === 'object' && !Array.isArray(value)),
then only construct and assign the merged object when at least one valid object
exists—follow the same conditional-merge pattern used for sqlConnectionConfig
and oauth2Config so you avoid materializing empty objects or spreading
non-object values.
In `@services/platform/convex/integrations/validators.ts`:
- Around line 87-92: The exported connectionConfigValidator currently uses
v.any() which permits non-object values and can cause Object.entries() in
consumers (build_test_secrets.ts, test_connection.ts) to fail; replace its
definition with v.record(v.string(), v.any()) so the validator guarantees an
object mapping string keys to any values while still allowing
integration-specific fields—update the exported symbol connectionConfigValidator
accordingly and run type tests to ensure callers no longer need additional
runtime guards.
In `@services/platform/convex/lib/agent_response/generate_response.ts`:
- Around line 367-374: The cancelContextStats totalTokens is being computed from
raw instructions (instructions) which undercounts rendered/templated content;
instead compute the system token estimate from the rendered system prompt used
to build cancelContextWindow (agentInstructions / rendered system prompt) so the
saved structuredThreadContext.stats.totalTokens + sysTokens + toolTokens matches
the persisted context; update the sysTokens calculation to use the rendered
agentInstructions/cancelContextWindow content (and include any appended
structured response instructions) before summing into cancelContextStats.
In
`@services/platform/convex/node_only/integration_sandbox/helpers/create_http_api.ts`:
- Around line 25-37: Replace the duplicated local type FormFieldInput with the
existing imported FormField interface and update usages: change the
BodyMethodOptions.formFields type from FormFieldInput[] to FormField[] and
remove any casts that were added to satisfy the older duplicate type; update any
references to FormFieldInput to use FormField so the code (e.g., where
BodyMethodOptions and formFields are used) no longer requires `as` casts and the
contracts stay consistent.
In
`@services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts`:
- Around line 27-33: Sanitize multipart header values by stripping CR and LF
characters from field.name, field.fileName, and field.contentType before
building the header: replace any \r or \n in field.name and field.fileName (used
to compute safeName and safeFileName) and in field.contentType with an empty
string so header concatenation for Content-Disposition and Content-Type cannot
be CR/LF-injected; update the logic around safeName, safeFileName and where
header is built in execute_http_request.ts to perform this cleanse in addition
to the existing quote-escaping.
- Around line 69-76: The code that builds multipart form requests loses or
mangles existing headers by spreading effectiveReq.options.headers; instead
normalize headers via const headers = new Headers(effectiveReq.options.headers)
(so any HeadersInit is preserved), then call headers.set('Content-Type',
contentType) to add the boundary, and pass that Headers instance into
fetchOptions (e.g., fetchOptions = { ...effectiveReq.options, headers, body });
update the block around effectiveReq.formFields/buildFormData to use new
Headers(...) and .set() rather than object spread so all original headers
survive.
---
Outside diff comments:
In `@services/platform/convex/agent_tools/integrations/internal_actions.ts`:
- Around line 230-257: The notifyThread calls should be best-effort so they
never mask or change the primary outcome: wrap both notifyThread(...,
buildCompletionMessage(...)) and notifyThread(..., buildErrorMessage(...)) in
their own try/catch blocks and log any notify errors without rethrowing; keep
the existing ctx.runMutation calls to
internal.agent_tools.integrations.internal_mutations.updateApprovalWithResult
(and the conversion toConvexJsonValue(result)) as the authoritative persisted
outcome, and ensure the original errorMessage/result is returned/propagated
unchanged if notification fails.
In `@services/platform/convex/approvals/helpers.ts`:
- Around line 262-289: The conflict check only queries approvals with
status='pending' so the branch checking existing.status === 'executing' is never
reached; update the query built in
ctx.db.query('approvals').withIndex('by_threadId_status_resourceType', (q) =>
...) to include both pending and executing statuses (e.g. use q.in('status',
['pending','executing']) or otherwise remove the status eq and filter for both
statuses after fetching) so that the loop over existing approvals will detect
executing approvals and the subsequent existing.status === 'executing' /
existing.status === 'pending' logic in the block will work as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 087e9bef-4e04-4cb4-be90-488cf167a1cc
⛔ Files ignored due to path filters (2)
examples/integrations/ai-image/icon.svgis excluded by!**/*.svgservices/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (35)
examples/agents/image-creator.jsonexamples/integrations/ai-image/config.jsonexamples/integrations/ai-image/connector.tsservices/platform/app/features/chat/components/approval-card-renderer.tsxservices/platform/app/features/chat/components/chat-interface.tsxservices/platform/app/features/chat/components/chat-messages.tsxservices/platform/app/features/chat/components/integration-approval-card.tsxservices/platform/app/features/chat/components/message-bubble/markdown-renderer.tsxservices/platform/app/features/settings/integrations/components/integration-manage/integration-credentials-form.tsxservices/platform/app/features/settings/integrations/hooks/use-integration-manage.tsservices/platform/convex/agent_tools/approval_shared.tsservices/platform/convex/agent_tools/integrations/create_bound_integration_tool.tsservices/platform/convex/agent_tools/integrations/fetch_operations_summary.tsservices/platform/convex/agent_tools/integrations/internal_actions.tsservices/platform/convex/agent_tools/integrations/internal_mutations.tsservices/platform/convex/agent_tools/integrations/trigger_completion_action.tsservices/platform/convex/agent_tools/integrations/types.tsservices/platform/convex/agent_tools/workflows/internal_mutations.tsservices/platform/convex/approvals/helpers.tsservices/platform/convex/approvals/mutations.tsservices/platform/convex/integrations/build_test_secrets.tsservices/platform/convex/integrations/credentials_schema.tsservices/platform/convex/integrations/file_actions.tsservices/platform/convex/integrations/load_integration.tsservices/platform/convex/integrations/validators.tsservices/platform/convex/lib/agent_chat/internal_actions.tsservices/platform/convex/lib/agent_response/generate_response.tsservices/platform/convex/lib/helpers/public_storage_url.tsservices/platform/convex/node_only/integration_sandbox/helpers/create_convex_storage_provider.tsservices/platform/convex/node_only/integration_sandbox/helpers/create_http_api.tsservices/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.tsservices/platform/convex/node_only/integration_sandbox/helpers/url_rewrite.tsservices/platform/convex/node_only/integration_sandbox/types.tsservices/platform/lib/shared/schemas/integrations.tsservices/platform/messages/en.json
| @@ -0,0 +1,88 @@ | |||
| { | |||
| "title": "AI Image", | |||
| "description": "Create and edit images with AI. Supports OpenAI (GPT Image, DALL-E), Together AI, and other compatible providers.", | |||
There was a problem hiding this comment.
The description overpromises the allowed provider set.
Line 3 says this supports "other compatible providers", but allowedHosts only permits api.together.xyz and api.openai.com. As written, users can configure a different compatible endpoint and still be blocked at runtime. Either widen the allowlist to the supported providers or narrow the copy here.
Also applies to: 15-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/integrations/ai-image/config.json` at line 3, The description field
overclaims by saying "other compatible providers" while the allowedHosts array
only permits api.together.xyz and api.openai.com; either change the
"description" string(s) (including the other description occurrences at the same
section) to explicitly list only OpenAI and Together AI, or expand the
allowedHosts array to include the other intended providers' hostnames so the
text matches runtime enforcement; update the "description" properties and/or the
"allowedHosts" array accordingly and ensure the two remain consistent.
Comment on lines
+300
to
+330
| function processImageItems( | ||
| imageItems: any[], | ||
| files: FilesApi | undefined, | ||
| prefix: string, | ||
| ): FileReference[] { | ||
| const imageRefs: FileReference[] = []; | ||
| for (let i = 0; i < imageItems.length; i++) { | ||
| const item = imageItems[i]; | ||
| const timestamp = Date.now(); | ||
| const fileName = prefix + '_' + timestamp + '_' + i + '.png'; | ||
|
|
||
| if (item.b64_json && files) { | ||
| const ref = files.store(item.b64_json, { | ||
| encoding: 'base64', | ||
| contentType: 'image/png', | ||
| fileName: fileName, | ||
| }); | ||
| imageRefs.push(ref); | ||
| } else if (item.url && files) { | ||
| // All supported models should return b64_json; URL responses are unexpected | ||
| console.warn( | ||
| 'Unexpected URL response from API, expected b64_json. Downloading: ' + | ||
| fileName, | ||
| ); | ||
| const ref = files.download(item.url, { | ||
| fileName: fileName, | ||
| }); | ||
| imageRefs.push(ref); | ||
| } | ||
| } | ||
| return imageRefs; |
There was a problem hiding this comment.
Fail when the API response can't be turned into stored image files.
processImageItems() silently drops every item when ctx.files is missing or the response shape isn't one of the handled cases, and both operations still return success: true. That leaves the chat/approval flow with count: 0 and nothing renderable even after a successful image API response.
♻️ Proposed fix
function processImageItems(
imageItems: any[],
files: FilesApi | undefined,
prefix: string,
): FileReference[] {
+ if (!files) {
+ throw new Error('Image operations require file storage support.');
+ }
const imageRefs: FileReference[] = [];
for (let i = 0; i < imageItems.length; i++) {
const item = imageItems[i];
@@
- if (item.b64_json && files) {
+ if (item.b64_json) {
const ref = files.store(item.b64_json, {
encoding: 'base64',
contentType: 'image/png',
fileName: fileName,
});
imageRefs.push(ref);
- } else if (item.url && files) {
+ } else if (item.url) {
// All supported models should return b64_json; URL responses are unexpected
console.warn(
'Unexpected URL response from API, expected b64_json. Downloading: ' +
fileName,
);
@@
}
}
+ if (imageRefs.length === 0) {
+ throw new Error(
+ 'The image API returned data, but no image could be persisted from the response.',
+ );
+ }
return imageRefs;
}Also applies to: 394-408, 498-511
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/integrations/ai-image/connector.ts` around lines 300 - 330,
processImageItems currently silently skips items when the FilesApi (files) is
missing or when an item isn't one of the handled shapes; change it to fail fast
and surface errors: if files is undefined, throw a descriptive Error (e.g.,
"Files API unavailable: cannot store generated images") before the loop; during
processing, if an item cannot be converted to a stored FileReference (neither
b64_json nor url handled), throw or accumulate an error and finally throw if no
images were stored (e.g., "No images could be stored from API response"); ensure
the same defensive checks and error behavior are applied to the other
image-processing blocks with the same logic (the other functions/blocks handling
image items at the indicated locations) so callers receive a failure instead of
success: true when count === 0.
Comment on lines
+399
to
+404
| data: { | ||
| images: imageRefs, | ||
| prompt: params.prompt, | ||
| model: params.model || 'default', | ||
| size: params.size || '1024x1024', | ||
| count: imageRefs.length, |
There was a problem hiding this comment.
Return the actual model that was used.
Execution always uses the connection-level configuredModel, but these responses report params.model || 'default'. When the model comes from connection settings—as this connector expects—the result mislabels every image as default.
♻️ Proposed fix
- model: params.model || 'default',
+ model: model,
@@
- model: params.model || 'default',
+ model: model,Also applies to: 503-507
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/integrations/ai-image/connector.ts` around lines 399 - 404, The
response payload currently sets model to params.model || 'default' but the
connector actually uses the connection-level configuredModel; replace
occurrences where the payload/model is set from params (e.g., the data object
using imageRefs, params.prompt, params.model || 'default') to use the actual
configuredModel value used at runtime (e.g., configuredModel or
connection.configuredModel), and apply the same change to the other identical
block around lines 503-507 so the returned result reports the real model used
rather than 'default'.
Comment on lines
+336
to
+341
| const handleSendMessageDirect = useCallback( | ||
| (message: string) => { | ||
| scrollingToBottomBehaviorRef.current = 'smooth'; | ||
| void sendMessage(message); | ||
| }, | ||
| [sendMessage], |
There was a problem hiding this comment.
Guard this fire-and-forget send path against overlap.
handleSendMessageDirect can invoke sendMessage multiple times before the loading state updates, but useSendMessage / useChatLoadingState only track a single pending request. Two rapid calls here can overlap, and a failure in one request will clearChatState() while the other is still running.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/chat-interface.tsx` around
lines 336 - 341, handleSendMessageDirect currently fires sendMessage without
guarding against concurrent calls; add a local guard (e.g., isSendingRef or
isSending state) in the chat-interface component to return early if a send is
already in progress, set the guard before calling sendMessage and clear it after
the send finishes (use finally). If sendMessage currently does not return a
promise, update the wrapper to await or consume its promise so the guard can be
cleared reliably; reference handleSendMessageDirect, sendMessage,
useSendMessage/useChatLoadingState and clearChatState when making the change.
Comment on lines
+38
to
+57
| function ParameterImagePreview({ src, alt }: { src: string; alt: string }) { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| return ( | ||
| <> | ||
| <button | ||
| type="button" | ||
| onClick={() => setIsOpen(true)} | ||
| className="ring-border focus:ring-ring mt-1 inline-block cursor-pointer overflow-hidden rounded border-none bg-transparent p-0 ring-1 transition-opacity hover:opacity-90 focus:ring-2 focus:outline-none" | ||
| > | ||
| <img src={src} alt={alt} className="block h-16 w-auto object-contain" /> | ||
| </button> | ||
| <ImagePreviewDialog | ||
| isOpen={isOpen} | ||
| onOpenChange={setIsOpen} | ||
| src={src} | ||
| alt={alt} | ||
| /> | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file mentioned in the review
fd -t f "integration-approval-card.tsx"Repository: tale-project/tale
Length of output: 137
🏁 Script executed:
# Check the structure of the repository to understand paths
git ls-files | head -30Repository: tale-project/tale
Length of output: 645
🏁 Script executed:
# Get the file size to determine how to read it
wc -l services/platform/app/features/chat/components/integration-approval-card.tsx 2>/dev/null || echo "File not found in expected location"Repository: tale-project/tale
Length of output: 141
🏁 Script executed:
# Read the specific sections mentioned in the review
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '1,70p'Repository: tale-project/tale
Length of output: 2891
🏁 Script executed:
# Read lines around 147-178
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '140,180p'Repository: tale-project/tale
Length of output: 1850
🏁 Script executed:
# Read lines around 337-345
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '330,350p'Repository: tale-project/tale
Length of output: 1136
🏁 Script executed:
# Check if the Image component exists
fd -t f "image.tsx" -o -name "image.ts" services/platform/app/components/ui/Repository: tale-project/tale
Length of output: 291
🏁 Script executed:
# Search for the Image component
find services/platform/app/components/ui -name "*image*" -type fRepository: tale-project/tale
Length of output: 186
🏁 Script executed:
# Also check for Image component in the app directory
find services/platform/app -name "*image*" -type f | grep -E "\.(tsx?|jsx?)$"Repository: tale-project/tale
Length of output: 689
🏁 Script executed:
# Check the imports at the top of the file to see what components are being used
head -40 services/platform/app/features/chat/components/integration-approval-card.tsxRepository: tale-project/tale
Length of output: 1462
🏁 Script executed:
# Search for the Image component definition across the codebase
rg "export.*Image.*component\|export const Image\|export function Image" --type ts --type tsxRepository: tale-project/tale
Length of output: 88
🏁 Script executed:
# Check the Image component implementation
cat -n services/platform/app/components/ui/data-display/image.tsxRepository: tale-project/tale
Length of output: 2085
🏁 Script executed:
# Search for Image imports in the file
rg "import.*Image" services/platform/app/features/chat/components/integration-approval-card.tsxRepository: tale-project/tale
Length of output: 136
🏁 Script executed:
# Check if Image is exported from a common location like `@/components/ui/image`
find services/platform/app/components/ui -name "index.ts" -o -name "index.tsx" | xargs grep -l "Image" 2>/dev/nullRepository: tale-project/tale
Length of output: 45
🏁 Script executed:
# Check the full path structure to understand the import alias
cat services/platform/tsconfig.json | grep -A 20 "compilerOptions"Repository: tale-project/tale
Length of output: 407
🏁 Script executed:
# Check if there's an index file that re-exports Image
cat services/platform/app/components/ui/index.ts 2>/dev/null || cat services/platform/app/components/ui/index.tsx 2>/dev/null || echo "No index file found"Repository: tale-project/tale
Length of output: 80
🏁 Script executed:
# Check for data-display index
cat services/platform/app/components/ui/data-display/index.ts 2>/dev/null || cat services/platform/app/components/ui/data-display/index.tsx 2>/dev/null || echo "No index file in data-display"Repository: tale-project/tale
Length of output: 90
🏁 Script executed:
# Let me check the broader structure
ls -la services/platform/app/components/ui/Repository: tale-project/tale
Length of output: 923
🏁 Script executed:
# Check how Image is used elsewhere in the codebase to see the import pattern
rg "from.*Image|import.*Image" services/platform/app --type ts --type tsx -A1 | head -50Repository: tale-project/tale
Length of output: 88
🏁 Script executed:
# Search for Image imports with a simpler pattern
rg "Image" services/platform/app/features/chat/components/message-bubble/ | head -20Repository: tale-project/tale
Length of output: 2610
🏁 Script executed:
# Check the message-bubble directory structure
ls -la services/platform/app/features/chat/components/message-bubble/Repository: tale-project/tale
Length of output: 565
🏁 Script executed:
# Look at how Image component would be imported based on the path
cat services/platform/app/features/documents/components/document-preview-image.tsx | head -50Repository: tale-project/tale
Length of output: 1307
🏁 Script executed:
# Verify the entire integration-approval-card file to see all usages
cat services/platform/app/features/chat/components/integration-approval-card.tsx | grep -n "Image\|image\|src="Repository: tale-project/tale
Length of output: 881
🏁 Script executed:
# Verify the path structure - the review says "@/components/ui/image" but let's confirm the actual path alias
cat services/platform/tsconfig.jsonRepository: tale-project/tale
Length of output: 450
🏁 Script executed:
# Check how the Image component is actually imported in the markdown-renderer
grep -A5 "import.*Image" services/platform/app/features/chat/components/message-bubble/markdown-renderer.tsx | head -20Repository: tale-project/tale
Length of output: 481
🏁 Script executed:
# Verify the full context of the feedback flow and how onSendMessage is used
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '59,75p'Repository: tale-project/tale
Length of output: 714
🏁 Script executed:
# Check if there are any URL validation/allowlist mechanisms elsewhere
rg "isImageUrl|allowlist|sandbox\|storage" services/platform/app/features/chat/components/ -B2 -A2Repository: tale-project/tale
Length of output: 1231
🏁 Script executed:
# Let me check the feedback flow more thoroughly - look at the updateApprovalStatus mutation
rg "updateApprovalStatus" services/platform/app/features/chat/hooks/mutations.ts -A20Repository: tale-project/tale
Length of output: 696
🏁 Script executed:
# Also check the backend mutation to understand the triggerAgentResponse behavior mentioned in the review
find services/platform -name "*.ts" -path "*/mutations*" | grep -i approvalRepository: tale-project/tale
Length of output: 108
🏁 Script executed:
# Get the full ParameterImagePreview component and its context
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '35,85p'Repository: tale-project/tale
Length of output: 2026
🏁 Script executed:
# Check the backend mutation implementation
cat -n services/platform/convex/approvals/mutations.ts | sed -n '1,100p'Repository: tale-project/tale
Length of output: 4056
🏁 Script executed:
# Look for the specific behavior mentioned in the review about triggerAgentResponse
grep -n "triggerAgentResponse" services/platform/convex/approvals/mutations.ts -B5 -A5Repository: tale-project/tale
Length of output: 952
🏁 Script executed:
# Check if Image component should actually be used by looking at similar parameter displays
rg "ParameterImagePreview\|isImageUrl" services/platform --type ts --type tsx -B3 -A3Repository: tale-project/tale
Length of output: 88
🏁 Script executed:
# Let me verify the import path structure more carefully
cat services/platform/app/components/ui/data-display/image.tsx | head -15Repository: tale-project/tale
Length of output: 404
🏁 Script executed:
# Check if there's a shorthand export path for Image
rg "export.*Image.*from.*image" services/platform/app/components/ui/ -A2Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Verify the feedback flow - check if onSendMessage is always passed or can be optional
grep -n "onSendMessage" services/platform/app/features/chat/components/integration-approval-card.tsxRepository: tale-project/tale
Length of output: 248
🏁 Script executed:
# Check how the component is instantiated to see if onSendMessage is always provided
rg "IntegrationApprovalCard" services/platform/app --type ts --type tsx -B3 -A3 | head -60Repository: tale-project/tale
Length of output: 88
🏁 Script executed:
# Check the complete feedback flow context to understand the onSendMessage usage
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '325,360p'Repository: tale-project/tale
Length of output: 1759
🏁 Script executed:
# Look at how IntegrationApprovalCard is used in the codebase
rg "IntegrationApprovalCard\|IntegrationApprovalCardComponent" services/platform/app --type ts -A5 | head -100Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Verify if the review's description of the coding guidelines is accurate - look for the exact guideline
rg "custom.*Image.*component" services/platform --type ts | head -5Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Let me check how the Image component is typically imported in the codebase
grep -n "import.*Image.*from.*@/app/components" services/platform/app/features/chat/components/message-bubble/markdown-renderer.tsxRepository: tale-project/tale
Length of output: 127
🏁 Script executed:
# Verify the complete picture - check if there's a central import location or if each file imports directly
find services/platform/app -name "*.tsx" -type f -exec grep -l "from.*@/app/components/ui/data-display/image" {} \;Repository: tale-project/tale
Length of output: 1128
🏁 Script executed:
# Double-check the isImageUrl implementation - does it have any allowlist or trust mechanism?
cat -n services/platform/app/features/chat/components/integration-approval-card.tsx | sed -n '147,162p'Repository: tale-project/tale
Length of output: 869
🏁 Script executed:
# Final verification - check the actual coding guideline about Image component path
grep -n "@/components/ui/image" services/platform -r --include="*.md" --include="*.ts" --include="*.tsx" 2>/dev/null | head -5Repository: tale-project/tale
Length of output: 43
Replace raw <img> with custom Image component and restrict image previews to trusted sources.
Line 47 uses a raw <img> tag; replace it with the custom Image component from @/app/components/ui/data-display/image.
More critically, isImageUrl() (lines 147–162) accepts arbitrary external http(s) URLs without validation. Line 47 will immediately fetch third-party images when the approval card renders, leaking viewer metadata and bypassing any sandbox controls. Restrict previews to first-party storage URLs (those containing /api/storage/), reject arbitrary external domains, and convert isImageUrl to a type predicate so the unsafe cast on line 177 becomes unnecessary.
Address missing fallback in the feedback flow.
Line 341 always sets triggerAgentResponse: true, which causes the backend to skip its rejection system message (see backend mutation lines 67–71) and rely instead on the frontend sending the user's feedback. However, onSendMessage is optional (line 68). If not provided by the caller, feedback is silently dropped and never delivered to the thread. Ensure feedback is always sent, or add a fallback mechanism if onSendMessage is unavailable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/integration-approval-card.tsx`
around lines 38 - 57, Replace the raw <img> in ParameterImagePreview with the
app Image component (import from "@/app/components/ui/data-display/image") and
only render the preview/button when isImageUrl(url) returns true for first-party
storage URLs (require the URL contains "/api/storage/"); update isImageUrl to be
a TypeScript type predicate (e.g., function isImageUrl(v): v is ImageUrl) so you
can avoid unsafe casts where it’s used. Additionally, change the feedback flow
around triggerAgentResponse and onSendMessage so feedback is never dropped: if
onSendMessage is provided use it, otherwise call the existing backend feedback
mutation/fallback submit function (or set triggerAgentResponse=false to let the
backend emit the rejection system message) so that feedback is always delivered
to the thread instead of being silently lost.
Comment on lines
+367
to
+374
| const sysTokens = instructions ? estimateTokens(instructions) : 0; | ||
| const toolTokens = toolsSummary ? estimateTokens(toolsSummary) : 0; | ||
| cancelContextStats = { | ||
| ...structuredThreadContext.stats, | ||
| totalTokens: | ||
| structuredThreadContext.stats.totalTokens + | ||
| sysTokens + | ||
| toolTokens, |
There was a problem hiding this comment.
Use the rendered system prompt for the cancellation token estimate.
cancelContextWindow is built from agentInstructions, but totalTokens is computed from raw instructions. That undercounts whenever template variables expand or structured response instructions are appended, so the saved stats no longer match the persisted context.
🛠️ Suggested fix
- const sysTokens = instructions ? estimateTokens(instructions) : 0;
+ const sysTokens = agentInstructions
+ ? estimateTokens(agentInstructions)
+ : 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const sysTokens = instructions ? estimateTokens(instructions) : 0; | |
| const toolTokens = toolsSummary ? estimateTokens(toolsSummary) : 0; | |
| cancelContextStats = { | |
| ...structuredThreadContext.stats, | |
| totalTokens: | |
| structuredThreadContext.stats.totalTokens + | |
| sysTokens + | |
| toolTokens, | |
| const sysTokens = agentInstructions | |
| ? estimateTokens(agentInstructions) | |
| : 0; | |
| const toolTokens = toolsSummary ? estimateTokens(toolsSummary) : 0; | |
| cancelContextStats = { | |
| ...structuredThreadContext.stats, | |
| totalTokens: | |
| structuredThreadContext.stats.totalTokens + | |
| sysTokens + | |
| toolTokens, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/lib/agent_response/generate_response.ts` around
lines 367 - 374, The cancelContextStats totalTokens is being computed from raw
instructions (instructions) which undercounts rendered/templated content;
instead compute the system token estimate from the rendered system prompt used
to build cancelContextWindow (agentInstructions / rendered system prompt) so the
saved structuredThreadContext.stats.totalTokens + sysTokens + toolTokens matches
the persisted context; update the sysTokens calculation to use the rendered
agentInstructions/cancelContextWindow content (and include any appended
structured response instructions) before summing into cancelContextStats.
Comment on lines
+77
to
+97
| export function toInternalStorageUrl(url: string): string { | ||
| let pathname: string; | ||
| try { | ||
| pathname = new URL(url).pathname; | ||
| } catch { | ||
| pathname = url; | ||
| } | ||
|
|
||
| if (!pathname.includes(STORAGE_PATH)) return url; | ||
|
|
||
| const storageIdx = url.indexOf(STORAGE_PATH); | ||
| if (storageIdx === -1) return url; | ||
|
|
||
| const internalOrigin = | ||
| process.env.CONVEX_CLOUD_URL ?? 'http://127.0.0.1:3210'; | ||
| const internalPrefix = internalOrigin.replace(/\/+$/, ''); | ||
|
|
||
| if (url.startsWith(internalPrefix)) return url; | ||
|
|
||
| return `${internalPrefix}${url.slice(storageIdx)}`; | ||
| } |
There was a problem hiding this comment.
Don't treat any /api/storage/ substring as trusted storage.
Lines 85 and 107 use includes, so URLs like https://evil.example/foo/api/storage/<id> are classified as storage, rewritten to CONVEX_CLOUD_URL, and then skipped by the sandbox allowlist via resolveAndValidateUrl(). Only rewrite/bypass validation when the path matches the real storage route (including any configured BASE_PATH) on one of your own public/internal origins.
Also applies to: 105-110
Comment on lines
+25
to
+37
| type FormFieldInput = { | ||
| name: string; | ||
| value: string; | ||
| fileName?: string; | ||
| contentType?: string; | ||
| isBase64?: boolean; | ||
| }; | ||
|
|
||
| type BodyMethodOptions = HttpMethodOptions & { | ||
| body?: string; | ||
| binaryBody?: string; | ||
| /** Send as multipart/form-data */ | ||
| formFields?: FormFieldInput[]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n services/platform/convex/node_only/integration_sandbox/helpers/create_http_api.ts | head -80Repository: tale-project/tale
Length of output: 2957
🏁 Script executed:
cat services/platform/convex/node_only/integration_sandbox/types.tsRepository: tale-project/tale
Length of output: 3465
🏁 Script executed:
rg "FormFieldInput" services/platform/convex/node_only/integration_sandbox/Repository: tale-project/tale
Length of output: 281
Reuse FormField instead of the duplicate local type.
Line 68 requires a cast only because formFields is typed as FormFieldInput[], which duplicates the shape of the imported FormField interface. Using FormField[] directly removes the cast and prevents the two contracts from drifting.
Suggested fix
-type FormFieldInput = {
- name: string;
- value: string;
- fileName?: string;
- contentType?: string;
- isBase64?: boolean;
-};
-
type BodyMethodOptions = HttpMethodOptions & {
body?: string;
binaryBody?: string;
/** Send as multipart/form-data */
- formFields?: FormFieldInput[];
+ formFields?: FormField[];
};
@@
- formFields: isForm ? (options.formFields as FormField[]) : undefined,
+ formFields: isForm ? options.formFields : undefined,Per coding guidelines: "Do not use type casting (as). Use type guards, generics, or proper type narrowing instead."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type FormFieldInput = { | |
| name: string; | |
| value: string; | |
| fileName?: string; | |
| contentType?: string; | |
| isBase64?: boolean; | |
| }; | |
| type BodyMethodOptions = HttpMethodOptions & { | |
| body?: string; | |
| binaryBody?: string; | |
| /** Send as multipart/form-data */ | |
| formFields?: FormFieldInput[]; | |
| type BodyMethodOptions = HttpMethodOptions & { | |
| body?: string; | |
| binaryBody?: string; | |
| /** Send as multipart/form-data */ | |
| formFields?: FormField[]; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/convex/node_only/integration_sandbox/helpers/create_http_api.ts`
around lines 25 - 37, Replace the duplicated local type FormFieldInput with the
existing imported FormField interface and update usages: change the
BodyMethodOptions.formFields type from FormFieldInput[] to FormField[] and
remove any casts that were added to satisfy the older duplicate type; update any
references to FormFieldInput to use FormField so the code (e.g., where
BodyMethodOptions and formFields are used) no longer requires `as` casts and the
contracts stay consistent.
Comment on lines
+27
to
+33
| const safeName = field.name.replace(/"/g, '\\"'); | ||
|
|
||
| if (field.fileName) { | ||
| const safeFileName = field.fileName.replace(/"/g, '\\"'); | ||
| header += | ||
| `Content-Disposition: form-data; name="${safeName}"; filename="${safeFileName}"\r\n` + | ||
| `Content-Type: ${field.contentType || 'application/octet-stream'}\r\n\r\n`; |
There was a problem hiding this comment.
Strip CR/LF from multipart header values.
Lines 27-33 only escape quotes. field.name, field.fileName, and field.contentType can still inject \r/\n into the multipart headers/body, which lets untrusted input smuggle extra headers or parts upstream.
Suggested fix
+ const sanitizeMultipartHeaderValue = (value: string) =>
+ value.replace(/[\r\n]/g, '').replace(/"/g, '\\"');
+
for (const field of fields) {
let header = `--${boundary}\r\n`;
- const safeName = field.name.replace(/"/g, '\\"');
+ const safeName = sanitizeMultipartHeaderValue(field.name);
if (field.fileName) {
- const safeFileName = field.fileName.replace(/"/g, '\\"');
+ const safeFileName = sanitizeMultipartHeaderValue(field.fileName);
header +=
`Content-Disposition: form-data; name="${safeName}"; filename="${safeFileName}"\r\n` +
- `Content-Type: ${field.contentType || 'application/octet-stream'}\r\n\r\n`;
+ `Content-Type: ${sanitizeMultipartHeaderValue(field.contentType || 'application/octet-stream')}\r\n\r\n`;
} else {
header += `Content-Disposition: form-data; name="${safeName}"\r\n\r\n`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts`
around lines 27 - 33, Sanitize multipart header values by stripping CR and LF
characters from field.name, field.fileName, and field.contentType before
building the header: replace any \r or \n in field.name and field.fileName (used
to compute safeName and safeFileName) and in field.contentType with an empty
string so header concatenation for Content-Disposition and Content-Type cannot
be CR/LF-injected; update the logic around safeName, safeFileName and where
header is built in execute_http_request.ts to perform this cleanse in addition
to the existing quote-escaping.
Comment on lines
+69
to
+76
| if (effectiveReq.formFields && effectiveReq.formFields.length > 0) { | ||
| const { body, contentType } = buildFormData(effectiveReq.formFields); | ||
| const headers = { ...effectiveReq.options.headers } as Record< | ||
| string, | ||
| string | ||
| >; | ||
| headers['Content-Type'] = contentType; | ||
| fetchOptions = { ...effectiveReq.options, headers, body }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts | head -100Repository: tale-project/tale
Length of output: 4161
🏁 Script executed:
wc -l services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.tsRepository: tale-project/tale
Length of output: 151
🏁 Script executed:
cat -n services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts | sed -n '50,90p'Repository: tale-project/tale
Length of output: 1769
🏁 Script executed:
find . -name "types.ts" -o -name "types.d.ts" | grep -i "integration_sandbox"Repository: tale-project/tale
Length of output: 126
🏁 Script executed:
cat -n services/platform/convex/node_only/integration_sandbox/types.ts | head -100Repository: tale-project/tale
Length of output: 3694
🏁 Script executed:
python3 << 'EOF'
# Check what HeadersInit can be according to Fetch spec
# and verify the issue with spreading Headers
info = """
According to Fetch API spec, HeadersInit can be:
1. A Headers instance
2. A record/object (Record<string, string>)
3. An iterable of [string, string][]
When you spread a Headers instance:
- Headers is not a plain object
- It stores headers in an internal structure
- Spreading does not access those internal values
- Result: empty object or wrong keys
"""
print(info)
EOFRepository: tale-project/tale
Length of output: 415
🏁 Script executed:
# Check if Headers is used elsewhere in the codebase
rg "new Headers" services/platform/convex/node_only/integration_sandbox/ -A 2 -B 2Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check the context around where fetchOptions is used
cat -n services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts | sed -n '84,87p'Repository: tale-project/tale
Length of output: 201
🏁 Script executed:
# Check where HttpRequest is created/called from
rg "executeHttpRequest" services/platform/convex/node_only/integration_sandbox/ -B 3 -A 3Repository: tale-project/tale
Length of output: 10755
🏁 Script executed:
# Search for where HttpRequest objects are constructed
rg "HttpRequest" services/platform/convex/node_only/integration_sandbox/ --type ts -B 2 -A 2 | head -100Repository: tale-project/tale
Length of output: 9404
🏁 Script executed:
# Check if Headers constructor is used in the codebase
rg "new Headers" services/platform/convex/node_only/integration_sandbox/ --type tsRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check for any fetch calls or RequestInit creation patterns
rg "RequestInit\|fetch(" services/platform/convex/node_only/integration_sandbox/ --type ts -B 2 -A 2 | head -80Repository: tale-project/tale
Length of output: 149
🏁 Script executed:
cat -n services/platform/convex/node_only/integration_sandbox/helpers/create_http_api.ts | head -150Repository: tale-project/tale
Length of output: 4292
🏁 Script executed:
# Check what type of headers are being passed when creating HttpRequest
grep -A 20 "const request: HttpRequest = {" services/platform/convex/node_only/integration_sandbox/helpers/create_http_api.ts | head -60Repository: tale-project/tale
Length of output: 1388
🏁 Script executed:
# Check if there are any tests that might pass Headers instances
rg "new Headers" services/platform/convex/node_only/integration_sandbox/__tests__/ -A 3 -B 3Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check test files for how options are created
cat -n services/platform/convex/node_only/integration_sandbox/__tests__/allowed_hosts.test.ts | sed -n '1,50p'Repository: tale-project/tale
Length of output: 1889
🏁 Script executed:
# Verify what RequestInit type allows for headers
python3 << 'EOF'
# RequestInit from DOM/Fetch spec allows:
# headers?: HeadersInit
# Where HeadersInit = Headers | Record<string, string> | [string, string][]
info = """
RequestInit type signature (from Fetch API):
- headers?: HeadersInit
- HeadersInit can be one of:
1. Headers instance
2. Record<string, string> (plain object)
3. Array of [string, string][] (tuples)
Current code in execute_http_request.ts (lines 71-74):
const headers = { ...effectiveReq.options.headers } as Record<string, string>;
ISSUE:
If effectiveReq.options.headers is:
1. A Headers instance: spread operator returns {} (loses all headers)
2. An array of tuples: spread operator returns {0: [...], 1: [...]} (numeric keys, breaks)
3. A plain object: spread works correctly
The code casts result to Record<string, string>, but:
- If Headers was spread, result is empty object
- If array was spread, result has numeric keys instead of string keys
IMPACT: Auth headers (Authorization, etc.) could be silently dropped.
"""
print(info)
EOFRepository: tale-project/tale
Length of output: 893
🏁 Script executed:
# Check if Headers is used or available in this Node.js environment
rg "Headers" services/platform/convex/node_only/integration_sandbox/ --type tsRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check package.json to see what Node version and environment this targets
cat services/platform/convex/package.json | head -30Repository: tale-project/tale
Length of output: 130
🏁 Script executed:
# Check if fetch/Headers are polyfilled or if they're using global fetch
grep -r "globalThis.fetch\|fetch" services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.tsRepository: tale-project/tale
Length of output: 303
🏁 Script executed:
# Verify the exact type definition in TypeScript
python3 << 'EOF'
# Verify the fix: new Headers() constructor behavior
info = """
Verification of the suggested fix:
Current problematic code:
const headers = { ...effectiveReq.options.headers } as Record<string, string>;
Issues with spreading HeadersInit:
1. If Headers instance: spread returns empty {} (Headers is not iterable)
2. If [key, value][]: spread returns {0: [...], 1: [...]} (loses key structure)
3. If plain object: works fine
Suggested fix:
const headers = new Headers(effectiveReq.options.headers);
headers.set('Content-Type', contentType);
Why this works:
- Headers constructor accepts HeadersInit (any of the three forms)
- It normalizes them into a Headers instance
- .set() method works on Headers objects
- When passed to fetch(), Headers objects are properly handled
However, the return type would change:
- Old code: Record<string, string> (plain object)
- New code: Headers instance
Both are valid HeadersInit values for fetch(), so this is compatible.
HOWEVER - need to verify that this approach preserves the headers correctly
when passed to fetch in the final call.
"""
print(info)
EOFRepository: tale-project/tale
Length of output: 1081
🏁 Script executed:
# Let's trace through to confirm headers are preserved when passed to fetch
# Check the actual flow of how fetchOptions is used
cat -n services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts | sed -n '67,87p'Repository: tale-project/tale
Length of output: 1034
🏁 Script executed:
# Check if there's any test showing headers being passed
rg "headers.*Authorization\|headers.*api" services/platform/convex/node_only/integration_sandbox/__tests__/ -A 3 -B 3Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Verify that Headers can be used in this environment
python3 << 'EOF'
# Check Node.js and Fetch API compatibility
info = """
Key Points to Verify:
1. CURRENT CODE PROBLEM (lines 71-74):
const headers = { ...effectiveReq.options.headers } as Record<string, string>;
- Spreads effectiveReq.options.headers which is typed as HeadersInit | undefined
- If HeadersInit is a Headers instance: spread returns {} (empty)
- If HeadersInit is [key, value][]: spread returns numeric keys
- Type cast to Record<string, string> masks the issue at compile time
2. FETCH FLOW:
- Line 67: fetchOptions = effectiveReq.options (initial value)
- Line 76: fetchOptions = { ...effectiveReq.options, headers, body }
- Line 84-87: await globalThis.fetch(..., { ...fetchOptions, redirect: 'manual' })
The headers object is passed inside fetchOptions, not spread again.
So as long as headers is a valid HeadersInit for fetch(), it works.
3. SUGGESTED FIX:
const headers = new Headers(effectiveReq.options.headers);
headers.set('Content-Type', contentType);
- Headers constructor accepts any HeadersInit
- Normalizes all forms (Headers, object, array) into a Headers instance
- .set() method properly adds the Content-Type header
- Headers instance is valid HeadersInit for fetch()
This fix is CORRECT.
4. CONCERN:
In create_http_api.ts, headers are always plain objects (Record<string, string>)
But RequestInit type signature allows HeadersInit which includes Headers instances
So theoretically, outside code could pass a Headers instance
"""
print(info)
EOFRepository: tale-project/tale
Length of output: 1527
🏁 Script executed:
# Check if the headers parameter type is actually HeadersInit or restricted
rg "RequestInit" services/platform/convex/node_only/integration_sandbox/ -A 5 -B 2 | head -40Repository: tale-project/tale
Length of output: 4555
🏁 Script executed:
# Final check: verify that spreading a Headers instance actually loses headers
python3 << 'EOF'
# Test the actual behavior of spreading Headers
# Since we can't run node here, verify the logic based on JavaScript spec
info = """
FINAL VERIFICATION:
Based on the Fetch API specification and JavaScript behavior:
1. Headers is a non-enumerable object
- It stores data internally, not as object properties
- Spreading it with { ...headers } results in {}
- Example: new Headers({a: '1'}) spread = {} (loses the header)
2. Array of tuples spread behavior
- [['a', '1'], ['b', '2']] spread = {0: ['a', '1'], 1: ['b', '2']}
- This creates numeric keys, not string keys
- Example: headers[0] would be undefined, headers['0'] would be the tuple
3. Current code impact
- Line 71-74: spreading could produce empty object or numeric keys
- Line 75: adding Content-Type to potentially empty/wrong headers object
- Line 76: passing malformed headers to fetch
- Auth headers (Authorization, etc.) would be dropped
4. The fix is correct
- new Headers(effectiveReq.options.headers) handles all HeadersInit forms
- .set() adds the boundary correctly
- Headers instance is valid for fetch()
CONCLUSION: Review comment is CORRECT and VALID.
The issue is real despite current code only passing plain objects internally,
because the type signature allows HeadersInit which includes problematic forms.
"""
print(info)
EOFRepository: tale-project/tale
Length of output: 1258
Preserve existing headers when switching to multipart.
When effectiveReq.options.headers is a Headers instance or [key, value][] array (both valid HeadersInit forms per the Fetch API), spreading with { ...effectiveReq.options.headers } loses all headers or converts them to numeric keys. Use new Headers(...) to properly normalize any HeadersInit form and then call .set() to add the boundary:
Suggested fix
if (effectiveReq.formFields && effectiveReq.formFields.length > 0) {
const { body, contentType } = buildFormData(effectiveReq.formFields);
- const headers = { ...effectiveReq.options.headers } as Record<
- string,
- string
- >;
- headers['Content-Type'] = contentType;
+ const headers = new Headers(effectiveReq.options.headers);
+ headers.set('Content-Type', contentType);
fetchOptions = { ...effectiveReq.options, headers, body };
} else if (effectiveReq.binaryBody) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (effectiveReq.formFields && effectiveReq.formFields.length > 0) { | |
| const { body, contentType } = buildFormData(effectiveReq.formFields); | |
| const headers = { ...effectiveReq.options.headers } as Record< | |
| string, | |
| string | |
| >; | |
| headers['Content-Type'] = contentType; | |
| fetchOptions = { ...effectiveReq.options, headers, body }; | |
| if (effectiveReq.formFields && effectiveReq.formFields.length > 0) { | |
| const { body, contentType } = buildFormData(effectiveReq.formFields); | |
| const headers = new Headers(effectiveReq.options.headers); | |
| headers.set('Content-Type', contentType); | |
| fetchOptions = { ...effectiveReq.options, headers, body }; |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 71-74: typescript-eslint (no-unsafe-type-assertion): Unsafe type assertion; type 'Record<string, string>' is more narrow than the original type.
[error] 71-72: typescript-eslint (no-misused-spread): Using the spread operator on an array in an object will result in a list of indices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/convex/node_only/integration_sandbox/helpers/execute_http_request.ts`
around lines 69 - 76, The code that builds multipart form requests loses or
mangles existing headers by spreading effectiveReq.options.headers; instead
normalize headers via const headers = new Headers(effectiveReq.options.headers)
(so any HeadersInit is preserved), then call headers.set('Content-Type',
contentType) to add the boundary, and pass that Headers instance into
fetchOptions (e.g., fetchOptions = { ...effectiveReq.options, headers, body });
update the block around effectiveReq.formFields/buildFormData to use new
Headers(...) and .set() rather than object spread so all original headers
survive.
larryro
force-pushed
the
feat/ai-image-integration
branch
2 times, most recently
from
73bb08f to
e207c4c
Compare
Add a new ai-image integration type with connector, config, and example agent. Implement trigger-based completion actions, URL rewriting for sandboxed HTTP requests, and an enhanced approval card UI for image rendering. Update credentials schema, sandbox helpers, and agent response pipeline to support the new integration capabilities.
…bility Address findings from comprehensive code review (20 agents + 8 verification): Security: - Fix SSRF bypass in isStorageUrl/toInternalStorageUrl — match /api/storage/ in URL pathname only, not query params or fragments - Remove overly broad blob.core.windows.net from allowedHosts (unused — all models return b64_json) - Escape quotes in multipart form field names/filenames to prevent header injection - Move model from secretBindings to connectionConfig (not a credential) DRY / Architecture: - Extract triggerCompletionResponseHandler into approval_shared.ts — eliminates 95% duplication between integration and workflow completion mutations - Extract IntegrationOperationMetadataLocal to shared types.ts - Extract resolveAndValidateUrl helper to deduplicate URL validation pattern - Extract processImageItems, safeParseJson, validateN helpers in connector - Consolidate storage URL utils (toInternalStorageUrl, isStorageUrl) into lib/helpers/public_storage_url.ts alongside existing toPublicUrl Reliability: - Add try-catch in trigger_completion_action — saves error to thread on failure - Wrap response.json() in try-catch in connector (handles non-JSON responses) - Enforce MODEL_CAPS.maxN validation before API calls - Fix connectionConfig merge (use spread instead of ?? to preserve defaults) - Add metadata + organizationId to approval card memo comparison - Add requiresApproval field to REST API operation extraction - Replace hardcoded image instruction in buildCompletionMessage with generic text
…rtions Remove unnecessary type assertions where the type already matches, replace unsafe assertions with proper type narrowing or oxlint disable comments where the underlying Convex schema uses v.any(), normalize HeadersInit handling in execute_http_request, and remove unused imports.
…odal connectionConfig fields (e.g. model, timeout) were invisible in the UI after being moved out of secretBindings. Now auto-derive editable fields from connectionConfig entries, infer input type from JS typeof, and render them in a "Configuration" section in the setup modal. Also display values in the active/details view. Schemas updated with catchall to preserve custom fields through validation.
The LLM was describing generated images in text instead of rendering them. Add explicit instructions to use markdown image syntax for fileReferences so images display inline in chat.
Only dall-e-2 supports n>1; the default gpt-image-1 and dall-e-3 are capped at 1. Remove n from parametersSchema to simplify the approval card and avoid confusion.
…messages Add INTEGRATION_OPERATION_COMPLETED (info) and INTEGRATION_OPERATION_FAILED (error) to system message tag registry so they render with proper styling instead of raw text.
Tell the LLM not to generate suggested next steps when an approval card has been created, since the user needs to act on the card first.
larryro
force-pushed
the
feat/ai-image-integration
branch
from
643517d to
2bfe8d9
Compare
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
This was referenced Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ai-imageintegration type with connector, config, and example agent that supports DALL-E and gpt-image modelsapproval_shared.ts,url_rewrite.ts, storage URL consolidation) to reduce duplication across integration and workflow completion pathsTest plan
npm run lint --workspace=@tale/platformandnpx tsc --noEmitfromservices/platform/Summary by CodeRabbit
New Features
Bug Fixes