Conversation
Broader fix for calling makeFunctionReference inside components
* complete proposal reduce convex ref escape hatches * Stop makeFunctionReference being defined inside components (#17) Broader fix for calling makeFunctionReference inside components * complete proposal reduce convex ref escape hatches * harden types
* complete proposal for expanding widget convex wrapper hooks * include generic typed calls to makeFunctionReference
Move and update docs
…ing-ref-factories sdk-core string ref replacements
…al-convex-wrapper-hooks introduce-mobile-local-convex-wrapper-hooks
* Manual edits to email capture widget (+ CI summary labels text) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Djanogly <45178753+djanogly@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…TBC) (#24) * Add file uploads - UI needs work * improve styling, remove Snippets button * Update unit tests * skip dismiss email collection test * fallback and tweaks * Limit filetypes Partially completed - the upload spoofing hardening is improved, but not all the way to magic-byte sniffing. We now require an allowlisted extension from the normalized filename on both the client and backend in supportAttachments.ts (line 70) and supportAttachments.ts (line 57), and added regression coverage in supportAttachments.test.ts (line 151) and supportAttachments.test.ts (line 8). I did not add signature checks, because the current finalizeUpload boundary is a Convex mutation and ctx.storage.get() is only available in actions. Doing true magic-byte validation would need a larger refactor of that finalize flow.
* Proper AI knowledge handling with Mastra * feat: replace legacy AI agent knowledge retrieval with Convex Vector Search * docs: update spec to mention convex instead of mastra * docs: archive use-convex-vector-search and update main specs * fix: use existing Vercel AI Gateway client for embedding generation * fix: retrieve full embedding document in aiAgentActionsKnowledge to access contentType and contentId * working AI agent and suggestions * Fixes for Typing gaps, unused ref, invalid vector filter chaining Unused ref warning in packages/convex/convex/embeddings.ts Invalid vector filter chaining in packages/convex/convex/suggestions.ts * harden types * update AGENTS.md
* Add embedding refresh button * Tighten permissions
…#29) * insert links to knowledge base from admin chat, and use vector search * Encourage checks in AGENTS, update dependency allowlist
* Use any model for AI Agent * restrict embedding model choice to vectors with 1536 dimensions, not 3072. * fix tests * Address type issues etc * Address typing issues etc. add docs, tests and display the models in UI * make error in fetching models explicit * update deps * Fix playwright tests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Review Summary by QodoModularize codebase, fix campaign deletion bug, and enhance settings/survey utilities
WalkthroughsDescription• Fixed infinite loop in email campaign deletion by adding explicit batch size tracking in emailCampaigns.ts • Refactored large monolithic files into modular, domain-specific submodules: - testData.ts reduced from 3356 to 24 lines by extracting mutations into separate files - series.ts split into three focused submodules (authoring, runtime, telemetry) - testing/helpers.ts reorganized into domain-specific helper modules - workspaces.ts extracted hosted onboarding logic into dedicated files • Introduced authMutation wrapper to simplify authentication and permission checks in mutation handlers • Added new settings page Convex data fetching hook (useSettingsPageConvex.ts) centralizing workspace settings queries and mutations • Added survey answer validation utility functions (surveyOverlay/answers.ts) for type checking and normalization • Added landing page demo data seeding mutations with comprehensive knowledge base articles and messenger configuration • Simplified E2E test setup by removing conditional workspace policy manipulation • Maintained backward compatibility across all refactored modules by preserving public API exports Diagramflowchart LR
A["Monolithic Files<br/>testData, series, helpers,<br/>workspaces"] -->|"Extract & Modularize"| B["Domain-Specific<br/>Submodules"]
C["Campaign Deletion<br/>Infinite Loop"] -->|"Fix Batch Tracking"| D["Corrected Loop Logic"]
E["Settings & Survey<br/>Requirements"] -->|"Add New Utilities"| F["useSettingsPageConvex<br/>Survey Validators"]
B --> G["Improved Maintainability<br/>& Code Organization"]
D --> G
F --> G
File Changes1. packages/convex/convex/testData.ts
|
Code Review by Qodo
1. npm/npx commands in docs
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+49
to
+103
| ```bash | ||
| npm create convex@latest my-app -- -t owner/repo | ||
| npm create convex@latest my-app -- -t owner/repo#branch | ||
| ``` | ||
|
|
||
| ### Scaffold the project | ||
|
|
||
| Always pass the project name and template flag to avoid interactive prompts: | ||
|
|
||
| ```bash | ||
| npm create convex@latest my-app -- -t react-vite-shadcn | ||
| cd my-app | ||
| npm install | ||
| ``` | ||
|
|
||
| The scaffolding tool creates files but does not run `npm install`, so you must run it yourself. | ||
|
|
||
| To scaffold in the current directory (if it is empty): | ||
|
|
||
| ```bash | ||
| npm create convex@latest . -- -t react-vite-shadcn | ||
| npm install | ||
| ``` | ||
|
|
||
| ### Start the dev loop | ||
|
|
||
| `npx convex dev` is a long-running watcher process that syncs backend code to a Convex deployment on every save. It also requires authentication on first run (browser-based OAuth). Both of these make it unsuitable for an agent to run directly. | ||
|
|
||
| **Ask the user to run this themselves:** | ||
|
|
||
| Tell the user to run `npx convex dev` in their terminal. On first run it will prompt them to log in or develop anonymously. Once running, it will: | ||
|
|
||
| - Create a Convex project and dev deployment | ||
| - Write the deployment URL to `.env.local` | ||
| - Create the `convex/` directory with generated types | ||
| - Watch for changes and sync continuously | ||
|
|
||
| The user should keep `npx convex dev` running in the background while you work on code. The watcher will automatically pick up any files you create or edit in `convex/`. | ||
|
|
||
| **Exception - cloud or headless agents:** Environments that cannot open a browser for interactive login should use Agent Mode (see below) to run anonymously without user interaction. | ||
|
|
||
| ### Start the frontend | ||
|
|
||
| The user should also run the frontend dev server in a separate terminal: | ||
|
|
||
| ```bash | ||
| npm run dev | ||
| ``` | ||
|
|
||
| Vite apps serve on `http://localhost:5173`, Next.js on `http://localhost:3000`. | ||
|
|
||
| ### What you get | ||
|
|
||
| After scaffolding, the project structure looks like: | ||
|
|
There was a problem hiding this comment.
1. npm/npx commands in docs 📘 Rule violation § Compliance
Changed documentation/agent-skill content introduces package-management commands using npm/npx instead of pnpm. This violates the repo requirement to use PNPM exclusively and can lead to inconsistent installs and lockfile drift.
Agent Prompt
## Issue description
New/updated docs and agent-skill references use `npm`/`npx` commands, but this repo requires PNPM-only commands.
## Issue Context
Use `pnpm install`, `pnpm run <script>`, and `pnpm dlx <cli>` equivalents wherever PNPM can perform the same task.
## Fix Focus Areas
- .agents/skills/convex-quickstart/SKILL.md[49-103]
- .agents/skills/convex-migration-helper/references/migrations-component.md[7-16]
- .agents/skills/convex-migration-helper/references/migrations-component.md[64-71]
- .agents/skills/convex-migration-helper/references/migrations-component.md[127-129]
- packages/convex/convex/testData/demoWorkspace.ts[295-305]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+27
to
+53
| export function webQueryRef<Args extends WebArgs, Result>( | ||
| functionName: string | ||
| ): WebQueryRef<Args, Result> { | ||
| return makeFunctionReference<"query", Args, Result>(functionName) as WebQueryRef<Args, Result>; | ||
| } | ||
|
|
||
| export function webMutationRef<Args extends WebArgs, Result>( | ||
| functionName: string | ||
| ): WebMutationRef<Args, Result> { | ||
| return makeFunctionReference<"mutation", Args, Result>(functionName) as WebMutationRef< | ||
| Args, | ||
| Result | ||
| >; | ||
| } | ||
|
|
||
| export function webActionRef<Args extends WebArgs, Result>( | ||
| functionName: string | ||
| ): WebActionRef<Args, Result> { | ||
| return makeFunctionReference<"action", Args, Result>(functionName) as WebActionRef<Args, Result>; | ||
| } | ||
|
|
||
| export function useWebQuery<Args extends WebArgs, Result>( | ||
| queryRef: WebQueryRef<Args, Result>, | ||
| args: Args | "skip" | ||
| ): Result | undefined { | ||
| return useQuery(queryRef as never, args as never) as Result | undefined; | ||
| } |
There was a problem hiding this comment.
2. webqueryref() calls makefunctionreference 📘 Rule violation ✧ Quality
apps/web/src/lib/convex/hooks.ts introduces exported ref-factory helpers that invoke makeFunctionReference(...) inside functions. This violates the requirement that makeFunctionReference values be created at module scope only (created once at module load).
Agent Prompt
## Issue description
`webQueryRef`, `webMutationRef`, and `webActionRef` call `makeFunctionReference(...)` inside function bodies, which violates the module-scope-only requirement.
## Issue Context
Even if callers invoke these helpers at top level, the `makeFunctionReference(...)` invocation itself is still inside a function body (not file top-level).
## Fix Focus Areas
- apps/web/src/lib/convex/hooks.ts[27-53]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+410
to
+427
| export async function waitForAIHandoffButton(page: Page, timeout = 15000): Promise<Locator> { | ||
| const frame = getWidgetContainer(page); | ||
| const handoffButton = frame.locator( | ||
| "[data-testid='handoff-button'], button:has-text('Talk to human'), button:has-text('Talk to a human'), button:has-text('human'), button:has-text('agent')" | ||
| ); | ||
|
|
||
| await expect(handoffButton.first()).toBeVisible({ timeout }); | ||
| return handoffButton.first(); | ||
| } | ||
|
|
||
| /** | ||
| * Waits for the AI feedback buttons to be visible. | ||
| */ | ||
| export async function waitForAIFeedbackButtons(page: Page, timeout = 15000): Promise<Locator> { | ||
| const frame = getWidgetContainer(page); | ||
| const feedbackButtons = frame.locator( | ||
| "[data-testid='feedback-helpful'], [data-testid='feedback-not-helpful'], .feedback-button, button[aria-label*='helpful'], button[aria-label*='not helpful']" | ||
| ); |
There was a problem hiding this comment.
3. E2e selectors use text/classes 📘 Rule violation ▣ Testability
Updated Playwright helpers locate elements using text and CSS class selectors (e.g., button:has-text(...), .feedback-button). This makes E2E tests brittle and violates the requirement to use data-testid selectors for element targeting.
Agent Prompt
## Issue description
Playwright E2E helpers use text/class selectors to find elements, which is disallowed for stable selectors.
## Issue Context
Selectors should target stable `[data-testid="..."]` attributes; if the UI lacks a stable test id, add one and update the test.
## Fix Focus Areas
- apps/web/e2e/helpers/widget-helpers.ts[172-176]
- apps/web/e2e/helpers/widget-helpers.ts[338-342]
- apps/web/e2e/helpers/widget-helpers.ts[410-427]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+469
to
+547
| const seedVisitor = internalMutation({ | ||
| args: { | ||
| workspaceId: v.id("workspaces"), | ||
| email: v.optional(v.string()), | ||
| name: v.optional(v.string()), | ||
| externalUserId: v.optional(v.string()), | ||
| customAttributes: v.optional(v.any()), | ||
| location: v.optional( | ||
| v.object({ | ||
| city: v.optional(v.string()), | ||
| region: v.optional(v.string()), | ||
| country: v.optional(v.string()), | ||
| countryCode: v.optional(v.string()), | ||
| }) | ||
| ), | ||
| device: v.optional( | ||
| v.object({ | ||
| browser: v.optional(v.string()), | ||
| os: v.optional(v.string()), | ||
| deviceType: v.optional(v.string()), | ||
| }) | ||
| ), | ||
| }, | ||
| handler: async (ctx, args) => { | ||
| requireTestDataEnabled(); | ||
| const timestamp = Date.now(); | ||
| const randomSuffix = Math.random().toString(36).substring(2, 8); | ||
| const sessionId = `${E2E_TEST_PREFIX}session_${timestamp}_${randomSuffix}`; | ||
|
|
||
| const visitorId = await ctx.db.insert("visitors", { | ||
| sessionId, | ||
| workspaceId: args.workspaceId, | ||
| email: args.email || `${E2E_TEST_PREFIX}visitor_${randomSuffix}@test.opencom.dev`, | ||
| name: args.name || `E2E Test Visitor ${randomSuffix}`, | ||
| externalUserId: args.externalUserId, | ||
| customAttributes: args.customAttributes || { | ||
| plan: "free", | ||
| signupDate: new Date().toISOString(), | ||
| }, | ||
| location: args.location || { | ||
| city: "San Francisco", | ||
| region: "California", | ||
| country: "United States", | ||
| countryCode: "US", | ||
| }, | ||
| device: args.device || { | ||
| browser: "Chrome", | ||
| os: "macOS", | ||
| deviceType: "desktop", | ||
| }, | ||
| firstSeenAt: timestamp, | ||
| lastSeenAt: timestamp, | ||
| createdAt: timestamp, | ||
| }); | ||
|
|
||
| await ctx.db.patch(visitorId, { | ||
| readableId: formatReadableVisitorId(visitorId), | ||
| }); | ||
|
|
||
| return { visitorId, sessionId }; | ||
| }, | ||
| }); | ||
|
|
||
| /** | ||
| * Seeds a test segment for E2E testing. | ||
| */ | ||
| const seedSegment = internalMutation({ | ||
| args: { | ||
| workspaceId: v.id("workspaces"), | ||
| name: v.optional(v.string()), | ||
| audienceRules: v.optional(v.any()), | ||
| }, | ||
| handler: async (ctx, args) => { | ||
| requireTestDataEnabled(); | ||
| const timestamp = Date.now(); | ||
| const randomSuffix = Math.random().toString(36).substring(2, 8); | ||
| const name = args.name || `${E2E_TEST_PREFIX}segment_${randomSuffix}`; | ||
|
|
||
| const defaultRules = { |
There was a problem hiding this comment.
4. v.any() used without exceptions 📘 Rule violation ⛨ Security
New/modified Convex internal mutations use v.any() for args validators, but the exceptions allowlist was cleared. This violates the requirement to use narrow validators or explicitly document each v.any() in security/convex-v-any-arg-exceptions.json.
Agent Prompt
## Issue description
Convex handlers introduce `v.any()` validators, but there are no corresponding allowlist entries.
## Issue Context
Either replace `v.any()` with a narrow validator (preferred) or add a specific, justified entry for each handler/arg in `security/convex-v-any-arg-exceptions.json`.
## Fix Focus Areas
- packages/convex/convex/testData/seeds.ts[469-547]
- packages/convex/convex/testing/helpers/content.ts[213-227]
- security/convex-v-any-arg-exceptions.json[1-5]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+30
to
+32
| function getInternalRef(name: string): unknown { | ||
| return makeFunctionReference(name); | ||
| } |
There was a problem hiding this comment.
5. Dynamic makefunctionreference(name) undocumented 📘 Rule violation ⌂ Architecture
packages/convex/convex/testAdmin.ts introduces a dynamic makeFunctionReference(name) path without an inline justification comment. This violates the requirement that dynamic function reference creation be explicitly documented as an intentional exception.
Agent Prompt
## Issue description
Dynamic `makeFunctionReference(name)` is used without an explicit comment documenting why this cannot be expressed with generated refs or a fixed constant.
## Issue Context
If this is intentionally dynamic (e.g., a secret-protected test admin gateway), it must be documented inline as an exception per the boundaries-ordering policy.
## Fix Focus Areas
- packages/convex/convex/testAdmin.ts[30-32]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+131
to
+173
| const AUTHORIZE_CONVERSATION_ACCESS_REF = makeFunctionReference< | ||
| "query", | ||
| ConversationAccessArgs, | ||
| ConversationAccessResult | ||
| >("aiAgent:authorizeConversationAccess") as unknown as ConvexRef< | ||
| "query", | ||
| "internal", | ||
| ConversationAccessArgs, | ||
| ConversationAccessResult | ||
| >; | ||
|
|
||
| const GET_RUNTIME_SETTINGS_FOR_WORKSPACE_REF = makeFunctionReference< | ||
| "query", | ||
| WorkspaceIdArgs, | ||
| RuntimeSettings | ||
| >("aiAgent:getRuntimeSettingsForWorkspace") as unknown as ConvexRef< | ||
| "query", | ||
| "internal", | ||
| WorkspaceIdArgs, | ||
| RuntimeSettings | ||
| >; | ||
|
|
||
| const RECORD_RUNTIME_DIAGNOSTIC_REF = makeFunctionReference< | ||
| "mutation", | ||
| RuntimeDiagnosticArgs, | ||
| unknown | ||
| >("aiAgent:recordRuntimeDiagnostic") as unknown as ConvexRef< | ||
| "mutation", | ||
| "internal", | ||
| RuntimeDiagnosticArgs, | ||
| unknown | ||
| >; | ||
|
|
||
| const CLEAR_RUNTIME_DIAGNOSTIC_REF = makeFunctionReference< | ||
| "mutation", | ||
| WorkspaceIdArgs, | ||
| Id<"aiAgentSettings"> | null | ||
| >("aiAgent:clearRuntimeDiagnostic") as unknown as ConvexRef< | ||
| "mutation", | ||
| "internal", | ||
| WorkspaceIdArgs, | ||
| Id<"aiAgentSettings"> | null | ||
| >; |
There was a problem hiding this comment.
6. Undocumented as unknown as casts 📘 Rule violation ≡ Correctness
New Convex runtime code adds chained type-escape casts (as unknown as) without the required localized justification comment. This violates the rule restricting unsafe casts in runtime paths to narrowly scoped, documented exceptions.
Agent Prompt
## Issue description
Runtime Convex code uses `as unknown as` casts as a typing escape hatch without documenting why the type system cannot represent the pattern.
## Issue Context
Preferred order is generated `api`/`internal` refs; if TS2589 forces a workaround, keep it hotspot-local with the smallest cast possible and add a brief comment explaining why and how to remove it later.
## Fix Focus Areas
- packages/convex/convex/aiAgentActions.ts[131-173]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.