Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBump to v0.6.0 with AI SDK v6 migration: introduce Output interface and LanguageModelV3 surfaces, add embeddingModel (deprecated textEmbeddingModel fallback), async UI message processing, tool approval workflow, mediaType replacing mimeType, expanded validators/content shapes, numerous test and docs updates, and a migration guide. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Agent as Agent
participant Tool as Tool
participant Approval as ApprovalHandler
User->>Agent: Trigger tool call (with `input`)
Agent->>Tool: Emit tool-call (toolCallId, input)
Agent->>Approval: needsApproval?(input, ctx)
alt approval required
Approval->>Agent: Emit tool-approval-request (approvalId)
Agent->>User: Present approval request
User->>Agent: Send approval response
Agent->>Approval: Deliver tool-approval-response (approved?/reason)
alt approved
Agent->>Tool: Execute tool (authorized)
Tool-->>Agent: Return tool-result
else denied
Agent-->>User: Emit execution-denied output
end
else no approval required
Agent->>Tool: Execute tool
Tool-->>Agent: Return tool-result
end
Agent->>User: Deliver final result (tool-result / execution-denied)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/client/createTool.ts (1)
287-298: Potential issue: lifecycle hooks bound with possibly undefined ctx.The hooks
onInputStart,onInputDelta,onInputAvailable, andtoModelOutputare bound withgetCtx(t)immediately after tool creation. At this point, ifdef.ctxwas not provided, the ctx will beundefined. WhenwrapToolslater sets the ctx, these already-bound hooks won't receive the updated context.This differs from the
executeandneedsApprovalhandlers which callgetCtx(this)at invocation time, correctly picking up the injected ctx.🔧 Proposed fix to defer ctx resolution
- if (def.onInputStart) { - t.onInputStart = def.onInputStart.bind(t, getCtx(t)); - } - if (def.onInputDelta) { - t.onInputDelta = def.onInputDelta.bind(t, getCtx(t)); - } - if (def.onInputAvailable) { - t.onInputAvailable = def.onInputAvailable.bind(t, getCtx(t)); - } - if (def.toModelOutput) { - t.toModelOutput = def.toModelOutput.bind(t, getCtx(t)); - } + if (def.onInputStart) { + const originalOnInputStart = def.onInputStart; + t.onInputStart = function (this: Tool<INPUT, OUTPUT>, options) { + return originalOnInputStart(getCtx(this), options); + }; + } + if (def.onInputDelta) { + const originalOnInputDelta = def.onInputDelta; + t.onInputDelta = function (this: Tool<INPUT, OUTPUT>, options) { + return originalOnInputDelta(getCtx(this), options); + }; + } + if (def.onInputAvailable) { + const originalOnInputAvailable = def.onInputAvailable; + t.onInputAvailable = function (this: Tool<INPUT, OUTPUT>, options) { + return originalOnInputAvailable(getCtx(this), options); + }; + } + if (def.toModelOutput) { + const originalToModelOutput = def.toModelOutput; + t.toModelOutput = function (this: Tool<INPUT, OUTPUT>, options) { + return originalToModelOutput(getCtx(this), options); + }; + }src/client/mockModel.ts (1)
240-243: Duplicate assignment of_supportedUrls.This assignment is a duplicate of lines 205-208. The second assignment is redundant and should be removed.
🧹 Proposed fix
return { stream, request: { body: {} }, response: { headers: {} }, }; } else if (args.doStream) { return args.doStream; } else { throw new Error("Provide either content or doStream"); } }; - this._supportedUrls = - typeof supportedUrls === "function" - ? supportedUrls - : async () => supportedUrls; }src/client/search.ts (1)
238-317: Keep tool-calls when approval requests exist.When a
tool-approval-requestis present but notool-approval-responseyet, tool-calls are currently filtered out. Sincetool-approval-requestonly containsapprovalIdand not the tool details, this removes critical context (tool name/args) needed during the approval phase. AI SDK v6 guidance emphasizes retaining tool calls as structured parts within the message stream so the model can see what happened. Consider keeping tool-calls whenever an approval request exists by adding ahasApprovalRequestcheck alongside the existinghasApprovalResponsecondition.
🤖 Fix all issues with AI agents
In `@docs/agent-usage.mdx`:
- Line 46: The docs have inconsistent import sources for stepCountIs (imported
from "@convex-dev/agent" on one example and from "ai" in another); update all
examples to use the same import path (pick one and apply consistently) — e.g.,
change the import that currently pulls stepCountIs from "@convex-dev/agent" so
it imports stepCountIs (and Agent if applicable) from "ai", and ensure every
other example in the file uses that same source for stepCountIs to avoid
confusion.
In `@example/convex/modelsForDemo.ts`:
- Around line 11-25: The exported variable textEmbeddingModel can be undefined
because it’s only set inside the OPENAI_API_KEY branch; update the
initialization so consumers always get a valid value: either assign a fallback
embedding model (e.g., use mockModel({}) or an embedding adapter) when
ANTHROPIC_API_KEY or GROQ_API_KEY is used, or explicitly type/export
textEmbeddingModel as optional and ensure callers handle undefined. Locate
textEmbeddingModel and languageModel in the file and initialize
textEmbeddingModel outside the conditional (or add a conditional export) so it’s
never unintentionally undefined at runtime.
In `@package.json`:
- Around line 72-73: Update the dependency version constraints for the two AI
SDK packages: change "ai" from ^6.0.35 to ^6.0.49 and "@ai-sdk/provider-utils"
from ^4.0.6 to ^4.0.9 in package.json, then run your package manager
(npm/yarn/pnpm) to install the updated versions and run the test/build pipeline
to ensure compatibility; target the dependency entries "ai" and
"@ai-sdk/provider-utils" when making the edits.
In `@src/deltas.ts`:
- Around line 521-526: Restore the compile-time exhaustiveness check for the
TextStreamPart switch by uncommenting the `const _: never = part` line in the
default branch (or, if you intentionally want to disable it, add a clear comment
explaining why and under what conditions that will remain safe); if uncommenting
produces TypeScript errors, fix them by adding any missing case handlers for the
new TextStreamPart variants or narrowing the switch's discriminant so the
assignment to `never` type succeeds, ensuring future ai SDK additions produce
compile-time failures rather than only runtime warnings.
In `@src/mapping.ts`:
- Around line 161-163: Replace the placeholder empty-object assignments for
token detail fields with undefined to match the SDK v6 optional pattern: change
any occurrences where inputTokenDetails: {} as any and outputTokenDetails: {} as
any are used (e.g., the object in mapping.ts and the similar spots in search.ts
around the search result mapping) so they initialize to undefined instead;
ensure the surrounding type annotations accept undefined (adjust union types to
include undefined if necessary) and run type checks to confirm no downstream
code assumes an object.
In `@src/react/useThreadMessages.ts`:
- Around line 244-261: The effect using useEffect with uiMessages, args,
setMessages and fromUIMessages has a race where an earlier async result can
overwrite a later one; replace the isMounted guard with a generation counter
stored in a useRef (import useRef) or an abort token: increment the ref at the
start of each effect run, capture the current generation in the async closure,
and only call setMessages(nested.flat()) if the captured generation matches the
ref.current; ensure you still clean up by incrementing or invalidating the ref
in the return function so stale promises never apply their results.
🧹 Nitpick comments (5)
src/component/files.ts (1)
49-58: Consider storing resolved value inmimeTypefor full backwards compatibility.Currently,
mimeTypeis only stored if the caller explicitly passes it. If a caller uses onlymediaType, existing code that readsfile.mimeTypewill getundefined. Consider storing the resolved value in both fields:♻️ Proposed fix
const fileId = await ctx.db.insert("files", { storageId: args.storageId, hash: args.hash, filename: args.filename, mediaType, - mimeType: args.mimeType, // Keep for backwards compatibility + mimeType: mediaType, // Keep for backwards compatibility // We start out with it unused - when it's saved in a message we increment. refcount: 0, lastTouchedAt: Date.now(), });src/client/files.ts (1)
86-89: Replacectx.storage.getMetadata()withctx.db.system.get("_storage", storageId).
ctx.storage.getMetadatais deprecated. Usectx.db.system.get("_storage", newStorageId)to retrieve file metadata (which includessha256). This returns the same metadata structure and works in both action and mutation contexts.src/deltas.ts (1)
493-511: Consider defining a proper type for the approval object.The type assertions here are understandable given SDK type constraints, but the
approvalproperty is typed loosely asobject. Consider defining an interface for better type safety:interface ToolApproval { id: string; }This would make the approval structure explicit and catch any mismatches at compile time.
src/react/useThreadMessages.ts (1)
261-261: Conditional expression in dependency array may trigger linter warnings.The expression
args === "skip" ? undefined : args.threadIdin the dependency array, while functional, is an unusual pattern that may confuse readers and potentially trigger ESLint'sreact-hooks/exhaustive-depsrule.Consider extracting to a variable for clarity:
const threadId = args === "skip" ? undefined : args.threadId; // ... then use [uiMessages, threadId] in depssrc/UIMessages.ts (1)
520-523: Consider consolidating the error text fallback.The
errorTextassignment has a subtle inconsistency: whenhasErroris true butmessage.erroris falsy andoutputis a non-string value,String(output)could produce"[object Object]"for object outputs.💡 Suggested improvement
- const errorText = - message.error || (hasError ? String(output) : undefined); + const errorText = + message.error || + (hasError + ? typeof output === 'string' + ? output + : JSON.stringify(output) ?? 'Unknown error' + : undefined);
|
|
||
| ```ts | ||
| import { Agent } from "@convex-dev/agent"; | ||
| import { Agent, stepCountIs } from "@convex-dev/agent"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the specific lines in the documentation file
head -n 210 docs/agent-usage.mdx | tail -n 170Repository: get-convex/agent
Length of output: 6651
🏁 Script executed:
# Search for where stepCountIs is exported from in the codebase
rg -n "export.*stepCountIs" --type ts --type jsRepository: get-convex/agent
Length of output: 116
🏁 Script executed:
# Also search for stepCountIs imports and usage throughout the codebase
rg "stepCountIs" -A 2 -B 2 --type ts --type js --type mdx | head -100Repository: get-convex/agent
Length of output: 87
Inconsistent import source for stepCountIs across code examples.
Line 46 imports stepCountIs from @convex-dev/agent, while the "Customizing the agent" section imports it from "ai". Although @convex-dev/agent re-exports the function from "ai", the documentation should consistently import from the same source across all examples to avoid confusing users about which import path to use.
🤖 Prompt for AI Agents
In `@docs/agent-usage.mdx` at line 46, The docs have inconsistent import sources
for stepCountIs (imported from "@convex-dev/agent" on one example and from "ai"
in another); update all examples to use the same import path (pick one and apply
consistently) — e.g., change the import that currently pulls stepCountIs from
"@convex-dev/agent" so it imports stepCountIs (and Agent if applicable) from
"ai", and ensure every other example in the file uses that same source for
stepCountIs to avoid confusion.
| if (process.env.ANTHROPIC_API_KEY) { | ||
| languageModel = anthropic.chat("claude-opus-4-20250514"); | ||
| } else if (process.env.OPENAI_API_KEY) { | ||
| languageModel = openai.chat("gpt-4o-mini"); | ||
| textEmbeddingModel = openai.textEmbeddingModel("text-embedding-3-small"); | ||
| textEmbeddingModel = openai.embedding("text-embedding-3-small"); | ||
| } else if (process.env.GROQ_API_KEY) { | ||
| languageModel = groq.languageModel( | ||
| "meta-llama/llama-4-scout-17b-16e-instruct", | ||
| ); | ||
| } else if (process.env.OPENROUTER_API_KEY) { | ||
| languageModel = openrouter.chat("openai/gpt-4o-mini") as LanguageModelV2; | ||
| } else { | ||
| languageModel = mockModel({}); | ||
| console.warn( | ||
| "Run `npx convex env set GROQ_API_KEY=<your-api-key>` or `npx convex env set OPENAI_API_KEY=<your-api-key>` or `npx convex env set OPENROUTER_API_KEY=<your-api-key>` from the example directory to set the API key.", | ||
| "Run `npx convex env set GROQ_API_KEY=<your-api-key>` or `npx convex env set OPENAI_API_KEY=<your-api-key>` from the example directory to set the API key.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
textEmbeddingModel may be undefined at runtime.
The textEmbeddingModel variable is only assigned when OPENAI_API_KEY is set, but it's exported unconditionally. If ANTHROPIC_API_KEY or GROQ_API_KEY is used instead, consumers importing textEmbeddingModel will get undefined.
Consider one of these approaches:
- Initialize a fallback/mock embedding model for non-OpenAI providers
- Export it conditionally or make it explicitly optional in the type
🔧 Suggested fix: Add explicit undefined handling
-let textEmbeddingModel: EmbeddingModel;
+let textEmbeddingModel: EmbeddingModel | undefined;Or provide a fallback for other providers if embedding functionality is needed.
🤖 Prompt for AI Agents
In `@example/convex/modelsForDemo.ts` around lines 11 - 25, The exported variable
textEmbeddingModel can be undefined because it’s only set inside the
OPENAI_API_KEY branch; update the initialization so consumers always get a valid
value: either assign a fallback embedding model (e.g., use mockModel({}) or an
embedding adapter) when ANTHROPIC_API_KEY or GROQ_API_KEY is used, or explicitly
type/export textEmbeddingModel as optional and ensure callers handle undefined.
Locate textEmbeddingModel and languageModel in the file and initialize
textEmbeddingModel outside the conditional (or add a conditional export) so it’s
never unintentionally undefined at runtime.
| "@ai-sdk/provider-utils": "^4.0.6", | ||
| "ai": "^6.0.35", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions of AI SDK packages
echo "Checking ai package:"
curl -s https://registry.npmjs.org/ai | jq '.["dist-tags"].latest'
echo ""
echo "Checking `@ai-sdk/provider-utils` package:"
curl -s https://registry.npmjs.org/@ai-sdk/provider-utils | jq '.["dist-tags"].latest'Repository: get-convex/agent
Length of output: 139
Update AI SDK packages to current patch versions for latest improvements and bug fixes.
The specified versions (ai@^6.0.35 and @ai-sdk/provider-utils@^4.0.6) are stable but outdated. Latest available versions are ai@6.0.49 and @ai-sdk/provider-utils@4.0.9. Consider updating the version constraints to use current releases.
🤖 Prompt for AI Agents
In `@package.json` around lines 72 - 73, Update the dependency version constraints
for the two AI SDK packages: change "ai" from ^6.0.35 to ^6.0.49 and
"@ai-sdk/provider-utils" from ^4.0.6 to ^4.0.9 in package.json, then run your
package manager (npm/yarn/pnpm) to install the updated versions and run the
test/build pipeline to ensure compatibility; target the dependency entries "ai"
and "@ai-sdk/provider-utils" when making the edits.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/createTool.ts (1)
309-320: Hooks are bound with undefined ctx when tools are created without context.When a tool is created without
ctxat definition time (relying onwrapToolsto inject it later), the hooks (onInputStart,onInputDelta,onInputAvailable,toModelOutput) are bound immediately withgetCtx(t)which returnsundefined:t.onInputStart = def.onInputStart.bind(t, getCtx(t)); // undefined is captured hereUnlike
executeandneedsApprovalwhich are inline methods that checkgetCtx(this)at call time (and throw if missing), the hooks are bound functions that permanently capture theundefinedvalue. WhenwrapToolslater spreads the tool object and setsctx, it only updates thectxproperty—the bound hooks still haveundefinedcaptured from creation time.The hooks should either be re-bound in
wrapToolswith the injected context, or they should include the same defensive checks asexecuteandneedsApprovalto ensure ctx is available at call time.
🤖 Fix all issues with AI agents
In `@CLAUDE.md`:
- Line 9: Replace the bare URL "https://docs.convex.dev/agents" with a proper
Markdown link in the CLAUDE.md content (e.g., [Convex Agents
Documentation](https://docs.convex.dev/agents)) so the URL is not bare and
matches project markdown style; update the line that currently contains the bare
URL accordingly.
🧹 Nitpick comments (5)
CLAUDE.md (1)
90-168: Consider referencing MIGRATION.md instead of duplicating.The upgrade section largely duplicates content from
MIGRATION.md. Consider referencing it to reduce maintenance burden:## Upgrading to v0.6.0 (AI SDK v6) See [MIGRATION.md](./MIGRATION.md) for detailed upgrade steps.This keeps the authoritative migration content in one place.
src/client/createTool.ts (2)
14-21: Duplicated deprecation utility.This
warnDeprecationfunction andMIGRATION_URLconstant are duplicated insrc/client/index.ts(lines 22-29). Consider extracting to a shared utility module to avoid duplication.Suggested approach
Create a shared utility file:
// src/client/deprecation.ts export const MIGRATION_URL = "https://github.com/get-convex/agent/blob/main/MIGRATION.md"; const warnedDeprecations = new Set<string>(); export function warnDeprecation(key: string, message: string) { if (!warnedDeprecations.has(key)) { warnedDeprecations.add(key); console.warn(`[`@convex-dev/agent`] ${message}\n See: ${MIGRATION_URL}`); } }Then import in both files.
69-73: Add a brief comment explaining the type's purpose.This conditional type is non-trivial. A short comment would aid future maintainers:
Suggested comment
+/** + * Makes T's properties optional when N is `any` or `never`, + * enabling tools without explicit output types to omit execute/outputSchema. + */ type NeverOptional<N, T> = 0 extends 1 & N ? Partial<T> : [N] extends [never] ? Partial<Record<keyof T, undefined>> : T;src/client/index.ts (2)
445-445: Type assertionas anybypasses type safety.The
stopWhenproperty is cast toanyhere and at line 573. While this may be necessary for complex generic interactions with AI SDK v6, it could mask type errors. Consider adding a comment explaining why the cast is needed, or using a more specific type assertion if possible.
1462-1466: Type assertion for customCtx invocation.The
as anycast onllmArgsis consistent with other type handling patterns in this file. Consider documenting why these casts are necessary (likely due to complex generic interactions with AI SDK v6 types).
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@zboyles I got tool approvals working, I'd love your thoughts. It was tricky given the translation required between AI SDK 6's api, and what anthropic wants to see in the messages with regards to tool ids. |
|
@sethconvex Excellent, I appreciate you pulling that through! I saw someone was having an issue, vercel/ai#11966, with Google's parallel calls hitting a race condition with AI SDK generating the same tool call id for multiple calls. I also think the AI SDK added the tool call id into those approval response types if I read their change log correctly, or maybe the data was there and they added the type definition. In any case, I'll be able to install this and run tests later this afternoon and will let you know how it goes. Thanks again. |
|
Hey team, what's the update here? This is critical to run "modern" models like gemini 2.5/3 flash. Thanks. |
|
Hi everyone, is there an update on how long it will take to get this merged? There are a few features we need from v6, and it would be super helpful for us! Thanks so much. |
|
There are some outstanding issues in the way we're handling tool approvals.
They are on my list for this week. Hopefully a more stable rc gets shipped
this week!
…On Sat, Feb 7, 2026 at 2:26 PM mainpingdream ***@***.***> wrote:
*mainpingdream* left a comment (get-convex/agent#216)
<#216 (comment)>
Hi everyone, is there an update on how long it will take to get this
merged?
There are a few features we need from v6, and it would be super helpful
for us!
Thanks so much.
—
Reply to this email directly, view it on GitHub
<#216 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BZEPEC6H2EH2WZ6J724SWJD4KZRDBAVCNFSM6AAAAACSZMO3C2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNRVGY2TEMBWGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Just a bit of external validation, we refactored to AI SDK v6 and have been running this branch in our prod for about a week and haven't run across any issues. (I can't speak to the tool approval bugs as not using tool approval) |
|
Any ETA? |
|
I've already got the beta version of the agent component + tool approvals working in my app: tools marked Would love to see updated docs on the “recommended” approval pattern so I can sanity-check that I'm doing it the intended way. |
|
Can't wait for this to land |
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
overall lgtm - I suspect keeping mimeType will make our lives simpler for now - though the mapping is annoying I will admit
docs/agent-usage.mdx
Outdated
| }), | ||
| // Standard AI SDK tool | ||
| myTool: tool({ description, parameters, execute: () => {}}), | ||
| myTool: tool({ description, inputSchema: parameters, execute: () => {}}), |
There was a problem hiding this comment.
| myTool: tool({ description, inputSchema: parameters, execute: () => {}}), | |
| myTool: tool({ description, inputSchema: z.object(...), execute: () => {}}), |
| /** | ||
| * An optional list of input examples that show the language | ||
| * model what the input should look like. | ||
| */ | ||
| inputExamples?: Array<{ | ||
| input: NoInfer<INPUT>; | ||
| }>; | ||
| /** | ||
| * Whether the tool needs approval before it can be executed. | ||
| */ | ||
| needsApproval?: | ||
| | boolean | ||
| | ToolNeedsApprovalFunctionCtx< | ||
| [INPUT] extends [never] ? unknown : INPUT, | ||
| Ctx | ||
| >; |
There was a problem hiding this comment.
is there a reason we're copying over so many types here? Is this b/c of complex type inference on INPUT/OUTPUT/Ctx?
This seems pretty fragile, would be nice to lean on the AI SDK types whenever possible
| throw new Error( | ||
| "To use a Convex tool, you must either provide the ctx" + | ||
| " at definition time (dynamically in an action), or use the Agent to" + | ||
| " call it (which injects the ctx, userId and threadId)", | ||
| ); |
There was a problem hiding this comment.
would be nice to unify these checks
| mediaType: v.optional(v.string()), | ||
| /** @deprecated Use `mediaType` instead. */ | ||
| mimeType: v.optional(v.string()), |
There was a problem hiding this comment.
this is ok, but it might be simpler for the component world to just use mimeType and translate- since we're going to need to do dual-reads ~indefinitely with this approach anyways (or ask users to do a migration flow)
| * @returns | ||
| */ | ||
| export function fromUIMessages<METADATA = unknown>( | ||
| export async function fromUIMessages<METADATA = unknown>( |
There was a problem hiding this comment.
I would love to make this synchronous if at all possible - simplifies frontend a lot.. but I guess AI SDK made the call to make this async..
| mediaType: v.optional(v.string()), | ||
| /** @deprecated Use `mediaType` instead. */ | ||
| mimeType: v.optional(v.string()), |
There was a problem hiding this comment.
if we revert the schema to only use mimeType (which is what we did for v5 support already) we should revert this too
| type: v.literal("image"), | ||
| image: v.union(v.string(), v.bytes()), | ||
| mediaType: v.optional(v.string()), | ||
| /** @deprecated Use `mediaType` instead. */ |
There was a problem hiding this comment.
we can live with the dual-read world, but worth considering if having both of these greatly simplifies something to make it worth it - we generally need to do some wrangling with images to turn the buffers into the right kind, save to file storage, etc.
| v.object({ | ||
| type: v.literal("file-url"), | ||
| url: v.string(), | ||
| providerOptions, | ||
| }), |
There was a problem hiding this comment.
bummer all of these can't be shared with existing validators. In general the validator / types for all agent-related functions is going to be pretty gnarly for any function that accepts a vMessage.. not that re-using validators helps us with that (yet - would be nice to leverage shared types to simplify in our code generation but that's just a proposal rn)
| args: Infer<typeof addFileArgs>, | ||
| ) { | ||
| // Support both mediaType (preferred) and mimeType (deprecated) | ||
| const mediaType = args.mediaType ?? args.mimeType; |
There was a problem hiding this comment.
I don't see the equivalent of this for images - which also is trying to rename
| reason?: string; | ||
| type: "tool-approval-response"; | ||
| } | ||
| >; |
There was a problem hiding this comment.
wowza this is a big file - hopfully we can flip the legacyComponentApi default to false so folks don't get this in their api.d.ts when they upgrade
Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
- Updated component schema to use optional mediaType and mimeType - Updated files.ts addFile mutation to accept both mediaType and mimeType - Updated client files.ts to use mediaType when calling component - Updated getFile to support both mediaType and mimeType from database - Updated mapping.ts to return mediaType instead of mimeType - Maintained backwards compatibility throughout Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
- Updated all file part definitions in generated component.ts - Changed mimeType from required to optional - Added mediaType as optional field - Resolved all TypeScript type errors related to mimeType/mediaType Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
…t types Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
…in generated types. Ran `convex codegen --component-dir ./src/component`.
…itions for consistency
Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
…esult output types Co-authored-by: zboyles <2215540+zboyles@users.noreply.github.com>
- Implemented handling of `tool-approval-request` and `tool-approval-response` in `deltas.ts` to update tool state accordingly. - Enhanced tests in `deltas.test.ts` and `toUIMessages.test.ts` to verify the approval workflow, including state transitions for approval requests and responses. - Updated `UIMessages.ts` to manage tool approval states and responses, ensuring correct state updates for tool calls based on approval outcomes.
…t` and `tool-approval-response`.
…tToolApprovals`. Without the fix, the tool call was being filtered out because it had no tool-result, even though it had an approval response - which caused the SDK to fail at line 91 in `ai/src/generate-text/collect-tool-approvals.ts`.
…rove deprecation handling.
…result errors since stored messages have the complete data.
… tool approval, as stored messages contain complete context.
- Upgrade @convex-dev/rag from 0.6.1 to 0.7.0 RAG 0.7.0 uses ai@^6.0.0 (compatible with v6), while 0.6.x used AI SDK v5 types (EmbeddingModelV2) causing type conflicts - Fix unused type params in Output interface (src/client/types.ts) Prefix with _ to indicate intentionally unused for type signature compatibility - Remove unused vContent import (src/mapping.ts) Co-Authored-By: Claude <noreply@anthropic.com>
- Add MIGRATION.md with step-by-step upgrade instructions - Add CLAUDE.md for AI agent guidance - Add runtime deprecation warnings that fire once per session: - textEmbeddingModel → embeddingModel - createTool args → inputSchema - createTool handler → execute - Warnings link to migration guide on GitHub Co-Authored-By: Claude <noreply@anthropic.com>
- Fix race condition in useThreadMessages.ts using generation counter
- Replace {} as any with properly typed token detail objects
- Convert bare URL to markdown link in CLAUDE.md
- Add warning comment about textEmbeddingModel potentially being undefined
- Document why exhaustiveness check is disabled in deltas.ts (forwards compat)
Co-Authored-By: Claude <noreply@anthropic.com>
sethconvex
force-pushed
the
rc/ai-sdk-v6
branch
from
231f81f to
36cede8
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/agent-usage.mdx (1)
196-214:⚠️ Potential issue | 🟡 MinorDocumentation references deprecated
textEmbeddingModelinstead ofembeddingModel.Line 199 mentions "you'll need a
textEmbeddingModelmodel" and Line 214 usestextEmbeddingModelin the code example. According to the migration guide (Step 3),textEmbeddingModelis deprecated in favor ofembeddingModel. This doc section should be updated to match.Proposed fix
The agent by default only needs a `chat` model to be configured. However, for -vector search, you'll need a `textEmbeddingModel` model. A `name` is helpful to +vector search, you'll need an `embeddingModel` model. A `name` is helpful to attribute each message to a specific agent. Other options are defaults that can be over-ridden at each LLM call-site.// Embedding model to power vector search of message history (RAG). - textEmbeddingModel: openai.embedding("text-embedding-3-small"), + embeddingModel: openai.embedding("text-embedding-3-small"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agent-usage.mdx` around lines 196 - 214, The docs and example use the deprecated textEmbeddingModel property; update the prose and the sharedDefaults example to use embeddingModel instead (replace mentions of textEmbeddingModel with embeddingModel and set sharedDefaults.embeddingModel = openai.embedding("text-embedding-3-small")); ensure any explanatory text referencing "textEmbeddingModel" is changed to "embeddingModel" so the Agent config (e.g., languageModel and embeddingModel in sharedDefaults) matches the current API.src/toUIMessages.test.ts (1)
442-442:⚠️ Potential issue | 🟡 MinorDuplicate test description — rename the second test.
Both tests at Line 442 and Line 549 share the exact name
"sets text field correctly when message has many parts with text part as final part". While vitest runs both, they appear identically in reports, making failures ambiguous. The second test exercises a multi-step flow with adateTimetool call, so something like"sets text field correctly in multi-step tool-call + final-text flow"would distinguish them.✏️ Suggested rename
- it("sets text field correctly when message has many parts with text part as final part", () => { + it("sets text field correctly when multi-step tool call precedes final text part", () => { const messages = [ baseMessageDoc({ text: "what time is it in paris?",Also applies to: 549-549
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/toUIMessages.test.ts` at line 442, Rename the duplicate test description string used in the second it(...) block that currently reads "sets text field correctly when message has many parts with text part as final part" to a unique, descriptive name (e.g. "sets text field correctly in multi-step tool-call + final-text flow") so test reports are unambiguous; locate the second it(...) declaration in the toUIMessages.test file (the test exercising dateTime tool call) and replace its first argument (the description string) with the new unique string.src/UIMessages.ts (1)
58-68:⚠️ Potential issue | 🟠 Major
fromUIMessagesis now async — a breaking API change for external consumers.The function signature at lines 58-68 confirms this is now
export async functionreturningPromise<...>. Since it's exported from the public API (src/client/index.tsline 181), external downstream consumers who depend on this library will need to addawaitto their call sites. All internal callers in the codebase have already been updated to properly await the result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UIMessages.ts` around lines 58 - 68, The exported function fromUIMessages was changed to an async function, introducing a breaking API change; revert it to a synchronous export to maintain compatibility by removing the async modifier and returning the array directly (so callers that await still work). Locate the fromUIMessages function and its return expression, remove the async keyword and ensure it returns (MessageDoc & { streaming: boolean; metadata?: METADATA })[] instead of Promise<...>; keep the function name and signature generic parameters the same so the public export used in client/index.ts is unchanged. If asynchronous work is truly required internally, implement that as an internal helper (e.g., fromUIMessagesAsync) and call it from fromUIMessages in a synchronous manner or document a new async helper instead of changing the public API.src/client/search.ts (1)
449-473:⚠️ Potential issue | 🟠 MajorSemantic mismatch: embedding usage padded with
undefinedfields to satisfyLanguageModelUsage.The
usageHandlercallback signature expectsLanguageModelUsage(which requiresinputTokenDetails/outputTokenDetails), butEmbeddingModelUsageonly exposestokens. The objects withundefinedfields satisfy TypeScript structurally, but any usage handler that destructures or accumulates these detail fields will silently getNaN/undefinedwithout knowing why.A cleaner solution is to adjust
UsageHandler'susageparameter to a union or a shared base type that accommodates embedding-only payloads, or at minimum document theundefinedsemantics explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/search.ts` around lines 449 - 473, The current call to usageHandler in search.ts passes an object shaped as LanguageModelUsage but fills inputTokenDetails/outputTokenDetails with undefined to represent embedding-only usage; instead, update the UsageHandler type (or the type used in the handler signature) to accept a union or shared base between LanguageModelUsage and EmbeddingModelUsage (e.g., UsageBase | LanguageModelUsage | EmbeddingModelUsage) so embedding-only payloads can be passed without padded undefined fields, then change the call site that constructs the usage to pass an EmbeddingModelUsage instance (only tokens) or the appropriate union variant; also add a short comment/doc on EmbeddingModelUsage semantics so consumers know those detail fields won’t be present for embeddings.src/client/createTool.ts (1)
309-320:⚠️ Potential issue | 🔴 Critical
toModelOutput(and pre-existing lifecycle hooks) bindctxat creation time, so injected runtimectxis never seen.
bind(t, getCtx(t))capturest.ctx(which equalsdef.ctx, typicallyundefinedat definition time) as a fixed first argument. WhenwrapToolslater creates{ ...tool, ctx }and the AI SDK callsout.onInputStart(options)/out.toModelOutput(options), the bound function ignores the newout.ctxand passes the stale/undefined ctx to the user's handler.By contrast,
executeandneedsApprovalcorrectly readgetCtx(this)inside a regular function, so they see the runtime-injected ctx when called as methods on the spread object. The newtoModelOutputbinding introduced by this PR has the same flaw, and the pattern should be fixed consistently across all four hooks.if (def.onInputStart) { - t.onInputStart = def.onInputStart.bind(t, getCtx(t)); + t.onInputStart = function(this: Tool<INPUT, OUTPUT>, options: ToolExecutionOptions) { + return def.onInputStart!(getCtx(this), options); + }; } if (def.onInputDelta) { - t.onInputDelta = def.onInputDelta.bind(t, getCtx(t)); + t.onInputDelta = function(this: Tool<INPUT, OUTPUT>, options: { inputTextDelta: string } & ToolExecutionOptions) { + return def.onInputDelta!(getCtx(this), options); + }; } if (def.onInputAvailable) { - t.onInputAvailable = def.onInputAvailable.bind(t, getCtx(t)); + t.onInputAvailable = function(this: Tool<INPUT, OUTPUT>, options: { input: any } & ToolExecutionOptions) { + return def.onInputAvailable!(getCtx(this), options); + }; } if (def.toModelOutput) { - t.toModelOutput = def.toModelOutput.bind(t, getCtx(t)); + t.toModelOutput = function(this: Tool<INPUT, OUTPUT>, options: { toolCallId: string; input: any; output: any }) { + return def.toModelOutput!(getCtx(this), options); + }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/createTool.ts` around lines 309 - 320, The lifecycle hooks (toModelOutput, onInputStart, onInputDelta, onInputAvailable) are currently bound with getCtx(t) at creation time, capturing a stale ctx; change each binding to a runtime wrapper function that computes ctx via getCtx(this) when invoked (e.g., replace t.toModelOutput = def.toModelOutput.bind(t, getCtx(t)) with a regular function that calls def.toModelOutput.call(this, getCtx(this), ...args)), mirroring how execute and needsApproval obtain ctx, so when wrapTools spreads and injects ctx the handler sees the injected runtime ctx; update the same pattern for onInputStart, onInputDelta and onInputAvailable and keep use of .call/.apply to preserve original this (t) and argument forwarding.
🧹 Nitpick comments (10)
src/deltas.ts (1)
69-103: Error suppression via string matching is fragile.The
"no tool invocation found"substring check (Line 77) couples this logic to the exact error message wording in the AI SDK. If the message changes in a future SDK update, suppression silently stops working and errors will propagate instead — which is a safe failure mode, but may cause unexpected regressions in the tool-approval continuation flow.Consider matching on error type/code if available, or adding a comment noting the specific AI SDK version/source where this message originates so it can be tracked during upgrades.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/deltas.ts` around lines 69 - 103, The current suppression in the onError callback of readUIMessageStream relies on matching the literal substring "no tool invocation found" which is fragile; update onError to detect the condition by checking a stable identifier (e.g., an error.code property or specific Error subclass) rather than string matching, using properties of the thrown error (e.g., e.code, e.name, or instanceof SomeSdkError) and fall back to the string check only if those fields are absent; reference the symbols readUIMessageStream, onError, suppressError, and the "no tool invocation found" check when making the change, and if the SDK doesn't expose a stable code/type add a clear comment documenting the SDK and version/source of that message so future upgrades can revisit this logic.src/client/types.ts (1)
24-31:Outputinterface declared between two import blocks.The
Outputinterface (Lines 25–31) is placed between the"ai"import block (Lines 1–23) and the"convex/server"import block (Lines 32–38). While TypeScript accepts this, it deviates from the idiomatic pattern of grouping all imports at the top. Move the interface below the last import statement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/types.ts` around lines 24 - 31, Move the exported Output interface declaration so it appears after the last import block (i.e., below the "convex/server" import) instead of between import groups; locate the `export interface Output<_T = any, _P = any, _E = any>` declaration and cut/paste it below the final import, keeping its name and export unchanged and verifying any referenced types still resolve.src/client/mockModel.ts (1)
205-208:_supportedUrlsis initialized twice; the first assignment is dead code.Lines 205–208 assign
this._supportedUrls, thenthis.doStream = ...is assigned (lines 209–239), thenthis._supportedUrlsis assigned again with the identical expression at lines 240–243. The first assignment is always overwritten and can be removed.♻️ Proposed fix
- this._supportedUrls = - typeof supportedUrls === "function" - ? supportedUrls - : async () => supportedUrls; - this.doStream = async (options) => { + this.doStream = async (options) => {Also applies to: 240-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/mockModel.ts` around lines 205 - 208, Remove the redundant early initialization of this._supportedUrls (the assignment before this.doStream) which is immediately overwritten later; keep the later identical assignment (after this.doStream) so _supportedUrls is only set once, ensuring the property remains a function or async wrapper as intended and leaving this.doStream and other constructor logic untouched.src/validators.ts (1)
381-389:MessageContentPartsis missing the new approval types.
MessageContentParts(Lines 381–389) is the public union of all possible content part types, but it doesn't includeInfer<typeof vToolApprovalRequest>orInfer<typeof vToolApprovalResponse>. Consumers who exhaustively switch on this type will silently miss approval parts.♻️ Proposed fix
export type MessageContentParts = | Infer<typeof vTextPart> | Infer<typeof vImagePart> | Infer<typeof vFilePart> | Infer<typeof vReasoningPart> | Infer<typeof vRedactedReasoningPart> | Infer<typeof vToolCallPart> | Infer<typeof vToolResultPart> - | Infer<typeof vSourcePart>; + | Infer<typeof vSourcePart> + | Infer<typeof vToolApprovalRequest> + | Infer<typeof vToolApprovalResponse>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validators.ts` around lines 381 - 389, The exported union type MessageContentParts is missing the approval part types; update the union to include Infer<typeof vToolApprovalRequest> and Infer<typeof vToolApprovalResponse> so consumers who pattern-match on MessageContentParts will see approval parts. Locate the MessageContentParts type definition and add Infer<typeof vToolApprovalRequest> and Infer<typeof vToolApprovalResponse> to the union alongside vToolCallPart and vToolResultPart.src/mapping.ts (1)
182-209:compatibilitywarning round-trip is correct — butas anycasts hide the type mismatch.
serializeWarningsconvertsCallWarningtype: "compatibility"→ storedtype: "unsupported-setting", andtoModelMessageWarningsreverses that mapping. This is correct: AI SDK v6 internally emitstype: "compatibility"(withfeaturefield) when wrapping V2 providers viaasLanguageModelV3. The storedvLanguageModelCallWarningonly hasunsupported-setting, so the round-trip mapping is the right approach.The
as anyat lines 190 and 208 is necessary becausevLanguageModelCallWarningdoesn't include thecompatibilityvariant. Consider adding a comment explaining this intentional type coercion so future maintainers understand why it's needed.📝 Suggested comment
- }) as any; + // `as any`: the stored `vLanguageModelCallWarning` validator doesn't include + // the `compatibility` type that AI SDK v6 emits for V2 provider wrapping. + }) as MessageWithMetadata["warnings"];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mapping.ts` around lines 182 - 209, Add a short explanatory comment above the intentional type coercions in serializeWarnings and toModelMessageWarnings that documents the round-trip mapping between CallWarning type "compatibility" (with field feature) and the stored vLanguageModelCallWarning "unsupported-setting", and why the `as any` cast is used (vLanguageModelCallWarning's type union omits the "compatibility" variant when wrapping V2 providers via asLanguageModelV3); reference the functions serializeWarnings and toModelMessageWarnings and the involved variants ("compatibility" ↔ "unsupported-setting") so future maintainers understand this deliberate coercion.src/UIMessages.ts (1)
480-512: Doubleas unknown ascasts bypass type safety in tool-result handling.The
contentPart as unknown as ToolResultPart & { output: {...} }pattern at Line 480 (and similar patterns at Lines 568 and 592) bypasses TypeScript's structural checks entirely. If AI SDK v6 changes the shape ofToolResultPart.output, these casts will silently produce runtime errors.Consider narrowing with a runtime type-guard instead:
♻️ Example type-guard approach
- const typedPart = contentPart as unknown as ToolResultPart & { - output: { type: string; value?: unknown; reason?: string }; - }; + function hasOutputType(p: unknown): p is { output: { type: string; value?: unknown; reason?: string } } { + return typeof p === "object" && p !== null && "output" in p && + typeof (p as any).output === "object" && "type" in (p as any).output; + } + if (!hasOutputType(contentPart)) break; + const typedPart = contentPart as ToolResultPart & { output: { type: string; value?: unknown; reason?: string } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UIMessages.ts` around lines 480 - 512, The double "as unknown as" cast for typedPart bypasses TypeScript checks and should be replaced with a runtime type guard that verifies contentPart has the expected ToolResultPart shape (e.g., has output with a string type and optional value/reason) before treating it as such; implement a function like isToolResultPartWithOutput(part): part is ToolResultPart & { output: { type: string; value?: unknown; reason?: string } } and use it in the conditional that checks typedPart.output?.type === "execution-denied", updating the logic that finds the matching call in allParts and mutates call.state/approval (symbols: typedPart, ToolResultPart, output, allParts, ToolUIPart, contentPart.toolName, contentPart.toolCallId) and replace the other similar double-cast sites in this file with the same guard.src/client/createTool.ts (1)
14-21:MIGRATION_URL/warnDeprecationduplicated fromsrc/client/index.ts.Both modules define identical utilities in isolation, so the same deprecation key (e.g.
"textEmbeddingModel") can emit a warning from both modules independently. Extract to a sharedsrc/client/deprecation.tsand import from both sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/createTool.ts` around lines 14 - 21, MIGRATION_URL and warnDeprecation are duplicated in createTool.ts and index.ts leading to duplicate warnings; remove the duplicate definitions and extract them into a single shared module (e.g., create a new src/client/deprecation.ts exporting MIGRATION_URL and warnDeprecation), then import warnDeprecation (and MIGRATION_URL if needed) from that module in both createTool.ts and index.ts so the warnedDeprecations Set is shared and a given deprecation key (like "textEmbeddingModel") only logs once.src/client/search.ts (1)
360-362: Confusing shadowing: renametextEmbeddingModeltoeffectiveEmbeddingModel.The local variable is assigned
options.embeddingModel ?? options.textEmbeddingModelbut is namedtextEmbeddingModel, shadowing the concept it's resolving. Every other function in this file (embedMany,generateAndSaveEmbeddings,fetchContextWithPrompt) uses the nameeffectiveEmbeddingModelfor the same pattern — this one diverges.♻️ Proposed fix
- const textEmbeddingModel = - options.embeddingModel ?? options.textEmbeddingModel; - if (!textEmbeddingModel) { + const effectiveEmbeddingModel = + options.embeddingModel ?? options.textEmbeddingModel; + if (!effectiveEmbeddingModel) { return undefined; } ... - const model = getModelName(textEmbeddingModel); + const model = getModelName(effectiveEmbeddingModel);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/search.ts` around lines 360 - 362, Rename the local variable textEmbeddingModel to effectiveEmbeddingModel to match the naming used elsewhere and avoid concept shadowing; change the declaration that assigns options.embeddingModel ?? options.textEmbeddingModel to use effectiveEmbeddingModel and update all subsequent references in this function (including the null-check if (!textEmbeddingModel) and any uses later in this scope) so they reference effectiveEmbeddingModel instead.src/client/index.ts (2)
22-30: Import appears after module-level code — move imports to the top.
MIGRATION_URL,warnedDeprecations, andwarnDeprecationwere inserted at lines 22–29, pushing theconvex-helpersimport to line 30. While ESimportdeclarations are hoisted and this works at runtime, mixing code and imports breaks conventional file layout and will confuse tools and readers.♻️ Proposed fix
import { assert, omit, pick } from "convex-helpers"; // ... (all other imports) ... +const MIGRATION_URL = "https://github.com/get-convex/agent/blob/main/MIGRATION.md"; +const warnedDeprecations = new Set<string>(); +function warnDeprecation(key: string, message: string) { + if (!warnedDeprecations.has(key)) { + warnedDeprecations.add(key); + console.warn(`[`@convex-dev/agent`] ${message}\n See: ${MIGRATION_URL}`); + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/index.ts` around lines 22 - 30, Move the import block to the top of the module so no module-level code (MIGRATION_URL, warnedDeprecations, warnDeprecation) appears before imports; specifically, relocate the line "import { assert, omit, pick } from 'convex-helpers'" above the declarations of MIGRATION_URL, warnedDeprecations, and the warnDeprecation function to restore conventional file layout and avoid mixing executable code with import statements.
22-29:MIGRATION_URLandwarnDeprecationare duplicated betweenindex.tsandcreateTool.ts.Both files define identical
MIGRATION_URL,warnedDeprecations, andwarnDeprecation. Because these are separate module-level Sets, the same deprecation key in both modules will fire two separate console warnings. Extract to a shared internal utility (e.g.src/client/deprecation.ts) and import from both.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/index.ts` around lines 22 - 29, Duplicate module-level deprecation logic (MIGRATION_URL, warnedDeprecations, warnDeprecation) causes duplicate console warnings; extract these three symbols into a single shared internal module (e.g., deprecation.ts) and have both modules import warnDeprecation and MIGRATION_URL from it, replacing their local definitions; ensure the shared warnedDeprecations Set is used by the exported warnDeprecation function so a deprecation key only logs once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/agent-usage.mdx`:
- Line 252: The example assigns an undefined symbol `parameters` to
`inputSchema`, which is misleading; replace the undefined reference with an
inline schema object to match other examples by updating the `myTool` definition
(the `tool` call) to provide a concrete `inputSchema` (e.g., an object
describing fields and types) instead of `parameters`, so readers can see the
expected schema shape directly in the `myTool: tool({ description, inputSchema:
... , execute })` example.
In `@src/client/createTool.ts`:
- Around line 241-263: The function currently throws when inputSchema is missing
before emitting deprecation warnings, so move the warnDeprecation checks to
occur before the inputSchema existence check: check def.args and def.handler and
call warnDeprecation for "createTool.args" and "createTool.handler" (referencing
def.args, def.inputSchema, def.handler) prior to throwing for missing
inputSchema, then compute inputSchema and executeHandler (inputSchema =
def.inputSchema ?? def.args; executeHandler = def.execute ?? def.handler) and
finally throw the existing error if inputSchema is falsy; this ensures
deprecated properties are warned about even when inputSchema is absent.
In `@src/client/search.test.ts`:
- Around line 295-338: The function filterOutOrphanedToolMessages is removing
tool-call entries that lack approval responses but leaves their matching
tool-approval-request items orphaned; update the logic so that after you
identify removed tool-call IDs (use the existing approvalRequestsByToolCallId
mapping or compute a removedToolCallIds set), change the content filtering step
that currently keeps non-tool-call items unconditionally to also skip any
tool-approval-request whose toolCallId is in removedToolCallIds; ensure
comparisons use the toolCallId field and apply this when building each
MessageDoc.message.content so both the tool-call and its associated
tool-approval-request are removed together.
In `@src/client/search.ts`:
- Around line 261-286: The assistant messages can end up containing orphaned
"tool-approval-request" parts when their sibling "tool-call" was filtered out;
update the second-pass filtering logic to also drop "tool-approval-request"
parts whose toolCallId has no surviving tool-call and no approval response. Use
the existing approvalRequestsByToolCallId map and
hasApprovalResponse(toolCallId) to detect orphans: when iterating assistant
parts, if part.type === "tool-approval-request" look up its toolCallId in
approvalRequestsByToolCallId and remove the part if the corresponding tool-call
was filtered (i.e., there is no surviving tool-call entry for that toolCallId
and hasApprovalResponse(toolCallId) is false). Ensure this uses the same
identifiers (approvalRequestsByToolCallId, hasApprovalResponse) so the
approval-request is kept only when its tool-call or an approval response exists.
In `@src/deltas.ts`:
- Around line 493-510: The case handling "tool-approval-request" currently uses
a double-cast on part (part as unknown as {...}) which masks the real SDK shape;
replace the unsafe assertion by reading the SDK shape directly or adding a
runtime type guard: check whether part.toolCall?.toolCallId exists and if so use
that value (otherwise fall back to a normalized part.toolCallId), then look up
toolPartsById with the resolved id and set toolPart.state and toolPart.approval
as before; remove the double assertion and/or add a small type-guard function
(e.g., isToolApprovalRequestPart) so TypeScript can’t be bypassed and runtime
shape mismatches are handled explicitly in the "tool-approval-request" case.
In `@src/UIMessages.ts`:
- Around line 97-101: convertToModelMessages() expands a single UI message into
multiple MessageDoc records and appends a suffix to the stored _id (constructed
from uiMessage.id + `-${i}`) to avoid collisions for children; add a concise
note to the CHANGELOG and migration guide describing this ID transformation,
show the recommended consumer update pattern (e.g., query by exact _id or
implement a fallback lookup by uiMessage.id as in
playground/src/components/MessageList.tsx), and mention that consumers who
previously queried by uiMessage.id must adapt their lookups to account for
suffixed document IDs.
---
Outside diff comments:
In `@docs/agent-usage.mdx`:
- Around line 196-214: The docs and example use the deprecated
textEmbeddingModel property; update the prose and the sharedDefaults example to
use embeddingModel instead (replace mentions of textEmbeddingModel with
embeddingModel and set sharedDefaults.embeddingModel =
openai.embedding("text-embedding-3-small")); ensure any explanatory text
referencing "textEmbeddingModel" is changed to "embeddingModel" so the Agent
config (e.g., languageModel and embeddingModel in sharedDefaults) matches the
current API.
In `@src/client/createTool.ts`:
- Around line 309-320: The lifecycle hooks (toModelOutput, onInputStart,
onInputDelta, onInputAvailable) are currently bound with getCtx(t) at creation
time, capturing a stale ctx; change each binding to a runtime wrapper function
that computes ctx via getCtx(this) when invoked (e.g., replace t.toModelOutput =
def.toModelOutput.bind(t, getCtx(t)) with a regular function that calls
def.toModelOutput.call(this, getCtx(this), ...args)), mirroring how execute and
needsApproval obtain ctx, so when wrapTools spreads and injects ctx the handler
sees the injected runtime ctx; update the same pattern for onInputStart,
onInputDelta and onInputAvailable and keep use of .call/.apply to preserve
original this (t) and argument forwarding.
In `@src/client/search.ts`:
- Around line 449-473: The current call to usageHandler in search.ts passes an
object shaped as LanguageModelUsage but fills
inputTokenDetails/outputTokenDetails with undefined to represent embedding-only
usage; instead, update the UsageHandler type (or the type used in the handler
signature) to accept a union or shared base between LanguageModelUsage and
EmbeddingModelUsage (e.g., UsageBase | LanguageModelUsage | EmbeddingModelUsage)
so embedding-only payloads can be passed without padded undefined fields, then
change the call site that constructs the usage to pass an EmbeddingModelUsage
instance (only tokens) or the appropriate union variant; also add a short
comment/doc on EmbeddingModelUsage semantics so consumers know those detail
fields won’t be present for embeddings.
In `@src/toUIMessages.test.ts`:
- Line 442: Rename the duplicate test description string used in the second
it(...) block that currently reads "sets text field correctly when message has
many parts with text part as final part" to a unique, descriptive name (e.g.
"sets text field correctly in multi-step tool-call + final-text flow") so test
reports are unambiguous; locate the second it(...) declaration in the
toUIMessages.test file (the test exercising dateTime tool call) and replace its
first argument (the description string) with the new unique string.
In `@src/UIMessages.ts`:
- Around line 58-68: The exported function fromUIMessages was changed to an
async function, introducing a breaking API change; revert it to a synchronous
export to maintain compatibility by removing the async modifier and returning
the array directly (so callers that await still work). Locate the fromUIMessages
function and its return expression, remove the async keyword and ensure it
returns (MessageDoc & { streaming: boolean; metadata?: METADATA })[] instead of
Promise<...>; keep the function name and signature generic parameters the same
so the public export used in client/index.ts is unchanged. If asynchronous work
is truly required internally, implement that as an internal helper (e.g.,
fromUIMessagesAsync) and call it from fromUIMessages in a synchronous manner or
document a new async helper instead of changing the public API.
---
Duplicate comments:
In `@docs/agent-usage.mdx`:
- Line 46: The docs have inconsistent imports for stepCountIs (one example
imports it from "@convex-dev/agent" while another imports from "ai");
standardize the examples by importing stepCountIs from the same package as Agent
(use "@convex-dev/agent") and update any imports elsewhere in the document so
every code sample imports Agent and stepCountIs from the same module.
In `@example/convex/modelsForDemo.ts`:
- Around line 8-11: The declared variable textEmbeddingModel is typed as
EmbeddingModel but is only assigned when OPENAI_API_KEY is present, so change
its type to EmbeddingModel | undefined to reflect possible absence; update the
declaration of textEmbeddingModel to use the union type and audit any
uses/exports of textEmbeddingModel (and any callers of LanguageModelV3 if
related) to handle the undefined case or add runtime guards where the variable
is consumed.
In `@src/mapping.ts`:
- Around line 161-172: The previous review flagged using "{} as any" for
LanguageModelUsage token details; update the object shape to explicitly provide
fields with undefined values instead of casting, so set inputTokenDetails
(cacheReadTokens, cacheWriteTokens, noCacheTokens) and outputTokenDetails
(textTokens, reasoningTokens) to objects with those keys assigned undefined to
satisfy the LanguageModelUsage type; ensure these explicit objects replace any
"{} as any" occurrences (look for inputTokenDetails, outputTokenDetails and the
LanguageModelUsage construction) so the types are correct and no casting is
used.
---
Nitpick comments:
In `@src/client/createTool.ts`:
- Around line 14-21: MIGRATION_URL and warnDeprecation are duplicated in
createTool.ts and index.ts leading to duplicate warnings; remove the duplicate
definitions and extract them into a single shared module (e.g., create a new
src/client/deprecation.ts exporting MIGRATION_URL and warnDeprecation), then
import warnDeprecation (and MIGRATION_URL if needed) from that module in both
createTool.ts and index.ts so the warnedDeprecations Set is shared and a given
deprecation key (like "textEmbeddingModel") only logs once.
In `@src/client/index.ts`:
- Around line 22-30: Move the import block to the top of the module so no
module-level code (MIGRATION_URL, warnedDeprecations, warnDeprecation) appears
before imports; specifically, relocate the line "import { assert, omit, pick }
from 'convex-helpers'" above the declarations of MIGRATION_URL,
warnedDeprecations, and the warnDeprecation function to restore conventional
file layout and avoid mixing executable code with import statements.
- Around line 22-29: Duplicate module-level deprecation logic (MIGRATION_URL,
warnedDeprecations, warnDeprecation) causes duplicate console warnings; extract
these three symbols into a single shared internal module (e.g., deprecation.ts)
and have both modules import warnDeprecation and MIGRATION_URL from it,
replacing their local definitions; ensure the shared warnedDeprecations Set is
used by the exported warnDeprecation function so a deprecation key only logs
once.
In `@src/client/mockModel.ts`:
- Around line 205-208: Remove the redundant early initialization of
this._supportedUrls (the assignment before this.doStream) which is immediately
overwritten later; keep the later identical assignment (after this.doStream) so
_supportedUrls is only set once, ensuring the property remains a function or
async wrapper as intended and leaving this.doStream and other constructor logic
untouched.
In `@src/client/search.ts`:
- Around line 360-362: Rename the local variable textEmbeddingModel to
effectiveEmbeddingModel to match the naming used elsewhere and avoid concept
shadowing; change the declaration that assigns options.embeddingModel ??
options.textEmbeddingModel to use effectiveEmbeddingModel and update all
subsequent references in this function (including the null-check if
(!textEmbeddingModel) and any uses later in this scope) so they reference
effectiveEmbeddingModel instead.
In `@src/client/types.ts`:
- Around line 24-31: Move the exported Output interface declaration so it
appears after the last import block (i.e., below the "convex/server" import)
instead of between import groups; locate the `export interface Output<_T = any,
_P = any, _E = any>` declaration and cut/paste it below the final import,
keeping its name and export unchanged and verifying any referenced types still
resolve.
In `@src/deltas.ts`:
- Around line 69-103: The current suppression in the onError callback of
readUIMessageStream relies on matching the literal substring "no tool invocation
found" which is fragile; update onError to detect the condition by checking a
stable identifier (e.g., an error.code property or specific Error subclass)
rather than string matching, using properties of the thrown error (e.g., e.code,
e.name, or instanceof SomeSdkError) and fall back to the string check only if
those fields are absent; reference the symbols readUIMessageStream, onError,
suppressError, and the "no tool invocation found" check when making the change,
and if the SDK doesn't expose a stable code/type add a clear comment documenting
the SDK and version/source of that message so future upgrades can revisit this
logic.
In `@src/mapping.ts`:
- Around line 182-209: Add a short explanatory comment above the intentional
type coercions in serializeWarnings and toModelMessageWarnings that documents
the round-trip mapping between CallWarning type "compatibility" (with field
feature) and the stored vLanguageModelCallWarning "unsupported-setting", and why
the `as any` cast is used (vLanguageModelCallWarning's type union omits the
"compatibility" variant when wrapping V2 providers via asLanguageModelV3);
reference the functions serializeWarnings and toModelMessageWarnings and the
involved variants ("compatibility" ↔ "unsupported-setting") so future
maintainers understand this deliberate coercion.
In `@src/UIMessages.ts`:
- Around line 480-512: The double "as unknown as" cast for typedPart bypasses
TypeScript checks and should be replaced with a runtime type guard that verifies
contentPart has the expected ToolResultPart shape (e.g., has output with a
string type and optional value/reason) before treating it as such; implement a
function like isToolResultPartWithOutput(part): part is ToolResultPart & {
output: { type: string; value?: unknown; reason?: string } } and use it in the
conditional that checks typedPart.output?.type === "execution-denied", updating
the logic that finds the matching call in allParts and mutates
call.state/approval (symbols: typedPart, ToolResultPart, output, allParts,
ToolUIPart, contentPart.toolName, contentPart.toolCallId) and replace the other
similar double-cast sites in this file with the same guard.
In `@src/validators.ts`:
- Around line 381-389: The exported union type MessageContentParts is missing
the approval part types; update the union to include Infer<typeof
vToolApprovalRequest> and Infer<typeof vToolApprovalResponse> so consumers who
pattern-match on MessageContentParts will see approval parts. Locate the
MessageContentParts type definition and add Infer<typeof vToolApprovalRequest>
and Infer<typeof vToolApprovalResponse> to the union alongside vToolCallPart and
vToolResultPart.
| const inputSchema = def.inputSchema ?? def.args; | ||
| if (!inputSchema) | ||
| throw new Error("To use a Convex tool, you must provide an `inputSchema` (or `args`)"); | ||
|
|
||
| if (def.args && !def.inputSchema) { | ||
| warnDeprecation( | ||
| "createTool.args", | ||
| "createTool: 'args' is deprecated. Use 'inputSchema' instead.", | ||
| ); | ||
| } | ||
| if (def.handler && !def.execute) { | ||
| warnDeprecation( | ||
| "createTool.handler", | ||
| "createTool: 'handler' is deprecated. Use 'execute' instead.", | ||
| ); | ||
| } | ||
|
|
||
| // Extra AI SDK pass-through options. | ||
| providerOptions?: ProviderOptions; | ||
| }): Tool<INPUT, OUTPUT> { | ||
| const t = tool({ | ||
| const executeHandler = def.execute ?? def.handler; | ||
| if (!executeHandler && !def.outputSchema) | ||
| throw new Error( | ||
| "To use a Convex tool, you must either provide an execute" + | ||
| " handler function, define an outputSchema, or both", | ||
| ); |
There was a problem hiding this comment.
Error thrown before deprecation warning fires when both inputSchema is missing and args/handler is present.
When !inputSchema is true (line 242), the function throws immediately — the deprecation warnings for args (line 245) and handler (line 251) are never emitted. A user who passes only the deprecated args property but makes a typo in the key will get a confusing error with no migration hint. Reversing the order (warn then throw) provides a better developer experience.
♻️ Proposed fix
const inputSchema = def.inputSchema ?? def.args;
- if (!inputSchema)
- throw new Error("To use a Convex tool, you must provide an `inputSchema` (or `args`)");
-
if (def.args && !def.inputSchema) {
warnDeprecation("createTool.args", "createTool: 'args' is deprecated. Use 'inputSchema' instead.");
}
if (def.handler && !def.execute) {
warnDeprecation("createTool.handler", "createTool: 'handler' is deprecated. Use 'execute' instead.");
}
+ if (!inputSchema)
+ throw new Error("To use a Convex tool, you must provide an `inputSchema` (or `args`)");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/createTool.ts` around lines 241 - 263, The function currently
throws when inputSchema is missing before emitting deprecation warnings, so move
the warnDeprecation checks to occur before the inputSchema existence check:
check def.args and def.handler and call warnDeprecation for "createTool.args"
and "createTool.handler" (referencing def.args, def.inputSchema, def.handler)
prior to throwing for missing inputSchema, then compute inputSchema and
executeHandler (inputSchema = def.inputSchema ?? def.args; executeHandler =
def.execute ?? def.handler) and finally throw the existing error if inputSchema
is falsy; this ensures deprecated properties are warned about even when
inputSchema is absent.
| it("should filter out tool calls with approval request but NO approval response", () => { | ||
| const messages: MessageDoc[] = [ | ||
| { | ||
| _id: "1", | ||
| message: { | ||
| role: "assistant", | ||
| content: [ | ||
| { type: "text", text: "I'll run the dangerous tool" }, | ||
| { | ||
| type: "tool-call", | ||
| toolCallId: "call_123", | ||
| toolName: "dangerousTool", | ||
| input: { action: "delete" }, | ||
| args: { action: "delete" }, | ||
| }, | ||
| { | ||
| type: "tool-approval-request", | ||
| toolCallId: "call_123", | ||
| approvalId: "approval_456", | ||
| }, | ||
| ], | ||
| }, | ||
| order: 1, | ||
| } as MessageDoc, | ||
| // No approval response provided | ||
| ]; | ||
|
|
||
| const result = filterOutOrphanedToolMessages(messages); | ||
| expect(result).toHaveLength(1); | ||
| // The assistant message should have the tool-call filtered out | ||
| const assistantContent = result[0].message?.content; | ||
| expect(Array.isArray(assistantContent)).toBe(true); | ||
| if (Array.isArray(assistantContent)) { | ||
| // Text and approval-request should remain, but tool-call should be filtered | ||
| expect(assistantContent).toHaveLength(2); | ||
| expect(assistantContent.find((p) => p.type === "text")).toBeDefined(); | ||
| expect( | ||
| assistantContent.find((p) => p.type === "tool-approval-request"), | ||
| ).toBeDefined(); | ||
| expect( | ||
| assistantContent.find((p) => p.type === "tool-call"), | ||
| ).toBeUndefined(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
ast-grep --pattern $'function filterOutOrphanedToolMessages($_) {
$$$
}'Repository: get-convex/agent
Length of output: 5001
🏁 Script executed:
# Search for other tests of filterOutOrphanedToolMessages
rg "filterOutOrphanedToolMessages" src/client/search.test.ts -A 5 -B 2Repository: get-convex/agent
Length of output: 2221
🏁 Script executed:
# Check for any comments in the implementation explaining the approval-request handling
sed -n '245,331p' src/client/search.ts | cat -nRepository: get-convex/agent
Length of output: 3435
Fix: Tool-approval-requests should be removed when their corresponding tool-calls are filtered.
The current implementation filters out tool-calls without an approval response (lines 49-54) but leaves their corresponding tool-approval-requests orphaned in the content. Lines 328-333 of the test assert this behavior, but there is no logic to remove these dangling approval-requests.
The filter condition keeps all non-tool-call items unconditionally, including approval-requests. To maintain data integrity, when a tool-call is removed due to lacking an approval response, its tool-approval-request should also be removed. The implementation already tracks approvalRequestsByToolCallId (lines 23-26) and could use this mapping to filter out orphaned approval-requests alongside their tool-calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/search.test.ts` around lines 295 - 338, The function
filterOutOrphanedToolMessages is removing tool-call entries that lack approval
responses but leaves their matching tool-approval-request items orphaned; update
the logic so that after you identify removed tool-call IDs (use the existing
approvalRequestsByToolCallId mapping or compute a removedToolCallIds set),
change the content filtering step that currently keeps non-tool-call items
unconditionally to also skip any tool-approval-request whose toolCallId is in
removedToolCallIds; ensure comparisons use the toolCallId field and apply this
when building each MessageDoc.message.content so both the tool-call and its
associated tool-approval-request are removed together.
| } else if (content.type === "tool-approval-request") { | ||
| const approvalRequest = content as { | ||
| type: "tool-approval-request"; | ||
| toolCallId: string; | ||
| approvalId: string; | ||
| }; | ||
| approvalRequestsByToolCallId.set( | ||
| approvalRequest.toolCallId, | ||
| approvalRequest.approvalId, | ||
| ); | ||
| } else if (content.type === "tool-approval-response") { | ||
| const approvalResponse = content as { | ||
| type: "tool-approval-response"; | ||
| approvalId: string; | ||
| }; | ||
| approvalResponseIds.add(approvalResponse.approvalId); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Helper: check if tool call has a corresponding approval response | ||
| const hasApprovalResponse = (toolCallId: string) => { | ||
| const approvalId = approvalRequestsByToolCallId.get(toolCallId); | ||
| return approvalId !== undefined && approvalResponseIds.has(approvalId); | ||
| }; |
There was a problem hiding this comment.
Orphaned tool-approval-request items can survive when their companion tool-call is filtered out.
In the second pass (lines 293–298), only tool-call parts are subject to removal from assistant messages; all other part types pass through unconditionally. If a tool-approval-request part exists alongside a tool-call that gets filtered (no tool result, no approval response), the assistant message will contain an approval request without a corresponding tool call, which may confuse the model on the next turn.
Consider either filtering tool-approval-request alongside its sibling tool-call (matching on toolCallId), or explicitly documenting that this intermediate state is intentional and harmless.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/search.ts` around lines 261 - 286, The assistant messages can end
up containing orphaned "tool-approval-request" parts when their sibling
"tool-call" was filtered out; update the second-pass filtering logic to also
drop "tool-approval-request" parts whose toolCallId has no surviving tool-call
and no approval response. Use the existing approvalRequestsByToolCallId map and
hasApprovalResponse(toolCallId) to detect orphans: when iterating assistant
parts, if part.type === "tool-approval-request" look up its toolCallId in
approvalRequestsByToolCallId and remove the part if the corresponding tool-call
was filtered (i.e., there is no surviving tool-call entry for that toolCallId
and hasApprovalResponse(toolCallId) is false). Ensure this uses the same
identifiers (approvalRequestsByToolCallId, hasApprovalResponse) so the
approval-request is kept only when its tool-call or an approval response exists.
| case "tool-approval-request": { | ||
| const typedPart = part as unknown as { | ||
| type: "tool-approval-request"; | ||
| toolCallId: string; | ||
| approvalId: string; | ||
| }; | ||
| const toolPart = toolPartsById.get(typedPart.toolCallId); | ||
| if (toolPart) { | ||
| toolPart.state = "approval-requested"; | ||
| (toolPart as ToolUIPart & { approval?: object }).approval = { | ||
| id: typedPart.approvalId, | ||
| }; | ||
| } else { | ||
| console.warn( | ||
| `Expected tool call part ${typedPart.toolCallId} for approval request`, | ||
| ); | ||
| } | ||
| break; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for tool-approval-request in the codebase
rg -n "tool-approval-request" --type ts -g '!*.test.*' -g '!node_modules'Repository: get-convex/agent
Length of output: 1438
🏁 Script executed:
# Check what TextStreamPart type looks like and where it's imported from
rg -n "TextStreamPart" --type ts -g '!*.test.*' -g '!node_modules' | head -20Repository: get-convex/agent
Length of output: 1568
🏁 Script executed:
# Look at the imports in deltas.ts to understand the type source
head -50 src/deltas.ts | grep -E "^import|^from"Repository: get-convex/agent
Length of output: 280
🌐 Web query:
AI SDK v6 TextStreamPart type definition tool-approval-request
💡 Result:
In AI SDK v6, tool-approval-request is represented in the TextStreamPart union via ToolApprovalRequestOutput.
ToolApprovalRequestOutput (the tool-approval-request part)
export type ToolApprovalRequestOutput<TOOLS extends ToolSet> = {
type: 'tool-approval-request';
approvalId: string;
toolCall: TypedToolCall<TOOLS>;
};[1]
Where it appears in TextStreamPart
TextStreamPart<TOOLS> is a union of many streamed event shapes and includes:
| ToolApprovalRequestOutput<TOOLS>as one of its variants. [2]
Sources: [1] [2]
🏁 Script executed:
# Check the AI SDK version in package.json
rg -n "@ai-sdk" package.jsonRepository: get-convex/agent
Length of output: 290
🏁 Script executed:
# Check if tool-approval-request is mentioned in deltas.ts context
sed -n '490,530p' src/deltas.tsRepository: get-convex/agent
Length of output: 1315
🏁 Script executed:
# Look at the imports and type definitions more carefully
rg -B5 -A10 "type TextStreamPart" src/deltas.ts | head -30Repository: get-convex/agent
Length of output: 455
🏁 Script executed:
# Check src/validators.ts to see how tool-approval-request is defined
sed -n '260,275p' src/validators.tsRepository: get-convex/agent
Length of output: 428
🏁 Script executed:
# Find the main "ai" package version
rg -n '"ai"' package.json | grep -v "@ai-sdk"Repository: get-convex/agent
Length of output: 121
🏁 Script executed:
# Check if there's a tsconfig or type declaration that shows what ai version is being used
cat package.json | grep -A20 '"dependencies"'Repository: get-convex/agent
Length of output: 42
🌐 Web query:
AI SDK TextStreamPart tool-approval-request type version history
💡 Result:
What changed, when (AI SDK TextStreamPart → type: "tool-approval-request")
- AI SDK v5.x:
TextStreamPartdid not include any"tool-approval-request"variant (string not present inai@5.0.29type definitions). [4] - AI SDK v6.0.0: Tool execution approval was introduced as part of the AI SDK 6 beta (i.e., the overall “tool approval system” feature set that enables human-in-the-loop tool execution). [1] [2]
- AI SDK v6.0.3: Added MCP tool approval support (changelog entry: “feat: add MCP tool approval”). [2]
- AI SDK v6.x (example:
ai@6.0.41):TextStreamPartincludesToolApprovalRequestOutput:type: "tool-approval-request"approvalId: stringtoolCall: TypedToolCall<TOOLS>[3]
Related docs/behavior
- For OpenAI’s MCP integration, when approval is required, the model can return a
tool-approval-requestcontent part. [5] - If an approval response references an unknown
approvalId, the SDK throwsAI_InvalidToolApprovalError(no matchingtool-approval-requestin history). [6]
🏁 Script executed:
# Check where tool-approval-request parts are generated/transformed
rg -B5 -A10 "toolCallId.*approvalId\|approvalId.*toolCallId" --type ts -g '!*.test.*' -g '!node_modules' | head -50Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
# Check the readUIMessageStream function to understand the pipeline
rg -B3 -A15 "readUIMessageStream" src/deltas.ts | head -40Repository: get-convex/agent
Length of output: 1244
🏁 Script executed:
# Look for any transformation of toolCall to toolCallId
rg -n "toolCall" src/deltas.tsRepository: get-convex/agent
Length of output: 1256
🏁 Script executed:
# Check if the part has a toolCall property that's being used
rg -B10 "tool-approval-request" src/deltas.ts | grep -A20 "case.*tool-approval"Repository: get-convex/agent
Length of output: 185
🏁 Script executed:
# Look at how other similar parts extract IDs from objects
rg -B2 -A5 "part\.id\|toolCall" src/deltas.ts | head -60Repository: get-convex/agent
Length of output: 42
🌐 Web query:
AI SDK v6 ToolApprovalRequestOutput toolCall structure toolCallId
💡 Result:
In AI SDK v6, a ToolApprovalRequestOutput is an output part shaped like this (TypeScript):
type ToolApprovalRequestOutput<TOOLS extends ToolSet> = {
type: 'tool-approval-request';
approvalId: string;
toolCall: TypedToolCall<TOOLS>;
};toolCall is the tool call being gated for approval, and its ID is toolCall.toolCallId (i.e., toolCallId is nested under toolCall, not on the approval request object itself). The base tool call shape is:
type BaseToolCall = {
type: 'tool-call';
toolCallId: string;
providerExecuted?: boolean;
providerMetadata?: ProviderMetadata;
};So, in JSON-like form you’ll typically see:
{
"type": "tool-approval-request",
"approvalId": "…",
"toolCall": {
"type": "tool-call",
"toolCallId": "…",
"toolName": "…",
"input": { }
}
}[1] [2]
Shape mismatch in tool-approval-request handling — double assertion masks actual SDK type.
The part as unknown as { ... } cast at line 494 works around a shape incompatibility: AI SDK v6 provides tool-approval-request parts with toolCall: TypedToolCall<TOOLS> (where toolCallId is nested as toolCall.toolCallId), but this code asserts a flattened shape with toolCallId as a direct property.
This suggests the codebase normalizes SDK parts to an internal representation. While the type is correctly included in TextStreamPart in AI SDK v6, the double assertion means the actual SDK shape is hidden from TypeScript's type checker. If the SDK's nested structure changes, no compile-time error will surface—only runtime behavior.
Either use the SDK shape directly (part.toolCall.toolCallId) or document why the normalized shape is necessary and maintain a type guard to validate it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/deltas.ts` around lines 493 - 510, The case handling
"tool-approval-request" currently uses a double-cast on part (part as unknown as
{...}) which masks the real SDK shape; replace the unsafe assertion by reading
the SDK shape directly or adding a runtime type guard: check whether
part.toolCall?.toolCallId exists and if so use that value (otherwise fall back
to a normalized part.toolCallId), then look up toolPartsById with the resolved
id and set toolPart.state and toolPart.approval as before; remove the double
assertion and/or add a small type-guard function (e.g.,
isToolApprovalRequestPart) so TypeScript can’t be bypassed and runtime shape
mismatches are handled explicitly in the "tool-approval-request" case.
| const doc: MessageDoc & { streaming: boolean; metadata?: METADATA } = | ||
| { | ||
| ...commonFields, | ||
| _id: uiMessage.id + `-${i}`, | ||
| stepOrder: stepOrder + i, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check callers of fromUIMessages
rg -n --type ts "fromUIMessages" -A 10Repository: get-convex/agent
Length of output: 14661
🏁 Script executed:
#!/bin/bash
# Get the full context of the line in question
cat -n src/UIMessages.ts | sed -n '90,110p'Repository: get-convex/agent
Length of output: 1090
🏁 Script executed:
#!/bin/bash
# Check if there's a git history showing what changed
git log --oneline -20 src/UIMessages.ts 2>/dev/null | head -20Repository: get-convex/agent
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Search for any lookups or queries using _id from UIMessages
rg -n "uiMessage\.id|_id.*uiMessage" --type tsRepository: get-convex/agent
Length of output: 535
🏁 Script executed:
cat -n src/UIMessages.ts | sed -n '70,115p'Repository: get-convex/agent
Length of output: 2303
🏁 Script executed:
# Check the git diff to see what changed
git diff HEAD~1 src/UIMessages.ts 2>/dev/null | head -100Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
# Check if playground is a test or example directory
fd -t f "MessageList.tsx" -x cat {}Repository: get-convex/agent
Length of output: 2005
🏁 Script executed:
# Search for any migration guides or documentation about this change
rg -n "migration|breaking" --type md --type txt --type js --type ts -iRepository: get-convex/agent
Length of output: 1654
🏁 Script executed:
# Check if there are other places handling ID mismatches similar to MessageList.tsx
rg -n "message\._id|_id.*===" --type ts -B 2 -A 2 | head -80Repository: get-convex/agent
Length of output: 4316
🏁 Script executed:
# Look for any comments or documentation in UIMessages.ts about the suffix
rg -n "suffix|breaking|migration|v1|v2" src/UIMessages.ts -i -B 2 -A 2Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
# Check CHANGELOG.md for any mention of the ID suffix changes
cat CHANGELOG.md | head -100Repository: get-convex/agent
Length of output: 4567
🏁 Script executed:
# Look at the full migration guide
cat MIGRATION.mdRepository: get-convex/agent
Length of output: 2223
🏁 Script executed:
# Check git history of the specific lines to see if suffix was recent
git log -p --follow -S 'uiMessage.id + `-${i}`' src/UIMessages.ts 2>/dev/null | head -150Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
# Check the CHANGELOG for any mention of the specific _id suffix behavior
rg -n "suffix|_id.*\-\$|tool.*message" CHANGELOG.md -B 2 -A 2Repository: get-convex/agent
Length of output: 1127
🏁 Script executed:
# Look at lines around the famous "Breaking: `.id` on `toUIMessages`" line to understand context
cat CHANGELOG.md | sed -n '230,250p'Repository: get-convex/agent
Length of output: 1171
🏁 Script executed:
# Search for any tests that verify the _id suffix behavior
rg -n "_id.*\-0|suffix" src/fromUIMessages.test.ts -B 3 -A 3Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
# Let's check what convertToModelMessages returns to understand when multiple messages are generated
rg -n "convertToModelMessages" src/ -A 5 -B 2 | head -100Repository: get-convex/agent
Length of output: 1797
🏁 Script executed:
# Check if there's any inline documentation about the _id suffix in the code
cat -n src/UIMessages.ts | sed -n '85,110p'Repository: get-convex/agent
Length of output: 1343
🏁 Script executed:
# Search for any documentation or comments about ID collision prevention
rg -n "separate.*document|split.*message|tool.*message" src/UIMessages.ts -i -B 2 -A 2Repository: get-convex/agent
Length of output: 814
🏁 Script executed:
# Let's check if there's a CHANGELOG entry about this specific _id change
rg -n "0.6.0" CHANGELOG.md -A 50 | grep -i "id\|break\|tool\|message"Repository: get-convex/agent
Length of output: 1539
🏁 Script executed:
# Check what convertToModelMessages is (from AI SDK) and whether it returns multiple messages
rg -n "export.*convertToModelMessages|import.*convertToModelMessages" -B 2 -A 2Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
# Verify: does a single UI message ever produce multiple model messages?
cat -n src/UIMessages.ts | sed -n '58,130p'Repository: get-convex/agent
Length of output: 3407
🏁 Script executed:
# Final verification: check test file to see if there are any assertions about _id format
cat src/fromUIMessages.test.ts | grep -A 5 -B 5 "_id" | head -50Repository: get-convex/agent
Length of output: 966
🏁 Script executed:
# Check if there are any comments explaining the _id suffixing design
cat -n src/UIMessages.ts | sed -n '85,108p'Repository: get-convex/agent
Length of output: 1174
🏁 Script executed:
# Look for any documentation about why the suffix is used
rg -n "separate|distinct|multiple|tool|step" src/UIMessages.ts -B 1 -A 1Repository: get-convex/agent
Length of output: 6357
_id suffix separates expanded tool and reasoning messages.
The code at line 100 sets _id: uiMessage.id + '-${i}' because convertToModelMessages() expands a single UI message into multiple document records (one for each tool call, reasoning step, etc.). This intentional suffix prevents ID collisions between the main message and its expanded children.
However, this design is not documented in the CHANGELOG or migration guide. Consumers that query by uiMessage.id directly may need to adapt (as shown in playground/src/components/MessageList.tsx, which implements a lookup fallback pattern). Consider adding a note to the migration guide explaining this ID transformation when messages are split into multiple documents.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/UIMessages.ts` around lines 97 - 101, convertToModelMessages() expands a
single UI message into multiple MessageDoc records and appends a suffix to the
stored _id (constructed from uiMessage.id + `-${i}`) to avoid collisions for
children; add a concise note to the CHANGELOG and migration guide describing
this ID transformation, show the recommended consumer update pattern (e.g.,
query by exact _id or implement a fallback lookup by uiMessage.id as in
playground/src/components/MessageList.tsx), and mention that consumers who
previously queried by uiMessage.id must adapt their lookups to account for
suffixed document IDs.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/validators.ts (1)
147-156:⚠️ Potential issue | 🟡 MinorAdd
mediaTypefield tovToolResultContentimage part for consistency.The image object within
vToolResultContent(line 150–155) currently has onlymimeType: v.optional(v.string()), butvImagePartandvFilePartboth includemediaType: v.optional(v.string())as the primary field withmimeTypemarked as deprecated. UpdatevToolResultContentto match this pattern.Suggested fix
v.object({ type: v.literal("image"), data: v.string(), + mediaType: v.optional(v.string()), + /** `@deprecated` Use `mediaType` instead. */ mimeType: v.optional(v.string()), }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validators.ts` around lines 147 - 156, vToolResultContent's image branch is missing the mediaType field; update the image object inside vToolResultContent to include mediaType: v.optional(v.string()) (and keep mimeType: v.optional(v.string()) for backward compatibility) so it matches vImagePart and vFilePart. Modify the union branch in vToolResultContent (the v.object that currently has data and mimeType) to add mediaType as an optional string field.src/client/mockModel.ts (1)
187-191:⚠️ Potential issue | 🟡 MinorFix off-by-one bug in array index access for
doGenerateanddoStream.When
doGenerateordoStreamis an array, the current code accesses the wrong index. Afterthis.doGenerateCalls.push(options)on line 182 increments the length, the code accessesargs.doGenerate[this.doGenerateCalls.length]on line 187, which skips the first element. On the first call,this.doGenerateCalls.lengthis 1, so it accesses index [1] instead of [0]. The same issue exists fordoStreamon line 209.Proposed fix
} else if (Array.isArray(args.doGenerate)) { - return args.doGenerate[this.doGenerateCalls.length]; + return args.doGenerate[this.doGenerateCalls.length - 1]; } else if (contentSteps.length) {And similarly for
doStream:} else if (Array.isArray(args.doStream)) { - return args.doStream[this.doStreamCalls.length]; + return args.doStream[this.doStreamCalls.length - 1]; } else if (contentSteps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/mockModel.ts` around lines 187 - 191, The array access for mocked responses is off-by-one: after pushing the call into this.doGenerateCalls (and this.doStreamCalls), the code uses args.doGenerate[this.doGenerateCalls.length] (and args.doStream[this.doStreamCalls.length]) which skips the first element; change those accesses to use the previous index (this.doGenerateCalls.length - 1 and this.doStreamCalls.length - 1) so the first call returns args.doGenerate[0] (and similarly for doStream). Locate the branches handling Array.isArray(args.doGenerate) and Array.isArray(args.doStream) in mockModel.ts and update the index expressions accordingly while keeping the existing push behavior.
🧹 Nitpick comments (2)
src/validators.ts (1)
261-306: Resolve@todocomments onproviderMetadata/providerOptionsbefore shipping.Lines 274-277 and 302-305 have
@todo Should we continue to include?on bothvToolApprovalRequestandvToolApprovalResponse. These should be resolved before release — if they aren't needed, removing them avoids growing the stored document size; if they are needed, the@todoshould be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validators.ts` around lines 261 - 306, Resolve the TODOs for providerMetadata and providerOptions in vToolApprovalRequest and vToolApprovalResponse: either remove those properties from both validator objects if they are unused (and delete any related storage/usage) or keep them but remove the `@todo` and explicitly define their shape/optional status (e.g., make providerMetadata and providerOptions optional with a concrete schema/types) so the validators (vToolApprovalRequest and vToolApprovalResponse) are final and documented.src/client/createTool.ts (1)
349-356:wrapToolsputs a bare{ ctx }object in the output for falsy tool entries.When
toolis falsy,tool && !(tool as any).__acceptsCtxevaluates tofalseand execution falls into theelsebranch. Spreadingnull/undefinedproduces{ ctx }, which is placed in the output set as a corrupt tool entry.ToolSetshouldn't carry nulls in well-typed code, but this is trivially broken in plain JavaScript or after anas anycast.♻️ Proposed fix
- if (tool && !(tool as any).__acceptsCtx) { + if (!tool || !(tool as any).__acceptsCtx) { output[name] = tool; } else { const out = { ...tool, ctx }; output[name] = out; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/createTool.ts` around lines 349 - 356, The loop in wrapTools incorrectly treats falsy tool entries as objects and places a bare { ctx } into output; update the conditional so falsy tools are preserved (or left as-is) instead of being spread: in the for loop over toolSet, first check if tool is falsy (if (!tool) output[name] = tool), otherwise check the __acceptsCtx flag (if (!(tool as any).__acceptsCtx) output[name] = tool), and only when tool is truthy and accepts ctx create the merged out = { ...tool, ctx } and assign it to output[name]; reference toolSet, output, wrapTools, ctx, and __acceptsCtx when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/createTool.ts`:
- Around line 309-332: The hook wrappers (onInputStart, onInputDelta,
onInputAvailable, toModelOutput) call getCtx(this) and may pass undefined to
user callbacks; mirror the existing guard used in execute and needsApproval by
checking the result of getCtx(this) and throwing the same descriptive error when
ctx is missing before calling the original hook
(origOnInputStart/origOnInputDelta/origOnInputAvailable/origToModelOutput), so
user hooks receive a validated ctx or get the clear actionable throw instead of
a cryptic runtime TypeError.
In `@src/client/mockModel.ts`:
- Around line 1-5: The array-access off-by-one bug occurs in the mock
implementation where doGenerate and doStream push a call then immediately index
it using this.doGenerateCalls.length and this.doStreamCalls.length (so first
call reads index 1); update the indexing to use this.doGenerateCalls.length - 1
and this.doStreamCalls.length - 1 when retrieving the last pushed entry
(references: doGenerate, doStream, doGenerateCalls, doStreamCalls) so the
returned/streamed response uses the correct zero-based index.
- Around line 15-21: DEFAULT_USAGE currently uses a flat token layout; update it
to match AI SDK v6’s LanguageModelV3GenerateResult usage shape by making
inputTokens and outputTokens nested objects and adjusting fields accordingly:
replace outputTokens:number and inputTokens:number with inputTokens:{ total:3,
noCache:3, cacheRead:undefined, cacheWrite:undefined } and outputTokens:{
total:10, text:10, reasoning:undefined } (keep totalTokens:13 if needed) and
remove or map inputTokenDetails/outputTokenDetails to the new
cacheRead/cacheWrite fields so DEFAULT_USAGE matches the v6 spec; modify the
DEFAULT_USAGE constant in src/client/mockModel.ts.
In `@src/validators.ts`:
- Around line 118-145: vToolCallPart currently uses the same discriminant
literal for both union variants which prevents fast-path discriminant matching;
update the two variants so the new-format object keeps type:
v.literal("tool-call") and the legacy-format object uses a distinct literal such
as type: v.literal("tool-call-legacy") (or "tool-call-v1") in the v.union
definition, and then search for any runtime/type checks that inspect the type
field and update them to accept the new legacy literal if needed; this change is
centered on the vToolCallPart validator to enable proper discriminated-union
behavior.
---
Outside diff comments:
In `@src/client/mockModel.ts`:
- Around line 187-191: The array access for mocked responses is off-by-one:
after pushing the call into this.doGenerateCalls (and this.doStreamCalls), the
code uses args.doGenerate[this.doGenerateCalls.length] (and
args.doStream[this.doStreamCalls.length]) which skips the first element; change
those accesses to use the previous index (this.doGenerateCalls.length - 1 and
this.doStreamCalls.length - 1) so the first call returns args.doGenerate[0] (and
similarly for doStream). Locate the branches handling
Array.isArray(args.doGenerate) and Array.isArray(args.doStream) in mockModel.ts
and update the index expressions accordingly while keeping the existing push
behavior.
In `@src/validators.ts`:
- Around line 147-156: vToolResultContent's image branch is missing the
mediaType field; update the image object inside vToolResultContent to include
mediaType: v.optional(v.string()) (and keep mimeType: v.optional(v.string()) for
backward compatibility) so it matches vImagePart and vFilePart. Modify the union
branch in vToolResultContent (the v.object that currently has data and mimeType)
to add mediaType as an optional string field.
---
Duplicate comments:
In `@docs/agent-usage.mdx`:
- Line 46: The docs show inconsistent imports for stepCountIs (imported once
from "@convex-dev/agent" and elsewhere from "ai"); pick the correct canonical
module (either "@convex-dev/agent" or "ai") and update every example to import
stepCountIs from that single source, adjusting the import lines that reference
stepCountIs and related symbols (e.g., Agent, stepCountIs) so all examples
consistently use the chosen module.
In `@src/client/createTool.ts`:
- Around line 241-243: The guard that throws when inputSchema is missing runs
before emitting deprecation warnings, so users passing only the deprecated
def.args never see the migration hint; move the deprecation warnings for
def.args and def.handler (calls to warnDeprecation) to run before the existence
check or perform the deprecation checks immediately before the throw, then
compute inputSchema = def.inputSchema ?? def.args and only throw after
warnDeprecation has been invoked so users get the migration message; reference
the inputSchema variable, def.inputSchema/def.args, and warnDeprecation when
making this change.
---
Nitpick comments:
In `@src/client/createTool.ts`:
- Around line 349-356: The loop in wrapTools incorrectly treats falsy tool
entries as objects and places a bare { ctx } into output; update the conditional
so falsy tools are preserved (or left as-is) instead of being spread: in the for
loop over toolSet, first check if tool is falsy (if (!tool) output[name] =
tool), otherwise check the __acceptsCtx flag (if (!(tool as any).__acceptsCtx)
output[name] = tool), and only when tool is truthy and accepts ctx create the
merged out = { ...tool, ctx } and assign it to output[name]; reference toolSet,
output, wrapTools, ctx, and __acceptsCtx when making the change.
In `@src/validators.ts`:
- Around line 261-306: Resolve the TODOs for providerMetadata and
providerOptions in vToolApprovalRequest and vToolApprovalResponse: either remove
those properties from both validator objects if they are unused (and delete any
related storage/usage) or keep them but remove the `@todo` and explicitly define
their shape/optional status (e.g., make providerMetadata and providerOptions
optional with a concrete schema/types) so the validators (vToolApprovalRequest
and vToolApprovalResponse) are final and documented.
| if (def.onInputStart) { | ||
| t.onInputStart = def.onInputStart.bind(t, getCtx(t)); | ||
| const origOnInputStart = def.onInputStart; | ||
| t.onInputStart = function (this: Tool<INPUT, OUTPUT>, options) { | ||
| return origOnInputStart.call(this, getCtx(this), options); | ||
| }; | ||
| } | ||
| if (def.onInputDelta) { | ||
| t.onInputDelta = def.onInputDelta.bind(t, getCtx(t)); | ||
| const origOnInputDelta = def.onInputDelta; | ||
| t.onInputDelta = function (this: Tool<INPUT, OUTPUT>, options) { | ||
| return origOnInputDelta.call(this, getCtx(this), options); | ||
| }; | ||
| } | ||
| if (def.onInputAvailable) { | ||
| t.onInputAvailable = def.onInputAvailable.bind(t, getCtx(t)); | ||
| const origOnInputAvailable = def.onInputAvailable; | ||
| t.onInputAvailable = function (this: Tool<INPUT, OUTPUT>, options) { | ||
| return origOnInputAvailable.call(this, getCtx(this), options); | ||
| }; | ||
| } | ||
| if (def.toModelOutput) { | ||
| const origToModelOutput = def.toModelOutput; | ||
| t.toModelOutput = function (this: Tool<INPUT, OUTPUT>, options) { | ||
| return origToModelOutput.call(this, getCtx(this), options); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Hook callbacks silently pass undefined ctx, unlike execute/needsApproval which throw a clear error.
execute (lines 296–302) and needsApproval (lines 279–285) both guard against a missing ctx with a descriptive throw. All four hook wrappers — onInputStart, onInputDelta, onInputAvailable, and toModelOutput — call getCtx(this) without this guard, so if a user defines any of these hooks but neither passes ctx at definition time nor uses wrapTools, the callback receives undefined as its ctx argument. This bypasses the type constraint at runtime and produces a cryptic TypeError deep in user code rather than the standard actionable message.
🛡️ Proposed fix — add the same ctx guard to all hook wrappers
if (def.onInputStart) {
const origOnInputStart = def.onInputStart;
t.onInputStart = function (this: Tool<INPUT, OUTPUT>, options) {
+ if (!getCtx(this)) {
+ throw new Error(
+ "To use a Convex tool, you must either provide the ctx" +
+ " at definition time (dynamically in an action), or use the Agent to" +
+ " call it (which injects the ctx, userId and threadId)",
+ );
+ }
return origOnInputStart.call(this, getCtx(this), options);
};
}
if (def.onInputDelta) {
const origOnInputDelta = def.onInputDelta;
t.onInputDelta = function (this: Tool<INPUT, OUTPUT>, options) {
+ if (!getCtx(this)) {
+ throw new Error(
+ "To use a Convex tool, you must either provide the ctx" +
+ " at definition time (dynamically in an action), or use the Agent to" +
+ " call it (which injects the ctx, userId and threadId)",
+ );
+ }
return origOnInputDelta.call(this, getCtx(this), options);
};
}
if (def.onInputAvailable) {
const origOnInputAvailable = def.onInputAvailable;
t.onInputAvailable = function (this: Tool<INPUT, OUTPUT>, options) {
+ if (!getCtx(this)) {
+ throw new Error(
+ "To use a Convex tool, you must either provide the ctx" +
+ " at definition time (dynamically in an action), or use the Agent to" +
+ " call it (which injects the ctx, userId and threadId)",
+ );
+ }
return origOnInputAvailable.call(this, getCtx(this), options);
};
}
if (def.toModelOutput) {
const origToModelOutput = def.toModelOutput;
t.toModelOutput = function (this: Tool<INPUT, OUTPUT>, options) {
+ if (!getCtx(this)) {
+ throw new Error(
+ "To use a Convex tool, you must either provide the ctx" +
+ " at definition time (dynamically in an action), or use the Agent to" +
+ " call it (which injects the ctx, userId and threadId)",
+ );
+ }
return origToModelOutput.call(this, getCtx(this), options);
};
}📝 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 (def.onInputStart) { | |
| t.onInputStart = def.onInputStart.bind(t, getCtx(t)); | |
| const origOnInputStart = def.onInputStart; | |
| t.onInputStart = function (this: Tool<INPUT, OUTPUT>, options) { | |
| return origOnInputStart.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.onInputDelta) { | |
| t.onInputDelta = def.onInputDelta.bind(t, getCtx(t)); | |
| const origOnInputDelta = def.onInputDelta; | |
| t.onInputDelta = function (this: Tool<INPUT, OUTPUT>, options) { | |
| return origOnInputDelta.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.onInputAvailable) { | |
| t.onInputAvailable = def.onInputAvailable.bind(t, getCtx(t)); | |
| const origOnInputAvailable = def.onInputAvailable; | |
| t.onInputAvailable = function (this: Tool<INPUT, OUTPUT>, options) { | |
| return origOnInputAvailable.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.toModelOutput) { | |
| const origToModelOutput = def.toModelOutput; | |
| t.toModelOutput = function (this: Tool<INPUT, OUTPUT>, options) { | |
| return origToModelOutput.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.onInputStart) { | |
| const origOnInputStart = def.onInputStart; | |
| t.onInputStart = function (this: Tool<INPUT, OUTPUT>, options) { | |
| if (!getCtx(this)) { | |
| throw new Error( | |
| "To use a Convex tool, you must either provide the ctx" + | |
| " at definition time (dynamically in an action), or use the Agent to" + | |
| " call it (which injects the ctx, userId and threadId)", | |
| ); | |
| } | |
| return origOnInputStart.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.onInputDelta) { | |
| const origOnInputDelta = def.onInputDelta; | |
| t.onInputDelta = function (this: Tool<INPUT, OUTPUT>, options) { | |
| if (!getCtx(this)) { | |
| throw new Error( | |
| "To use a Convex tool, you must either provide the ctx" + | |
| " at definition time (dynamically in an action), or use the Agent to" + | |
| " call it (which injects the ctx, userId and threadId)", | |
| ); | |
| } | |
| return origOnInputDelta.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.onInputAvailable) { | |
| const origOnInputAvailable = def.onInputAvailable; | |
| t.onInputAvailable = function (this: Tool<INPUT, OUTPUT>, options) { | |
| if (!getCtx(this)) { | |
| throw new Error( | |
| "To use a Convex tool, you must either provide the ctx" + | |
| " at definition time (dynamically in an action), or use the Agent to" + | |
| " call it (which injects the ctx, userId and threadId)", | |
| ); | |
| } | |
| return origOnInputAvailable.call(this, getCtx(this), options); | |
| }; | |
| } | |
| if (def.toModelOutput) { | |
| const origToModelOutput = def.toModelOutput; | |
| t.toModelOutput = function (this: Tool<INPUT, OUTPUT>, options) { | |
| if (!getCtx(this)) { | |
| throw new Error( | |
| "To use a Convex tool, you must either provide the ctx" + | |
| " at definition time (dynamically in an action), or use the Agent to" + | |
| " call it (which injects the ctx, userId and threadId)", | |
| ); | |
| } | |
| return origToModelOutput.call(this, getCtx(this), options); | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/createTool.ts` around lines 309 - 332, The hook wrappers
(onInputStart, onInputDelta, onInputAvailable, toModelOutput) call getCtx(this)
and may pass undefined to user callbacks; mirror the existing guard used in
execute and needsApproval by checking the result of getCtx(this) and throwing
the same descriptive error when ctx is missing before calling the original hook
(origOnInputStart/origOnInputDelta/origOnInputAvailable/origToModelOutput), so
user hooks receive a validated ctx or get the clear actionable throw instead of
a cryptic runtime TypeError.
| import type { | ||
| LanguageModelV2, | ||
| LanguageModelV2Content, | ||
| LanguageModelV2StreamPart, | ||
| LanguageModelV3, | ||
| LanguageModelV3Content, | ||
| LanguageModelV3StreamPart, | ||
| } from "@ai-sdk/provider"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file to understand the context
head -100 src/client/mockModel.ts | cat -nRepository: get-convex/agent
Length of output: 3833
🏁 Script executed:
# Check if the file exists and get basic info
wc -l src/client/mockModel.tsRepository: get-convex/agent
Length of output: 87
🏁 Script executed:
# Search for LanguageModelV3 imports and usage
rg "LanguageModelV3" src/client/mockModel.ts -A 2 -B 2Repository: get-convex/agent
Length of output: 2718
🏁 Script executed:
# Check package.json to see AI SDK versions
cat package.json | grep -A 5 '"@ai-sdk/provider"'
cat package.json | grep -A 5 '"ai"'Repository: get-convex/agent
Length of output: 675
🏁 Script executed:
# Check the simulateReadableStream import
rg "simulateReadableStream" src/client/mockModel.ts -B 2 -A 2Repository: get-convex/agent
Length of output: 469
🏁 Script executed:
# Look at lines around 180-200 to see the doGenerate array access
sed -n '175,200p' src/client/mockModel.ts | cat -nRepository: get-convex/agent
Length of output: 1010
🏁 Script executed:
# Check the full doStream method to see the parallel structure
sed -n '200,225p' src/client/mockModel.ts | cat -nRepository: get-convex/agent
Length of output: 1082
🏁 Script executed:
# Search for all uses of 'as any' in the file
rg "as any" src/client/mockModel.ts -B 2 -A 2Repository: get-convex/agent
Length of output: 350
🏁 Script executed:
# Get the complete context around the array access with line numbers
sed -n '176,230p' src/client/mockModel.ts | cat -nRepository: get-convex/agent
Length of output: 2132
🏁 Script executed:
# Check if there are any recent commits that might show if this is pre-existing
git log --oneline -20 -- src/client/mockModel.tsRepository: get-convex/agent
Length of output: 161
Fix off-by-one bug in array indexing for doGenerate and doStream.
The migration to LanguageModelV3 types is correct and complete. However, there's an off-by-one error in the array response path:
- Lines 190 and 212 access
this.doGenerateCalls.lengthandthis.doStreamCalls.lengthrespectively, but these are incremented before the access. Afterpush(), the length is already 1 for the first call, so it accesses index[1]instead of[0].
Change both to use this.doGenerateCalls.length - 1 and this.doStreamCalls.length - 1 to get the correct zero-based index.
All imports, types, and signatures correctly use LanguageModelV3 from @ai-sdk/provider v3.0.3 and simulateReadableStream from ai v6.0.35.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/mockModel.ts` around lines 1 - 5, The array-access off-by-one bug
occurs in the mock implementation where doGenerate and doStream push a call then
immediately index it using this.doGenerateCalls.length and
this.doStreamCalls.length (so first call reads index 1); update the indexing to
use this.doGenerateCalls.length - 1 and this.doStreamCalls.length - 1 when
retrieving the last pushed entry (references: doGenerate, doStream,
doGenerateCalls, doStreamCalls) so the returned/streamed response uses the
correct zero-based index.
| const DEFAULT_USAGE = { | ||
| outputTokens: 10, | ||
| inputTokens: 3, | ||
| totalTokens: 13, | ||
| inputTokenDetails: undefined, | ||
| outputTokenDetails: undefined, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -type f -name "mockModel.ts" -o -name "*.ts" | head -20Repository: get-convex/agent
Length of output: 115
🏁 Script executed:
cat -n src/client/mockModel.tsRepository: get-convex/agent
Length of output: 9298
🏁 Script executed:
rg "LanguageModelV3" --type ts -A 3 -B 3Repository: get-convex/agent
Length of output: 6195
🏁 Script executed:
rg "usage.*Type\|UsageType" --type ts | head -20Repository: get-convex/agent
Length of output: 42
🌐 Web query:
AI SDK v6 LanguageModelV3 usage type doGenerate fields documentation
💡 Result:
In AI SDK v6, a LanguageModelV3 provider implements:
doGenerate(options: LanguageModelV3CallOptions): Promise<LanguageModelV3GenerateResult>(See the V3 provider spec overview.) [1]
LanguageModelV3GenerateResult (what doGenerate() must return)
A minimal valid result looks like this (from the official MockLanguageModelV3 examples): [2]
{
content: [{ type: 'text', text: 'Hello, world!' }],
finishReason: { unified: 'stop', raw: undefined },
usage: {
inputTokens: {
total: 10,
noCache: 10,
cacheRead: undefined,
cacheWrite: undefined,
},
outputTokens: {
total: 20,
text: 20,
reasoning: undefined,
},
},
warnings: [],
}Fields
content: array of generated content parts (e.g.text,reasoning,tool-call,file,source). [1]finishReason:{ unified, raw }(unified reason + provider raw reason). [2]usage(nested in v6):inputTokens:{ total, noCache, cacheRead?, cacheWrite? }outputTokens:{ total, text?, reasoning? }[2]
warnings: provider warnings (often[]). [2]- Optional metadata commonly returned by providers:
request/responsebodies (and in streaming there areresponse-metadataparts). [1]
Where this type is used
Middleware wrapGenerate receives doGenerate: () => PromiseLike<LanguageModelV3GenerateResult>. [3]
Sources: [1] [2] [3]
DEFAULT_USAGE structure does not match AI SDK v6 LanguageModelV3GenerateResult specification.
The mock uses a flat structure with outputTokens, inputTokens, totalTokens, and *TokenDetails fields, but AI SDK v6 expects nested objects: inputTokens: { total, noCache, cacheRead?, cacheWrite? } and outputTokens: { total, text?, reasoning? }. Update the structure at lines 15-21 to conform to the v6 usage type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/mockModel.ts` around lines 15 - 21, DEFAULT_USAGE currently uses a
flat token layout; update it to match AI SDK v6’s LanguageModelV3GenerateResult
usage shape by making inputTokens and outputTokens nested objects and adjusting
fields accordingly: replace outputTokens:number and inputTokens:number with
inputTokens:{ total:3, noCache:3, cacheRead:undefined, cacheWrite:undefined }
and outputTokens:{ total:10, text:10, reasoning:undefined } (keep totalTokens:13
if needed) and remove or map inputTokenDetails/outputTokenDetails to the new
cacheRead/cacheWrite fields so DEFAULT_USAGE matches the v6 spec; modify the
DEFAULT_USAGE constant in src/client/mockModel.ts.
| // Union type to support both old (args) and new (input) formats | ||
| // Both include input for type hint support | ||
| export const vToolCallPart = v.union( | ||
| // New format: input is primary, args is optional for backwards compat | ||
| v.object({ | ||
| type: v.literal("tool-call"), | ||
| toolCallId: v.string(), | ||
| toolName: v.string(), | ||
| input: v.any(), | ||
| /** @deprecated Use `input` instead. */ | ||
| args: v.optional(v.any()), | ||
| providerExecuted: v.optional(v.boolean()), | ||
| providerOptions, | ||
| providerMetadata, | ||
| }), | ||
| // Legacy format: args is present, input is optional | ||
| v.object({ | ||
| type: v.literal("tool-call"), | ||
| toolCallId: v.string(), | ||
| toolName: v.string(), | ||
| /** @deprecated Use `input` instead. */ | ||
| args: v.any(), | ||
| input: v.optional(v.any()), | ||
| providerExecuted: v.optional(v.boolean()), | ||
| providerOptions, | ||
| providerMetadata, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's read the actual file to see the code at lines 118-145
head -150 src/validators.ts | tail -35Repository: get-convex/agent
Length of output: 1098
🏁 Script executed:
#!/bin/bash
# Look for all v.union patterns in the validators.ts file to understand the pattern
rg -n 'v\.union' src/validators.ts -A 3Repository: get-convex/agent
Length of output: 4408
🏁 Script executed:
#!/bin/bash
# Check if there are tests for vToolCallPart that would validate the union behavior
fd -e test.ts -e test.js -e spec.ts -e spec.js | xargs rg -l 'vToolCallPart\|ToolCallPart' 2>/dev/null | head -5Repository: get-convex/agent
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Look for how other discriminated unions are structured in the codebase
rg -n 'v\.literal\(' src/validators.ts | head -20Repository: get-convex/agent
Length of output: 727
vToolCallPart union should use distinct type literals for each variant, like other discriminated unions in the codebase.
Both variants currently share type: v.literal("tool-call"), forcing Convex to use sequential fallback validation instead of fast-path discriminant matching. While this works correctly (variant 1 requires input, variant 2 requires args), it's inconsistent with the codebase pattern. Every other discriminated union here uses distinct type literal values (e.g., vToolResultOutput uses "text", "json", "execution-denied"; vToolResultContent variants use distinct types). Consider using type: v.literal("tool-call") for the new format and type: v.literal("tool-call-legacy") (or a version discriminant) for backwards compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/validators.ts` around lines 118 - 145, vToolCallPart currently uses the
same discriminant literal for both union variants which prevents fast-path
discriminant matching; update the two variants so the new-format object keeps
type: v.literal("tool-call") and the legacy-format object uses a distinct
literal such as type: v.literal("tool-call-legacy") (or "tool-call-v1") in the
v.union definition, and then search for any runtime/type checks that inspect the
type field and update them to accept the new legacy literal if needed; this
change is centered on the vToolCallPart validator to enable proper
discriminated-union behavior.
Summary
Upgrades
@convex-dev/agentto support AI SDK v6, dropping support for AI SDK v5.Fixes #202
Based on @zboyles's work in #208, merged with latest main and lint/typecheck fixes.
Breaking Changes
ai@^6.0.35)Key Changes
tool-approval-request,tool-approval-response)inputSchemareplacesparametersin tool definitionsinputreplacesargsin tool calls (with backwards compat)mediaTypereplacesmimeType(with backwards compat)embeddingModelconfig option (deprecatestextEmbeddingModel)Migration
See the AI SDK v6 migration guide for details on AI SDK changes.
Test plan
npm run typecheckpassesnpm run lintpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Breaking Changes
Bug Fixes
Documentation