012 visual builder update

[MikePfunk28]

Merge pull request #19 from MikePfunk28/main

@MikePfunk28
MikePfunk28 authored on Oct 24, 2025
Merge pull request #20 from MikePfunk28/main

@MikePfunk28
MikePfunk28 authored on Oct 24, 2025
Commits on Oct 27, 2025
fix som✅ Complete Summary

@MikePfunk28
MikePfunk28 committed on Oct 27, 2025
Commits on Oct 29, 2025
🚀 Next Steps

@MikePfunk28
MikePfunk28 committed on Oct 29, 2025
Commits on Nov 4, 2025
✅ Complete Summary - What's ACTUALLY Working Now

@MikePfunk28
MikePfunk28 committed on Nov 4, 2025
Commits on Nov 12, 2025
Add GitHub Actions workflow for project automation and enhance error …

@MikePfunk28
MikePfunk28 committed on Nov 12, 2025
Commits on Feb 9, 2026
Fixed github oauth and removed google as I have not yet gotten google…

@MikePfunk28
MikePfunk28 committed 14 hours ago
feat: update model references to Claude 4.5 Haiku across the application

@MikePfunk28
MikePfunk28 committed 18 hours ago
refactor: remove Ollama installation wizard and status components

@MikePfunk28
MikePfunk28 committed 17 hours ago
Commits on Feb 10, 2026
feat: enhance MCP server integration and dynamic tool loading in visu…

@MikePfunk28
MikePfunk28 committed 24 minutes ago

Summary by CodeRabbit

• New Features

  • Visual Scripting builder, session-based AI Agent Builder, multi‑agent swarm orchestration, prompt‑chain executor, and unified multimodal agent execution (text/image/video/speech).
  • Local model support (Ollama/LMStudio), Ollama installer, model‑switching, Agent‑as‑Tool registry, Memory tools, Node Settings editor, and a dynamic Tool/Model catalog.

• Improvements

  • Conversation analysis with AI improvement plans and one‑click apply/preview; usage tracking, tiered limits and Stripe billing; richer model selector with local detection.

• Chores

  • README security badge, .gitignore updates, CI workflow added, Google OAuth sign‑in removed.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agent-builder-application	Ready	Preview, Comment	Feb 12, 2026 6:56pm

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds many new Convex backend modules (workflows, multi‑agent/swarm orchestration, model/modality switching, local model/Ollama support, analysis/improvement, memory, rate limiting, Stripe integration, audit logs), a large schema expansion, frontend visual‑scripting UIs, CI workflow, and numerous tool/utility files.

Changes

Cohort / File(s)	Summary
CI & repo metadata /.github/workflows/setup-github-projects_Version3.yml, /.gitignore, README.md, generateKeys.mjs	Adds GitHub Actions workflow to manage Projects, expands .gitignore, inserts Codacy badge(s) in README, and adds RSA/JWKS key generator.
Schema & core tables convex/schema.ts	Massive schema expansion: new/public tables (users, agents, deployments, workflows, conversationMessages, conversationAnalyses, agentImprovementHistory, agentBuildSessions, rateLimitEntries, toolMemory, testExecutions, multiAgentSessions, multiAgentResults, etc.) and indices.
Agent builder & templates convex/automatedAgentBuilder.ts, convex/agentTemplates.ts, convex/agentAsToolGenerator.ts, src/components/AIAgentBuilder.tsx, src/components/AgentBuilder.tsx	Interactive session-based agent builder backend, template registry, agent-as-tool & coordinator codegen, and corresponding frontend UI/workflow changes.
Analysis & improvement convex/conversationAnalysis.ts, convex/agentImprovement.ts, src/components/ConversationChat.tsx	Conversation analysis, persistable analyses, improvement-plan generation, preview/apply improvement actions, history logging, and UI hooks for analyze/improve.
Multi‑agent & swarm orchestration convex/multiAgentRuntime.ts, convex/swarmTestingOrchestrator.ts	New multi-agent runtime and swarm orchestrator: create/run swarms, messaging, coordination tests, session/result recording, and tool-bridge helpers.
Dynamic & unified model/modality switching convex/lib/dynamicModelSwitching.ts, convex/lib/unifiedModalitySwitching.ts, convex/unifiedAgentExecution.ts, convex/strandsAgentExecutionDynamic.ts, convex/unifiedAgentExecutionTest.ts	Model-tier catalog, complexity scoring, model-selection decision logic, modality detection, unified multimodal execution endpoint, and tests.
Execution routing & runtime convex/agentcoreTestExecution.ts, convex/strandsAgentExecution.ts, convex/deploymentRouter.ts, convex/awsDeployment.ts, convex/containerOrchestrator.ts, convex/queueProcessor.ts, convex/testExecution.ts	Refactors execution paths to prefer direct Bedrock with Ollama/local and Lambda fallback; introduces tiered gating, centralized tier config, billing hooks, retries, and Dockerfile/deployment adjustments.
Local models & Ollama convex/localModelDetector.ts, convex/ollamaInstaller.ts, convex/ollamaMCPIntegration.ts, src/components/ModelSelector.tsx	Local model detection, Ollama installer guides, direct Ollama integration for tests, and frontend model selector merged with local provider detection.
Prompt & workflow executors, tools convex/workflowExecutor.ts, convex/promptChainExecutor.ts, convex/tools.ts, convex/workflowTemplates.ts, convex/workflowTemplateSeeder.ts	Adds workflow execution engine (prompt/model, routed, DAG), prompt-chain executor (sequential/parallel), tool suite (memory/reasoning/HITL), templates, and seeding utilities.
Visual scripting & strands tools convex/strandsAgentsTools.ts, convex/lib/strandsTools.ts, src/components/ToolSelector.tsx, src/components/NodeSettingsDrawer.tsx, src/App.tsx	Large Strands tool registry, normalization utilities, dynamic ToolSelector, NodeSettingsDrawer editor, and Visual Scripting UI integration.
Memory & message execution convex/lib/memoryStore.ts, convex/lib/messageExecutor.ts	Tool memory store with TTL/search and composed message execution that supports Bedrock and Ollama runtime calls plus token usage extraction.
Rate limiting, audit & auth convex/rateLimiter.ts, convex/auditLogs.ts, convex/auth.ts, convex/authDebug.ts, src/SignInForm.tsx	Adds sliding-window rate limiter with admin observability, audit logging APIs, removes Google provider from auth flows, and minor auth UI tweaks.
MCP & connectors convex/mcpClient.ts, convex/mcpConfig.ts, convex/ollamaMCPIntegration.ts	MCP client updates (capabilities changes), adds built-in Ollama MCP server entry, and direct Ollama integration endpoints.
Benchmarks, platform value, utilities convex/modelBenchmarks.ts, convex/platformValue.ts, generateKeys.mjs	Model benchmark queries, platform-value calculation query, and key-generation utility.
Removals & tests convex/lib/stateValidation.ts (removed), convex/multiEnvironmentExecution.test.ts (removed)	Removes the FSM state validator and a large multi-environment test suite.
Stripe & billing convex/stripe.ts, convex/stripeMutations.ts, convex/http.ts, convex/lib/tierConfig.ts, convex/apiKeys.ts	Stripe checkout/portal/reporting, centralized stripeMutations for subscription/usage handling, webhook route, and centralized tier config for limits and provider gating.
Frontend & misc many src/components/*, index.html, mydiagram.py, Security_checkup.py, requirements.txt, package.json	UI guards for no-agents, form autocomplete, dynamic model/tool listings, tool catalog refactor, title change, dependency updates (AWS SDK, Polly, reactflow, Stripe), and assorted small cleanups and formatting changes.

Sequence Diagram(s)

sequenceDiagram
    rect rgba(100,150,240,0.5)
    Client->>Convex: executeUnifiedAgent(agentId, message, options)
    end

    rect rgba(120,200,100,0.5)
    Convex->>DB: fetch agent, conversation history, user tier
    Convex->>UnifiedModelSwitch: decideUnifiedModelSwitch(message, history, agent, options)
    UnifiedModelSwitch-->>Convex: ModelDecision(selectedModel, modality, reasoning)
    end

    alt selectedModel is local (Ollama/LMStudio)
        rect rgba(240,180,80,0.5)
        Convex->>Ollama: POST /api/chat (messages)
        Ollama-->>Convex: response
        end
    else cloud model (Bedrock)
        rect rgba(200,100,160,0.5)
        Convex->>Bedrock: ConverseCommand (modelId, messages)
        Bedrock-->>Convex: response
        end
    end

    rect rgba(180,120,220,0.5)
    Convex->>S3/DB: upload media or persist execution/result
    S3/DB-->>Convex: URLs/IDs
    Convex-->>Client: unified execution result (content, mediaUrls, metadata)
    end

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~150 minutes

"🐇
I hopped through code and stitched the streams,
Built swarms of agents and stitched their dreams,
Ollama hummed and Bedrock answered true,
Workflows and tools in a shiny new view,
Hop in — the forge awaits what you'll do!"

🚥 Pre-merge checks | ✅ 1 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (49 files): ⚔️ .gitignore (content) ⚔️ README.md (content) ⚔️ assistant/main.py (content) ⚔️ convex/_generated/api.d.ts (content) ⚔️ convex/_generated/dataModel.d.ts (content) ⚔️ convex/_generated/server.d.ts (content) ⚔️ convex/_generated/server.js (content) ⚔️ convex/agentBuilderWorkflow.ts (content) ⚔️ convex/agentExecution.test.ts (content) ⚔️ convex/agentcoreSetup.ts (content) ⚔️ convex/agentcoreTestExecution.ts (content) ⚔️ convex/agents.ts (content) ⚔️ convex/apiKeys.ts (content) ⚔️ convex/auth.ts (content) ⚔️ convex/authDebug.ts (content) ⚔️ convex/awsDeployment.ts (content) ⚔️ convex/codeGenerator.ts (content) ⚔️ convex/containerOrchestrator.ts (content) ⚔️ convex/deploymentRouter.ts (content) ⚔️ convex/errorLogging.ts (content) ⚔️ convex/guardrails.ts (content) ⚔️ convex/http.ts (content) ⚔️ convex/integration.test.ts (content) ⚔️ convex/interleavedReasoning.ts (content) ⚔️ convex/mcpClient.ts (content) ⚔️ convex/mcpConfig.ts (content) ⚔️ convex/modelBenchmarks.ts (content) ⚔️ convex/modelRegistry.ts (content) ⚔️ convex/queueProcessor.ts (content) ⚔️ convex/realExecution.test.ts (content) ⚔️ convex/schema.ts (content) ⚔️ convex/strandsAgentExecution.ts (content) ⚔️ convex/testExecution.ts (content) ⚔️ convex/testHelpers.test.ts (content) ⚔️ convex/toolModelIntegration.test.ts (content) ⚔️ index.html (content) ⚔️ package.json (content) ⚔️ requirements.txt (content) ⚔️ src/App.tsx (content) ⚔️ src/SignInForm.tsx (content) ⚔️ src/components/AIAgentBuilder.tsx (content) ⚔️ src/components/AgentBuilder.tsx (content) ⚔️ src/components/CognitoLogin.tsx (content) ⚔️ src/components/ConversationChat.tsx (content) ⚔️ src/components/InterleavedChat.tsx (content) ⚔️ src/components/ModelSelector.tsx (content) ⚔️ src/components/OAuthDebugPanel.tsx (content) ⚔️ src/components/ToolSelector.tsx (content) ⚔️ src/main.tsx (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.
Title check	❓ Inconclusive	The title '012 visual builder update' is vague and non-descriptive, using generic phrasing that doesn't clearly convey what the primary change is.	Replace with a more specific title that highlights the main change, such as 'Add visual workflow builder with dynamic model switching and multimodal tool support' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 012-visual-builder-update

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch 012-visual-builder-update
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR introduces a “Visual Scripting” workflow builder path and expands backend support for workflows, agent improvement, dynamic model selection, and rate limiting, while also removing Google OAuth references and adjusting local dev defaults.

Changes:

• Add workflow persistence/publishing APIs (workflows, templates, seeding) and expose Visual Scripting view in the UI.
• Add conversation analysis + auto-improvement actions and expand schema for unified identity, usage tracking, and multi-agent sessions.
• Security/ops updates: remove Google OAuth UI/config, add rate limiting primitives, and adjust dev server port.

Reviewed changes

Copilot reviewed 71 out of 86 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/components/OAuthDebugPanel.tsx	Removes Google OAuth setup hint from debug panel.
src/components/InterleavedChat.tsx	Adds “no agents” blocking state and null-safe message count rendering.
src/components/ConversationChat.tsx	Adds analyze/improve actions and an analysis UI panel.
src/components/CognitoLogin.tsx	Adds autocomplete attributes for username/password fields.
src/components/AgentBuilder.tsx	Updates default model selection to use catalog provider default.
src/SignInForm.tsx	Adds email autocomplete; removes Google OAuth sign-in button.
src/App.tsx	Introduces Visual Scripting navigation/view wiring.
package.json	Changes Vite dev port; bumps AWS SDK packages; adds reactflow.
mydiagram.py	Removes an unprofessional placeholder file.
index.html	Updates app title.
generateKeys.mjs	Adds script to generate JWT private key + JWKS output.
convex/workflows.ts	Adds workflow CRUD, sanitization, and publish-to-agent action.
convex/workflowTemplates.ts	Adds static workflow templates + template-to-workflow creation mutation.
convex/workflowTemplateSeeder.ts	Adds DB seeder for workflow templates.
convex/users.ts	Adds user queries including internal user lookup.
convex/unifiedUserIdentity.ts	Adds unified identity + anonymous merge and access checks.
convex/unifiedAgentExecutionTest.ts	Adds actions to test unified modality/model switching logic.
convex/testExecution.ts	Adds auth enforcement, rate limiting, and usage/audit tracking changes.
convex/strandsAgentExecutionDynamic.ts	Adds dynamic model switching execution path (direct Bedrock).
convex/strandsAgentExecution.ts	Updates default/fallback model selection.
convex/schema.ts	Expands schema (workflows, templates, analyses, improvements, usage, rate limits, multi-agent).
convex/rateLimiter.ts	Adds sliding-window rate limiter utilities + storage mutations/queries.
convex/queueProcessor.ts	Simplifies queue routing/logging and adjusts failure stage.
convex/promptChainExecutor.ts	Adds prompt chaining/parallel execution actions and tool execution stubs.
convex/platformValue.ts	Adds “platform value” query for cost/value breakdown.
convex/ollamaMCPIntegration.ts	Adds action-based Ollama HTTP integration for tests.
convex/ollamaInstaller.ts	Adds actions to provide Ollama install guidance and recommendations.
convex/multiAgentRuntime.ts	Adds multi-agent orchestration wiring + session/result persistence mutations.
convex/modelBenchmarks.ts	Exposes benchmark data via Convex queries.
convex/mcpConfig.ts	Adds built-in Ollama MCP server config entry.
convex/mcpClient.ts	Adjusts MCP init capabilities payload.
convex/lib/strandsTools.ts	Adds tool name normalization/metadata lookup helper.
convex/lib/stateValidation.ts	Removes test execution state machine validator module.
convex/lib/messageExecutor.ts	Adds server-side executor for composed Bedrock/Ollama messages.
convex/lib/memoryStore.ts	Adds internal toolMemory CRUD helpers with TTL filtering.
convex/lib/dynamicModelSwitching.ts	Adds complexity scoring and model tier decision logic.
convex/interleavedReasoning.ts	Updates prompts/typing and adds explicit AWS credential checks + model env var.
convex/guardrails.ts	Updates default model id for cost calculation.
convex/errorLogging.ts	Tightens handler typing.
convex/deploymentRouter.ts	Adds validation for AgentCore-only model formats.
convex/conversationAnalysis.ts	Adds conversation analysis storage/query and improvement plan generation.
convex/containerOrchestrator.ts	Adds usage tracking hooks for ECS test execution.
convex/codeGenerator.ts	Switches tool import path; adds agent-as-tool wrapper generation.
convex/awsDeployment.ts	Generates Dockerfile differently for Ollama vs Bedrock agents.
convex/automatedAgentBuilder.ts	Adds Woz-style automated builder session flow and Bedrock invocation.
convex/authDebug.ts	Removes Google provider from debug; updates local URL to port 4000.
convex/auth.ts	Removes Google auth provider from providers list.
convex/auditLogs.ts	Adds audit log mutations/queries (admin TODO).
convex/agents.ts	Adds modelProvider and sourceWorkflowId fields to agents create/update.
convex/agentcoreSetup.ts	Minor comment tweak.
convex/agentTemplates.ts	Adds pre-built agent templates queries/data.
convex/agentImprovement.ts	Adds agent auto-improvement pipeline using conversation analysis.
convex/agentBuilderWorkflow.ts	Updates default workflow model id.
convex/agentAsToolGenerator.ts	Adds dedicated agent-as-tool + coordinator generator actions.
convex/_generated/server.js	Updates generated server helper docs/imports.
Security_checkup.py	Adds a Python file with imports (currently invalid syntax).
README.md	Adds Codacy badge under Security section.
.github/workflows/setup-github-projects_Version3.yml	Adds workflow to auto-create/configure GitHub Projects and add issues/PRs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The mutation accepts userId from the client and writes it directly to workflows.userId, which allows creating workflows on behalf of other users. Derive userId from auth inside the handler (e.g., via ctx.auth.getUserIdentity()/getAuthUserId()/your getUserScope() approach) and remove userId from the public args.

Suggested change

	userId: v.string()
	},
	handler: async (ctx, { templateId, name, userId }) => {
	},
	handler: async (ctx, { templateId, name }) => {
	const identity = await ctx.auth.getUserIdentity();
	if (!identity) {
	throw new Error("Not authenticated");
	}
	const userId = identity.subject;

[Copilot]

The mutation accepts userId from the client and writes it directly to workflows.userId, which allows creating workflows on behalf of other users. Derive userId from auth inside the handler (e.g., via ctx.auth.getUserIdentity()/getAuthUserId()/your getUserScope() approach) and remove userId from the public args.

[Copilot]

This uses identity?.subjectId, but other code in this repo treats the identity field as identity.subject (see e.g. workflows.ts). If subjectId is undefined at runtime, withRateLimit will always throw. Use the correct identity field (or getAuthUserId(ctx) if you expect a users table Id) and make the stored rateLimitEntries.userId consistent with that choice.

[Copilot]

testsThisMonth is incremented here at submission time, but it is also incremented again in incrementUserUsage (which is described as tracking successful executions). This will double-count tests. Choose a single source of truth: either increment on submission (and don’t increment on success), or keep testsThisMonth as 'successful tests' and only increment in incrementUserUsage.

Suggested change

	// RATE LIMITING: Increment user's test count ONLY for cloud models (not Ollama)
	if (!isOllamaModel) {
	const user = await ctx.db.get(userId);
	if (user) {
	await ctx.db.patch(userId, {
	testsThisMonth: (user.testsThisMonth || 0) + 1,
	lastTestAt: Date.now(),
	});
	}
	}
	// RATE LIMITING: Track last test submission ONLY for cloud models (not Ollama)
	if (!isOllamaModel) {
	const user = await ctx.db.get(userId);
	if (user) {
	await ctx.db.patch(userId, {
	lastTestAt: Date.now(),
	});
	}
	}

[Copilot]

This increments testsThisMonth for successful completions, but submitTest/retryTest already increment testsThisMonth earlier. If you keep this as 'successful tests', remove the earlier increments; if you keep 'attempted tests', then this internal mutation should not increment it (or should increment a separate successfulTestsThisMonth).

[Copilot]

Convex documents do not serialize undefined values reliably; storing objects with undefined fields can fail at runtime or lead to inconsistent data. Omit the validator key entirely (or set it to null/a concrete value) before inserting templates.

Suggested change

	validator: undefined,
	validator: null,

[Copilot]

ctx.auth.getUserIdentity().subject is typically an auth subject string, not guaranteed to be a Convex Id<\"users\">. Using it with ctx.db.get(...) can return null or the wrong record. Use getAuthUserId(ctx) (if you want the users table Id) or query by a stable indexed field (e.g., by_email, or provider identifiers) consistent with how users are created.

[Copilot]

getAllLogs currently returns audit logs to any authenticated user, and explicitly notes an admin check is missing. This is sensitive data exposure. Implement an authorization check before returning logs (e.g., verify user.tier/role/flag, or restrict to system/admin identities) and otherwise return an error or empty set.

Suggested change

	// TODO: Add admin check here
	// Enforce admin-only access before returning any audit logs.
	const user = await ctx.db.get(userId);
	if (!user || user.role !== "admin") {
	// Non-admins are not allowed to view global audit logs.
	return [];
	}

[Copilot]

This file contains invalid Python syntax (trailing semicolon on an import) and looks like a scratchpad rather than production code. If it’s not used, remove it from the repo; if it is intended to be used, fix syntax and place it under an appropriate package/module with tests or documentation.

Suggested change

	from strands_tools import think, readfile;
	from strands_tools import think, readfile

[Copilot]

The comment says pricing is 'as of 2024' while the default model id is for a newer model generation; this makes the pricing assumptions unclear and potentially misleading. Update the comment (and/or pricing constants) to match the actual model and pricing source/date being used.

Suggested change

	// Claude Haiku 4.5 pricing (as of 2024)
	// Claude Haiku 4.5 pricing assumptions for us.anthropic.claude-haiku-4-5-20250514-v1:0 (based on Anthropic pricing as of 2025-05-14)

[cloudflare-workers-and-pages]

Deploying agent-builder-application with    Cloudflare Pages

Latest commit:	86250e7
Status:	✅  Deploy successful!
Preview URL:	https://a80c641b.agent-builder-application.pages.dev
Branch Preview URL:	https://012-visual-builder-update.agent-builder-application.pages.dev

View logs

[coderabbitai]

Actionable comments posted: 33

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (16)

convex/agentBuilderWorkflow.ts (1)

354-396: ⚠️ Potential issue | 🟠 Major

streamWorkflowExecution uses a different model and provider than the rest of the file.

executeWorkflowStage / invokeWorkflowModel use Bedrock with WORKFLOW_MODEL_ID (Haiku 4.5), but streamWorkflowExecution uses the Anthropic SDK directly with a hardcoded "claude-3-7-sonnet-20250219". This means:

1. Streaming uses a more expensive Sonnet model vs. Haiku for non-streaming — likely unintentional divergence.
2. Requires a separate ANTHROPIC_API_KEY env var in addition to AWS credentials.
3. The model string is hardcoded with no env-var override, unlike the Bedrock path.

Consider aligning the model selection or at minimum making the streaming model configurable.

src/components/CognitoLogin.tsx (1)

233-244: ⚠️ Potential issue | 🟠 Major

Default credentials displayed in plaintext in the UI.

The password AgentBuilder2025! is rendered directly in the component. Even if these are "starter" credentials from a CloudFormation stack, shipping them in the frontend bundle makes it trivial for anyone to discover them. Consider removing the password from the UI and directing users to their CloudFormation stack outputs instead.

src/SignInForm.tsx (1)

22-22: ⚠️ Potential issue | 🟠 Major

Form data (including password) is logged to the console.

Object.fromEntries(formData) will include the plaintext password. This should be removed or at minimum redact the password field before logging.

Proposed fix

-          console.log("Form data being sent:", Object.fromEntries(formData));
+          // Avoid logging sensitive form data in production

convex/authDebug.ts (1)

3-69: ⚠️ Potential issue | 🟡 Minor

Debug query is unauthenticated and exposes deployment URLs and provider configuration status.

getOAuthConfig has no auth check, so any client can discover your deployment URLs and which OAuth providers are configured. This is useful during development but should be gated behind authentication or removed before production.

convex/errorLogging.ts (2)

147-184: ⚠️ Potential issue | 🟠 Major

Post-fetch success filter applied after take(limit) — returns fewer results than expected.

take(args.limit || 100) truncates results first (lines 159-174), then logs.filter(...) on line 179 further reduces the set. When success is specified alongside eventType or userId, the caller may receive significantly fewer than limit results even though matching records exist in the database.

Move the success filter into the query chain, or apply it before take, or document this as "best effort."

---

66-73: ⚠️ Potential issue | 🟠 Major

Audit logs capture ipAddress and userAgent — verify PII/compliance posture.

Storing IP addresses and user-agent strings in an unbounded audit table may trigger GDPR/CCPA obligations (data-subject access requests, retention limits, right-to-erasure). Ensure there is a documented retention policy and a mechanism to purge or anonymize this data on request.

convex/awsDeployment.ts (3)

637-660: ⚠️ Potential issue | 🟠 Major

config.enableMonitoring || true always evaluates to true — monitoring can never be disabled.

The || operator returns the first truthy operand; since true is always truthy, this expression is always true regardless of the config value. Use the nullish coalescing operator (??) to only default when the value is undefined/null.

Proposed fix

     monitoring: {
-      enabled: config.enableMonitoring || true,
+      enabled: config.enableMonitoring ?? true,
       logLevel: "INFO"
     }

---

662-672: ⚠️ Potential issue | 🟠 Major

deployToAgentCore returns a hardcoded mock — production callers will receive fake ARNs.

The function returns a static mock response with a fake account ID (123456789012). The executeDeployment action on line 158 calls this and stores the result as a completed deployment. If this code path is reachable in production, users will see a "successful" deployment that doesn't actually exist.

Consider either throwing a "not implemented" error or gating the mock behind an environment check.

---

694-698: ⚠️ Potential issue | 🟡 Minor

Fragile user ID type coercion based on string prefix.

Line 696 checks args.userId.startsWith('j') to decide whether to cast to any. This is a brittle heuristic that makes assumptions about Convex's internal ID format, which could change. If the userId arg type is v.union(v.id("users"), v.string()), consider separating the two cases more explicitly or documenting why this check exists.

convex/queueProcessor.ts (2)

12-12: ⚠️ Potential issue | 🟡 Minor

parseInt of an invalid env var yields NaN, silently disabling all queue processing.

If MAX_CONCURRENT_TESTS is set to a non-numeric value, parseInt returns NaN. Since NaN >= NaN is false (line 41) and NaN - runningCount is NaN (line 46), the processor will always see availableSlots as NaN and take(NaN) will likely return zero results. Add a fallback or validate:

Proposed fix

-const MAX_CONCURRENT_TESTS = parseInt(process.env.MAX_CONCURRENT_TESTS || "10");
+const MAX_CONCURRENT_TESTS = Math.max(1, parseInt(process.env.MAX_CONCURRENT_TESTS || "10", 10) || 10);

---

139-181: ⚠️ Potential issue | 🟡 Minor

Test is marked FAILED before retry requeue — brief state flicker.

Lines 143–149 set the test to FAILED, and then lines 163–170 check retry eligibility and requeue (which resets to QUEUED on line 298). During that window, any UI polling the test status will briefly show it as failed. This is a minor UX inconsistency but not blocking.

convex/mcpClient.ts (2)

392-394: ⚠️ Potential issue | 🟠 Major

Overly broad routing condition may hijack calls intended for other servers.

The condition toolName === "execute_agent" (Line 392) routes any tool invocation named "execute_agent" to Bedrock AgentCore, regardless of which MCP server was targeted. If any other MCP server exposes a tool with this name, calls to it will be incorrectly routed to Bedrock.

The server name check alone should be sufficient:

Proposed fix

-  if (server.name === "bedrock-agentcore-mcp-server" || toolName === "execute_agent") {
+  if (server.name === "bedrock-agentcore-mcp-server") {

---

548-554: ⚠️ Potential issue | 🟡 Minor

Non-null assertions on AWS credentials will throw opaque runtime errors.

process.env.AWS_ACCESS_KEY_ID! and process.env.AWS_SECRET_ACCESS_KEY! (Lines 551–552) bypass TypeScript's undefined check. If these env vars are missing, the AWS SDK will receive undefined credentials and produce a confusing downstream error rather than a clear message. The testMCPServerConnection function (Line 686) already validates these before use — this fallback path should do the same or let the SDK use its default credential chain.

Proposed fix — use default credential chain

     const client = new BedrockRuntimeClient({
       region: process.env.AWS_REGION || "us-east-1",
-      credentials: {
-        accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
-        secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
-      },
+      ...(process.env.AWS_ACCESS_KEY_ID && process.env.AWS_SECRET_ACCESS_KEY
+        ? {
+            credentials: {
+              accessKeyId: process.env.AWS_ACCESS_KEY_ID,
+              secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY,
+            },
+          }
+        : {}),
     });

convex/codeGenerator.ts (2)

264-269: ⚠️ Potential issue | 🔴 Critical

Empty tool list generates invalid Python: from strands_tools import ().

When toolImports is empty, Lines 130 + 268 produce:

from strands_tools import (
)

This is a SyntaxError in Python. The import statement should be omitted entirely when no tools are selected.

Proposed fix

-  imports.push("# Tool imports");
-  imports.push("from strands_tools import (");
-  
   // Add tool imports from strands-agents-tools
   const toolImports = new Set<string>();
   tools.forEach(tool => {
     // ... switch cases ...
   });
   
   if (toolImports.size > 0) {
+    imports.push("# Tool imports");
+    imports.push("from strands_tools import (");
     imports.push(...Array.from(toolImports));
     imports.push(")");
   } else {
-    imports.push(")");
+    imports.push("# No tool imports needed");
   }

---

1017-1017: ⚠️ Potential issue | 🟠 Major

linkedAgents parameter is never passed through the main code generation flow.

generateToolConfigs was updated to accept linkedAgents (Line 315), but generateCompleteAgentCode on Line 1017 calls it as generateToolConfigs(tools) without forwarding any linked agents. The generateAgent action's args (Lines 37–63) also don't accept a linkedAgents parameter. The new agent-as-tool wrapping feature is effectively dead code in the main generation path.

src/components/ConversationChat.tsx (1)

409-432: ⚠️ Potential issue | 🟡 Minor

onKeyPress is deprecated; hint text is misleading for a single-line input.

Two issues here:

1. Line 413: onKeyPress is deprecated in React. Use onKeyDown instead.
2. Line 431: The hint says "Shift+Enter for new line" but this is an <input type="text">, which doesn't support multi-line input. Either switch to a <textarea> or remove the misleading hint.

Proposed fix

-            onKeyPress={(e) => e.key === "Enter" && !e.shiftKey && handleSend()}
+            onKeyDown={(e) => e.key === "Enter" && !e.shiftKey && handleSend()}

-        <p className="text-xs text-green-600 mt-2">
-          Press Enter to send • Shift+Enter for new line
-        </p>
+        <p className="text-xs text-green-600 mt-2">
+          Press Enter to send
+        </p>

🤖 Fix all issues with AI agents

In @.github/workflows/setup-github-projects_Version3.yml:
- Around line 64-80: Remove the invalid view-creation commands that use
non-existent gh flags (the three lines invoking "gh project view ... --add-view
table/board/timeline" and their preceding jq checks) because GitHub Projects v2
views cannot be created via the gh CLI; instead either delete that whole "Add
Board, Table, Timeline views" step (references: GH_TOKEN, PROJECT_ID, and the gh
project view commands) or replace it with a single step that echoes clear manual
setup instructions directing the maintainer to create "Tasks Table", "Kanban
Board", and "Schedule Timeline" in the web UI.

In `@convex/agentAsToolGenerator.ts`:
- Around line 38-113: The template injection risk comes from directly
interpolating agentName, description, agentId (and systemPrompt in
generateCoordinatorCode) into Python triple-quoted strings; sanitize these
inputs before interpolation by escaping triple quotes and backslashes (e.g.,
replace """ with an escaped sequence and replace \ with \\) or alternatively
serialize values safely (e.g., JSON/base64 encode or use Python repr-style
escaping) so the generated code cannot be broken out of its string context;
update generateToolCode and generateCoordinatorCode to apply this
sanitization/serialization to agentName, description, agentId, and systemPrompt
before embedding them into the template and also consider validating
length/allowed characters for extra safety.

In `@convex/agentTemplates.ts`:
- Around line 8-93: Update the model ID in the customer_service template: locate
the customer_service object in AGENT_TEMPLATES and replace the incorrect model
string "us.anthropic.claude-haiku-4-5-20250514-v1:0" with the correct Bedrock
model ID "anthropic.claude-haiku-4-5-20251001-v1:0" so the Code Review Agent
uses the valid Claude Haiku 4.5 model.

In `@convex/auditLogs.ts`:
- Around line 72-102: getAllLogs currently allows any authenticated user to read
all audit logs; add an admin-only guard or convert the endpoint to
internal-only. Update the handler in getAllLogs to call your authorization
helper (e.g., getAuthUserId plus a check like isAdminForUserId or getUserRole)
and return [] or throw if the caller is not an admin, or replace export const
getAllLogs = query(...) with export const getAllLogs = internalQuery(...) so it
is inaccessible to non-server callers; reference getAllLogs, getAuthUserId, and
the ctx/db auditLogs queries when applying the change.
- Around line 13-44: Change the public mutation logEvent to an internal-only
endpoint and enforce server-side caller restrictions: replace mutation({ ... })
with internalMutation({ ... }) for logEvent so it is not client-callable, remove
or validate direct client-supplied userId (use ctx.auth or server-verified
identity inside the handler to set userId), and ensure any callers invoke
logEvent only from trusted server code paths; keep the same payload fields but
perform server-side verification of userId and rate/duplicate protection inside
the logEvent handler (referencing the logEvent function and its
handler/ctx.args).

In `@convex/automatedAgentBuilder.ts`:
- Around line 360-372: The code clears gathered requirements by returning
requirements: {} when readyToGenerate is false; change the non-final return in
automatedAgentBuilder (the block using parsed, suggestions and
formattedQuestion) to preserve/merge existing requirements (e.g., use
parsed.requirements merged into the current session requirements) instead of an
empty object, and also update the caller in processResponse/updateBuildSession
so the patch merges with session.agentRequirements rather than replacing it;
reference parsed, suggestions, formattedQuestion, readyToGenerate, requirements,
session.agentRequirements, processResponse and updateBuildSession to locate and
implement the merge behavior.

In `@convex/awsDeployment.ts`:
- Around line 602-619: generateAgentCoreDockerfile currently treats
undefined/null agent.model as an Ollama model and unsafely interpolates it into
the entrypoint shell; change the detection so only a defined string without a
dot is treated as Ollama (e.g., set isOllamaModel = typeof agent.model ===
'string' && !agent.model.includes('.')), and before embedding agent.model into
the generated entrypoint.sh validate or safely quote it (either validate against
a safe pattern like /^[A-Za-z0-9._-]+$/ and throw/fallback if invalid, or
escape/JSON.stringify the value) so the ollama pull ${agent.model} invocation
cannot be exploited; update references in generateAgentCoreDockerfile and the
entrypoint.sh generation accordingly.

In `@convex/conversationAnalysis.ts`:
- Around line 315-332: The calculateImpact function can divide by zero when
successes + issues === 0; modify calculateImpact to guard that denominator by
computing const denom = successes + issues; if denom === 0 set currentScore = 0
(or derive a safe default) otherwise compute currentScore = (successes / denom)
* 100; then compute projectedScore = Math.min(100, currentScore +
(analysis.suggestedImprovements.length * 10)); ensure both scores are clamped
and rounded before computing improvement so Math.round(projectedScore -
currentScore) never operates on NaN; reference calculateImpact,
analysis.identifiedIssues, analysis.successfulInteractions, and
analysis.suggestedImprovements.
- Around line 64-70: The avgResponseLength calculation in the analysis object
can divide by zero when assistantMessages is empty; update the computation in
conversationAnalysis.ts (the analysis object where conversationMetrics and
avgResponseLength are computed) to check assistantMessages.length and use 0 (or
null) when it's 0, e.g. compute total via assistantMessages.reduce(...) and set
avgResponseLength = assistantMessages.length ? total / assistantMessages.length
: 0 so downstream checks (like > 1000) won't get NaN.

In `@convex/interleavedReasoning.ts`:
- Around line 376-389: The batch insert assigns the same sequenceNumber
(timestamp) to every message in addMessageBatch (loop over args.messages),
causing non-deterministic ordering; fix by computing a monotonically increasing
sequence per message (e.g., capture a base timestamp once and add the loop index
or an increment for each msg) and use that value for sequenceNumber instead of
the raw timestamp so messages in the same batch retain deterministic order when
later sorted by sequenceNumber.

In `@convex/localModelDetector.ts`:
- Around line 1-79: The detectLocalModels internalAction (and its helper
functions detectOllama, detectLlamaCpp, detectLMStudio, detectGGUF) incorrectly
perform local filesystem/process operations (execSync, spawn, fs.*) inside a
Convex action which runs in the cloud; move all actual detection logic to
client-side code (Electron/CLI/browser) and make detectLocalModels a thin
server-side stub that accepts trusted detection results or triggers the client
to run detection and POST results back; specifically remove any
execSync/spawn/fs calls from detectLocalModels and the helper functions,
implement a client module that runs
detectOllama/detectLlamaCpp/detectLMStudio/detectGGUF locally and calls a new
API endpoint (or uses existing api.*) to send the ModelDetectionResult to the
server, and update the server-side detectLocalModels to validate and store the
client-submitted ModelDetectionResult instead of performing local detection
itself.
- Around line 251-333: The setupOllama handler currently builds and execSyncs
raw shell strings (installCommand) including unsafe "curl | sh" and
platform-mismatched commands and directly interpolates args.installModels into
`ollama pull ${model}`, creating command-injection and Windows compatibility
risks; fix by removing/avoiding piping remote scripts (do not run "curl | sh" in
installCommand), implement platform-specific install paths without shell
concatenation for Windows (map 'win32' -> use documented Windows installer or
return a clear manual-install response), replace execSync(rawCommand) with safer
APIs (spawnFile/execFile or run bundled installers) and/or a strict whitelist
for allowed install operations, and validate/sanitize model names from
args.installModels (allow only /^[a-z0-9-_\.]+$/i or match against an explicit
whitelist) before invoking `ollama pull`, catching and returning sanitized
errors; update references in code: setupOllama, installCommand, execSync usage,
and the loop over args.installModels.

In `@convex/mcpConfig.ts`:
- Around line 77-99: The built-in MCP server entry with _id "system_ollama" in
BUILT_IN_MCP_SERVERS hardcodes a developer-specific Windows path in args; change
it to read the executable path from an environment variable (e.g.
process.env.OLLAMA_MCP_PATH) and use that in the args array (or omit/disable the
entry if the env var is unset) so other machines/CI/production won't break;
update the object with the args reference and ensure any consumers of
BUILT_IN_MCP_SERVERS still handle a missing/disabled Ollama entry.

In `@convex/modelBenchmarks.ts`:
- Around line 505-510: The MODEL_BENCHMARKS array contains Infinity for Ollama
models which breaks Convex serialization; replace all Infinity occurrences in
the Ollama model entries inside MODEL_BENCHMARKS with a safe sentinel (e.g., -1
or null) and update any consumers—specifically getAllBenchmarks and
getBenchmarkForModel will now return the sanitized values, and update
getBestValueModels to ignore/filter out the sentinel when computing best-value
models so no Infinity is ever returned or used in comparisons.

In `@convex/ollamaMCPIntegration.ts`:
- Around line 1-11: This module is marked "use node" and currently makes fetch
calls to http://127.0.0.1:11434 (Ollama) from server-side actions, which will
fail on hosted Convex; update the implementation so these calls do not run on
the server: either move the Ollama-calling logic into client-side code (where
browser/local machine can reach localhost), or implement a configurable proxy
endpoint the user hosts and read its URL from an env/config flag, or gate the
server actions behind a development-only feature flag and return a clear
graceful error in production; locate the server actions created with action(...)
in this module and change them to call a client-side helper or check an ENV
(e.g., process.env.USE_LOCAL_OLLAMA) before attempting to fetch localhost.

In `@convex/platformValue.ts`:
- Around line 9-13: calculatePlatformValue's handler fetches an agent without
any authorization; update the handler to get the current user id (via
ctx.auth.userId or equivalent), fetch the agent (ctx.db.get(args.agentId)), and
enforce the same access rule used by agents.get(): only proceed and return the
calculated value if agent.createdBy === userId or agent.isPublic is true,
otherwise return null (or throw unauthorized). Ensure you reference the
calculatePlatformValue handler, use ctx.auth to obtain identity, and apply the
agent.createdBy/isPublic check before performing any value calculation.

In `@convex/rateLimiter.ts`:
- Around line 270-310: The code calls
ctx.runQuery(internal.rateLimiter.getRateLimitEntry, ...) inside the
internalQuery getRateLimitStatus which will fail at runtime; replace that call
with a direct database access using ctx.db to fetch the rate-limit entry for
args.userId and args.action (e.g., use ctx.db.get or ctx.db.query/table lookups
against your rate limit table/record to locate the entry), keep the rest of the
handler logic (window calculation, validRequests, return shape) unchanged and
remove usage of ctx.runQuery.

In `@convex/strandsAgentExecution.ts`:
- Line 155: The fallback Bedrock model ID used to set modelId is wrong and will
fail at runtime; update the default string in the modelId assignment (the
expression setting modelId in strandsAgentExecution.ts) to use the correct
Claude Haiku 4.5 inference profile "us.anthropic.claude-haiku-4-5-20251001-v1:0"
instead of the old date stamp; if you need cross-region routing consider using
the global profile "global.anthropic.claude-haiku-4-5-20251001-v1:0" as an
alternative.

In `@convex/strandsAgentExecutionDynamic.ts`:
- Around line 230-240: The payload currently always includes the Claude-only
"thinking" block which will break non-Claude models; update the code that builds
the payload (where payload is constructed before calling InvokeModel) to include
thinking: { type: "enabled", budget_tokens: 3000 } only when the resolved model
from decideModelSwitch (or its resolved identifier) is a Claude/Anthropic model
(e.g., check the model name or a isClaude boolean returned by
decideModelSwitch). Ensure other fields like anthropic_version are likewise
conditional or adjusted when a non-Anthropic model is selected so InvokeModel
receives only supported params; reference the payload object, the thinking
property, and the decideModelSwitch/InvokeModel call to locate and gate the
conditional inclusion.
- Around line 203-215: Filter history so messages only include roles "user" and
"assistant": in the loop that builds messages (the messages array from history
and the later push of the current message), skip entries where msg.role ===
"system" or msg.role === "tool". Extract any "system" messages into a separate
systemText (or systemMessages) value to set the top-level system field on the
Anthropic/Bedrock request instead of placing them into messages, and transform
"tool" entries into the API's expected tool_result content blocks (or otherwise
omit/handle them according to the API) rather than pushing them as normal
content; update the construction that uses messages and message to include the
systemText in the top-level system field when invoking InvokeModelCommand.

In `@convex/swarmTestingOrchestrator.ts`:
- Around line 624-643: storeSwarm currently just logs and discards the swarm and
getSwarm always returns null, which makes all callers (sendMessageToSwarm,
switchToAgent, testSwarmCoordination, getSwarmStatus, testStrandsSwarm, etc.)
fail with "Swarm not found"; implement persistence by saving swarms to a real
table or an in-memory map and returning them from getSwarm: update storeSwarm to
persist args.swarm (e.g., insert/put into a new "swarms" table or a module-level
Map keyed by swarm.id) and update getSwarm to look up and return the stored
SwarmDefinition (or null only if missing), ensuring types match SwarmDefinition
and the internalAction/internalQuery handlers use the persistence API you
choose.
- Around line 674-697: The code incorrectly sets createdBy to args.parentAgentId
(an agent id) and never actually creates agents, so createdAgentIds is filled
with the same parent id; change createdBy to the correct user id (e.g.,
args.userId or args.viewer?.id — whatever the codebase uses for the requesting
user's id) instead of args.parentAgentId, and replace the placeholder push with
a real create call (use the existing createAgent mutation/function or db insert
that accepts agentData) capturing the returned agent id and pushing that to
createdAgentIds; update references: agentData (set createdBy to the user id),
agentConfigs loop, and the createdAgentIds collection to store the newly created
agent ids rather than args.parentAgentId.
- Around line 779-788: The code calls .map on result.responses but
sendMessageToSwarm returns responses as a Record (plain object), so
result.responses.map will throw; update the formattedResults construction to
iterate Object.entries(result.responses) (or Object.keys/values as appropriate)
and then map over those entries to produce the same shape (referencing
formattedResults, result.responses and sendMessageToSwarm) so agentId comes from
the entry key and response from the entry value.

In `@convex/testExecution.ts`:
- Line 85: The Ollama detection should prioritize deploymentType to avoid
misclassifying Bedrock models; change the isOllamaModel logic to check
agent.deploymentType === "ollama" first and only fall back to the ':' heuristic
when deploymentType is falsy (e.g., const isOllamaModel = agent.deploymentType
=== "ollama" || (!agent.deploymentType && agent.model.includes(':'))), and apply
the same fix in retryTest where the same colon heuristic is used so cloud models
with colons aren’t treated as Ollama and bypass rate limiting.

In `@convex/tools.ts`:
- Around line 64-94: The invokeLLM function currently hardcodes the Ollama
endpoint to "http://localhost:11434" which fails in Convex cloud; update
invokeLLM to derive the ollama endpoint dynamically (prefer using the agent's
ollamaEndpoint field if available on the agent/context passed into invokeLLM,
otherwise fallback to an environment variable like OLLAMA_ENDPOINT) and plug
that value into the composed.ollama.endpoint; modify the call sites or signature
of invokeLLM to accept a context/agent parameter (or read
process.env.OLLAMA_ENDPOINT) so executeComposedMessages receives the correct
remote endpoint instead of localhost.

In `@convex/unifiedAgentExecution.ts`:
- Around line 196-210: The current code builds an Anthropic-specific payload and
always sends it via InvokeModelCommand (payload, InvokeModelCommand,
anthropic_version, system, thinking), which will fail for non-Anthropic models
chosen by decideUnifiedModelSwitch; update unifiedAgentExecution to branch on
the selected model family (use decideUnifiedModelSwitch or inspect
config.modelId) and either: 1) construct an Anthropic-specific body and call
InvokeModelCommand for Anthropic models, or 2) use a provider-agnostic request
path such as ConverseCommand (or the provider-specific schema) for Titan/Cohere
etc., mapping agent.systemPrompt/messages into the appropriate fields; ensure
the code paths reference the same variables
(payload/messages/config.modelId/agent.systemPrompt) and only send the matching
schema to the SDK.

In `@convex/users.ts`:
- Around line 27-34: The code incorrectly treats identity.subject as a Convex
document Id and calls ctx.db.get(identity.subject), which will return null;
instead, fetch the user by the token index: call ctx.db.query or ctx.db.getAll
with the users.by_token index (or the equivalent index name created by
`@convex-dev/auth`) using identity.subject from ctx.auth.getUserIdentity(), e.g.,
query the users table by_token with identity.subject and return the found user;
ensure you remove the cast to any and rely on the index lookup (verify the
users.by_token index exists in the schema).

In `@convex/workflowExecutor.ts`:
- Around line 224-258: The recursive executor suffers a race where multiple
paths can start the same node before it finishes; replace the post-completion
Set "executed" with an in-flight Map<string, Promise<any>> so duplicate calls
return the same promise. In executeNodeRecursive, check if inFlight.has(nodeId)
and return that promise; otherwise create the promise immediately (e.g. const p
= (async () => { ... })()), set inFlight.set(nodeId, p) before awaiting deps or
executing the node, perform the existing dependency resolution, call
executeNode(ctx, node, nodeInput, nodeResults) inside that promise, then set
nodeResults.set(nodeId, result), push to executionLog, remove the entry from
inFlight (or keep it if you prefer memoization) and resolve with the result.
Keep references to nodeResults, executeNode, dependencies, and executionLog
intact.

In `@convex/workflows.ts`:
- Around line 414-423: The provider detection treats any model with ":" as
Ollama which is inverted for Bedrock IDs; update the heuristic in the block that
sets provider (and thus deploymentType and modelProvider) so that when
config.provider is empty you inspect the substring before the ":" and classify
as "bedrock" if that prefix contains a dot (e.g., "anthropic.claude-...:0") and
as "ollama" if it does not (e.g., "llama3:latest"); then derive deploymentType
and modelProvider from that provider value as before (references: provider,
model.includes(":"), deploymentType, modelProvider).

In `@convex/workflowTemplates.ts`:
- Around line 273-315: createWorkflowFromTemplate accepts an arbitrary userId
and writes it directly; remove the user-supplied userId arg and instead derive
the owner from the authenticated context (e.g., call your auth helper like
getAuthUserId(ctx) or use ctx.auth.userId), throw an error if the caller is
unauthenticated, and use that authenticated id when inserting into "workflows";
update the mutation args to drop userId and adjust any callers/tests accordingly
so workflows are always attributed to the authenticated user.

In `@requirements.txt`:
- Around line 73-74: Update the cryptography dependency minimum to a safe
patched version: replace the current requirement "cryptography>=42.0.0" with a
minimum of at least "cryptography>=44.0.1" (preferably "cryptography>=46.0.0" or
newer) in requirements.txt so the CVE-2024-12797 and other known vulnerabilities
are excluded; ensure any dependency pins or CI dependency checks are updated
accordingly and run tests to validate compatibility with functions that import
or use the cryptography package.

In `@src/components/ModelSelector.tsx`:
- Around line 1-9: The ModelProvider union type is missing "lmstudio", causing a
type mismatch with LMStudioModelConfig and failing where NodeSettingsDrawer
assigns provider = "lmstudio"; update the ModelProvider definition in
workflowNodes.ts to include "lmstudio" (so the union matches
LMStudioModelConfig), then ensure ModelConfig and any usages (e.g.,
NodeSettingsDrawer.tsx) type-check correctly against the updated ModelProvider.

In `@src/components/NodeSettingsDrawer.tsx`:
- Around line 453-462: When renaming a key in the entries map the current
onChange handler calls removeEntry(key) then setEntry(nextKey, value), causing
two separate updateConfig/setDraft calls and a stale-closure overwrite; change
the handler on the input so it performs a single updateConfig/setDraft update
that removes the old key and adds the new key in one functional state update
(use the functional updater form to read prev state and produce a new inputs
object that deletes the old key and sets nextKey to value), updating the UI
atomically; target the onChange for the input that currently calls removeEntry
and setEntry, and update removeEntry/setEntry usages to be merged into one
atomic update via updateConfig/setDraft (or equivalent) to avoid lost updates.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Code injection risk: user-controlled strings are interpolated unsanitized into generated Python code.

agentName, description, and agentId are injected directly into Python template literals. If description contains """ or a backslash sequence, it breaks the generated Python or injects arbitrary code. Same applies to systemPrompt in generateCoordinatorCode (line 192).

For example, an agent with description = 'foo"""\nimport os; os.system("rm -rf /")\n"""' would escape the docstring context.

Sanitize all interpolated values — at minimum, escape triple-quotes and backslashes:

Proposed fix

+function escapePython(s: string): string {
+  return s.replace(/\\/g, '\\\\').replace(/"""/g, '\\"\\"\\"').replace(/\n/g, '\\n');
+}
+
 function generateToolCode(
   agentName: string,
   toolName: string,
   description: string,
   agentId: string
 ): string {
+  const safeAgentName = escapePython(agentName);
+  const safeDescription = escapePython(description);
   return `"""
-Agent-as-Tool: ${agentName}
-Auto-generated wrapper to use ${agentName} as a tool in other agents.
+Agent-as-Tool: ${safeAgentName}
+Auto-generated wrapper to use ${safeAgentName} as a tool in other agents.
 """
 ...

🤖 Prompt for AI Agents

In `@convex/agentAsToolGenerator.ts` around lines 38 - 113, The template injection
risk comes from directly interpolating agentName, description, agentId (and
systemPrompt in generateCoordinatorCode) into Python triple-quoted strings;
sanitize these inputs before interpolation by escaping triple quotes and
backslashes (e.g., replace """ with an escaped sequence and replace \ with \\)
or alternatively serialize values safely (e.g., JSON/base64 encode or use Python
repr-style escaping) so the generated code cannot be broken out of its string
context; update generateToolCode and generateCoordinatorCode to apply this
sanitization/serialization to agentName, description, agentId, and systemPrompt
before embedding them into the template and also consider validating
length/allowed characters for extra safety.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

logEvent is a public mutation with no authentication — any caller can inject fake audit log entries.

Audit logs exist for compliance and forensics. Exposing the write path as an unauthenticated public mutation allows any client to:

• Insert fabricated events to cover tracks or frame other users
• Flood the audit log table (no rate limiting)
• Spoof the userId field since there's no verification that the caller owns that ID

This should be an internalMutation invoked only by server-side code (other mutations/actions), not a client-callable endpoint.

🔒 Proposed fix

-import { mutation, query } from "./_generated/server";
+import { internalMutation, query } from "./_generated/server";

-export const logEvent = mutation({
+export const logEvent = internalMutation({

🤖 Prompt for AI Agents

In `@convex/auditLogs.ts` around lines 13 - 44, Change the public mutation
logEvent to an internal-only endpoint and enforce server-side caller
restrictions: replace mutation({ ... }) with internalMutation({ ... }) for
logEvent so it is not client-callable, remove or validate direct client-supplied
userId (use ctx.auth or server-verified identity inside the handler to set
userId), and ensure any callers invoke logEvent only from trusted server code
paths; keep the same payload fields but perform server-side verification of
userId and rate/duplicate protection inside the logEvent handler (referencing
the logEvent function and its handler/ctx.args).

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n 'type ModelProvider|export.*ModelProvider' --type=ts -A 2

Repository: MikePfunk28/agent_builder_application

Length of output: 914

---

🏁 Script executed:

cat src/data/modelCatalog.ts | head -100

Repository: MikePfunk28/agent_builder_application

Length of output: 2824

---

🏁 Script executed:

cat src/types/workflowNodes.ts | head -80

Repository: MikePfunk28/agent_builder_application

Length of output: 1625

---

🏁 Script executed:

rg -n 'from.*workflowNodes.*ModelProvider|import.*ModelProvider' --type=ts

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

rg -n 'LMStudioModelConfig' --type=ts -B 2 -A 5

Repository: MikePfunk28/agent_builder_application

Length of output: 968

---

🏁 Script executed:

rg -n 'ModelConfig\]' --type=ts -B 3 -A 3

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

rg -n 'import.*ModelProvider' --type=ts

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

rg -n 'ModelConfig.*=' --type=ts -B 2 -A 2

Repository: MikePfunk28/agent_builder_application

Length of output: 356

---

🏁 Script executed:

rg -n 'type ModelConfig|ModelConfig\|' --type=ts -B 1 -A 1

Repository: MikePfunk28/agent_builder_application

Length of output: 260

---

🏁 Script executed:

rg -n "from.*workflowNodes|import.*from.*types" src/components/ModelSelector.tsx

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

rg -n "workflowNodes" src/components/ --type=ts

Repository: MikePfunk28/agent_builder_application

Length of output: 232

---

🏁 Script executed:

rg -n "BedrockModelConfig|OllamaModelConfig|LMStudioModelConfig|ModelConfig" src/components/ModelSelector.tsx --type=ts

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

cat src/components/ModelSelector.tsx | head -15

Repository: MikePfunk28/agent_builder_application

Length of output: 479

---

🏁 Script executed:

rg -n "ModelProvider|ModelMetadata" src/components/ModelSelector.tsx --type=ts | head -20

Repository: MikePfunk28/agent_builder_application

Length of output: 295

---

🏁 Script executed:

rg -n "useLocalModels\|detectedToMetadata" src/hooks/useLocalModels.ts --type=ts -A 3 | head -30

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

rg -n "ModelProvider\|ModelConfig" src/types/workflowNodes.ts --type=ts

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

rg -n "ModelConfig" src/components/NodeSettingsDrawer.tsx --type=ts -B 2 -A 2

Repository: MikePfunk28/agent_builder_application

Length of output: 1076

---

🏁 Script executed:

rg -n "listModelsByProvider" src/components/ --type=ts -B 2 -A 2

Repository: MikePfunk28/agent_builder_application

Length of output: 4519

---

🏁 Script executed:

cat src/components/NodeSettingsDrawer.tsx | sed -n '600,630p'

Repository: MikePfunk28/agent_builder_application

Length of output: 1512

---

🏁 Script executed:

cat src/types/workflowNodes.ts | sed -n '53,82p'

Repository: MikePfunk28/agent_builder_application

Length of output: 693

---

ModelProvider type in workflowNodes.ts is incomplete and mismatches its own LMStudioModelConfig interface.

src/types/workflowNodes.ts (line 53) defines ModelProvider as "bedrock" | "ollama", but on line 72–79 it also defines LMStudioModelConfig with provider: "lmstudio". The union type ModelConfig (line 81) includes LMStudioModelConfig, yet the ModelProvider type doesn't include "lmstudio", creating a type safety violation. Code in NodeSettingsDrawer.tsx (line 629–630) already assigns "lmstudio" as a provider value, which should fail type checking against the incomplete ModelProvider definition. Add "lmstudio" to the ModelProvider union in workflowNodes.ts.

🤖 Prompt for AI Agents

In `@src/components/ModelSelector.tsx` around lines 1 - 9, The ModelProvider union
type is missing "lmstudio", causing a type mismatch with LMStudioModelConfig and
failing where NodeSettingsDrawer assigns provider = "lmstudio"; update the
ModelProvider definition in workflowNodes.ts to include "lmstudio" (so the union
matches LMStudioModelConfig), then ensure ModelConfig and any usages (e.g.,
NodeSettingsDrawer.tsx) type-check correctly against the updated ModelProvider.

[coderabbitai]

Actionable comments posted: 17

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (26)

convex/queueProcessor.ts (3)

346-371: ⚠️ Potential issue | 🔴 Critical

Bug: Abandoned-test retry path never removes the old queue entry — causes infinite requeue loop.

When cleanupAbandonedTests finds an abandoned entry with attempts < 3, it calls requeueTest (which inserts a new "pending" entry) but never deletes the original "claimed" entry. On the next cleanup run, the stale "claimed" entry is found again (same status, same claimedAt), and the cycle repeats indefinitely — creating duplicate queue entries on every cleanup pass.

Compare with the processQueue error path (lines 157–170), which correctly calls removeFromQueue before requeueTest.

🐛 Proposed fix: remove old entry before requeue

       if (queueEntry.attempts < 3) {
         // Retry
+        await ctx.runMutation(internal.queueProcessor.removeFromQueue, {
+          queueId: queueEntry._id,
+        });
         await ctx.runMutation(internal.queueProcessor.requeueTest, {
           testId: queueEntry.testId,
           priority: queueEntry.priority,
           attempts: queueEntry.attempts + 1,
           lastError: "Test abandoned - claimed but never started",
         });
       } else {

---

139-171: ⚠️ Potential issue | 🟠 Major

Retry path sets status to FAILED then immediately resets to QUEUED — confusing and racy.

Lines 143–149 mark the test as FAILED, then lines 162–170 requeue it (which patches status back to QUEUED inside requeueTest). Any UI polling or webhook that reads the intermediate FAILED status will show a false failure. Consider skipping the FAILED status update when retries remain, or using a distinct RETRYING status.

♻️ Proposed fix: skip FAILED status when retrying

          if ("error" in result) {
-           // Failed to execute test
-           console.error(`❌ Failed to execute test ${test._id}:`, result.error);
-
-           await ctx.runMutation(internal.testExecution.updateStatus, {
-             testId: test._id,
-             status: "FAILED",
-             success: false,
-             error: result.error,
-             errorStage: "execution",
-           });
-
-           await ctx.runMutation(internal.testExecution.appendLogs, {
-             testId: test._id,
-             logs: [`❌ Test execution failed: ${result.error}`],
-             timestamp: Date.now(),
-           });
-
-           // Remove from queue
-           await ctx.runMutation(internal.queueProcessor.removeFromQueue, {
-             queueId: queueEntry._id,
-           });
-
-           // Retry if attempts < 3
-           if (queueEntry.attempts < 3) {
+           console.error(`❌ Failed to execute test ${test._id}:`, result.error);
+
+           // Remove from queue
+           await ctx.runMutation(internal.queueProcessor.removeFromQueue, {
+             queueId: queueEntry._id,
+           });
+
+           // Retry if attempts < 3
+           if (queueEntry.attempts < 3) {
              console.log(`🔄 Retrying test ${test._id} (attempt ${queueEntry.attempts + 1}/3)`);
+             await ctx.runMutation(internal.testExecution.appendLogs, {
+               testId: test._id,
+               logs: [`⚠️ Attempt ${queueEntry.attempts + 1}/3 failed: ${result.error}. Retrying...`],
+               timestamp: Date.now(),
+             });
              await ctx.runMutation(internal.queueProcessor.requeueTest, {
                testId: test._id,
                priority: queueEntry.priority,
                attempts: queueEntry.attempts + 1,
                lastError: result.error ?? "",
              });
+           } else {
+             // Final failure after all retries exhausted
+             await ctx.runMutation(internal.testExecution.updateStatus, {
+               testId: test._id,
+               status: "FAILED",
+               success: false,
+               error: result.error,
+               errorStage: "execution",
+             });
+             await ctx.runMutation(internal.testExecution.appendLogs, {
+               testId: test._id,
+               logs: [`❌ Test execution failed after 3 attempts: ${result.error}`],
+               timestamp: Date.now(),
+             });
            }

---

182-190: ⚠️ Potential issue | 🟡 Minor

Catch block doesn't update test status or remove the queue entry.

If an unexpected exception is thrown (as opposed to a returned {error: ...}), the test stays in "claimed" status and the capacity slot is consumed until the 15-minute abandoned-test cleanup fires. Consider at minimum resetting the queue entry status to "pending" so the processor can retry sooner, or applying the same retry/failure logic used in the if ("error" in result) branch.

src/components/CognitoLogin.tsx (1)

235-239: ⚠️ Potential issue | 🟡 Minor

Default credentials displayed in the UI.

The info box renders a username and password (AgentBuilder2025!) in plain text. While this is pre-existing and may be intentional for a dev/demo environment, consider whether this should be removed or gated behind an environment flag before shipping to production.

src/SignInForm.tsx (1)

22-22: ⚠️ Potential issue | 🟠 Major

Form data (including password) logged to console.

Object.fromEntries(formData) will include the password field. This is a credential leak risk in browser dev tools and any log-forwarding setup. Remove or redact this before production.

Proposed fix

-          console.log("Form data being sent:", Object.fromEntries(formData));
+          console.log("Form data being sent:", { email: formData.get("email"), flow: formData.get("flow") });

convex/agentBuilderWorkflow.ts (1)

355-396: ⚠️ Potential issue | 🟡 Minor

streamWorkflowExecution uses Anthropic direct API instead of Bedrock.

This function imports @anthropic-ai/sdk and uses a hardcoded model claude-3-7-sonnet-20250219, while the rest of the file uses Bedrock. This means:

1. It requires a separate ANTHROPIC_API_KEY env var.
2. The model used for streaming differs from WORKFLOW_MODEL_ID.
3. Cost/routing goes through a different provider.

If this is intentional (e.g., Bedrock doesn't support streaming for this use case), consider adding a comment. Otherwise, align this with the Bedrock path.

convex/agentcoreSetup.ts (1)

124-128: ⚠️ Potential issue | 🟠 Major

Rate limiting is completely stubbed out.

The user tier is hardcoded to "freemium" and monthlyTests is always 0, making the rate-limit check on lines 139–144 a no-op. This means every user effectively has unlimited test executions regardless of tier. The same pattern repeats in getAgentCoreStatus (lines 237–240).

The PR objectives note this as an outstanding issue. Consider tracking it if not already.

Would you like me to open an issue to track implementing actual rate limiting with user tier lookup?

src/components/InterleavedChat.tsx (1)

264-265: ⚠️ Potential issue | 🟡 Minor

Escaped apostrophe renders literally in JSX.

you\'re in a JSX string will render as the literal text you\'re (with a visible backslash). JSX doesn't need escaped apostrophes — use you're directly or {you're}. Same issue on line 414.

Proposed fix

-              Use this space to scope your agent before automation runs. When you\'re ready, send the
+              Use this space to scope your agent before automation runs. When you're ready, send the

-                Talk through the agent you want to build. The workflow will take over when you\'re ready.
+                Talk through the agent you want to build. The workflow will take over when you're ready.

convex/authDebug.ts (1)

3-4: ⚠️ Potential issue | 🟡 Minor

Debug query exposes infrastructure details without authentication.

getOAuthConfig is a public query with no auth check. It reveals which OAuth providers are configured, deployment URLs, and callback URLs to any caller. Consider gating this behind authentication or restricting it to development environments.

src/components/AgentBuilder.tsx (3)

648-654: ⚠️ Potential issue | 🔴 Critical

Sparkles is not imported — this will throw a runtime error.

AutomationSummaryPanel references Sparkles (Lines 653 and 669) but it's not included in the lucide-react import on Lines 7–23.

Proposed fix

 import {
   Bot,
   Settings,
   Code,
   Rocket,
   ChevronRight,
   ChevronLeft,
   Trash2,
   Copy,
   Download,
   Upload,
   TestTube,
   Network,
   Shield,
   AlertCircle,
-  CheckCircle
+  CheckCircle,
+  Sparkles
 } from "lucide-react";

---

269-309: ⚠️ Potential issue | 🟠 Major

Stale closure: config.model is read but missing from the dependency array.

handleAutoGenerateFromPlan reads config.model (Line 272) and config.tools (Line 274), but the useCallback dependency array (Line 309) only includes config.tools, not config.model. If the user changes the model after the callback is memoized, the old model value will be used.

Proposed fix

-  }, [automationSummary, automationData, config.tools, generateAgent]);
+  }, [automationSummary, automationData, config.model, config.tools, generateAgent]);

---

374-385: ⚠️ Potential issue | 🟡 Minor

config.mcpConfig doesn't exist on AgentConfig — always evaluates to undefined.

Line 384 accesses config.mcpConfig?.servers, but the AgentConfig interface (Lines 43–53) has no mcpConfig property. This means mcpServers in the download payload will always be []. Either add mcpConfig to AgentConfig or remove this reference.

convex/deploymentRouter.ts (2)

196-210: ⚠️ Potential issue | 🟠 Major

incrementUsage is a public mutation with no authorization check.

Any authenticated (or unauthenticated) caller can invoke incrementUsage with an arbitrary userId to artificially exhaust another user's free-tier quota. This should either be an internal mutation or validate that the caller is the target user.

Proposed fix — make it internal

-import { mutation, query } from "./_generated/server";
+import { mutation, query, internalMutation } from "./_generated/server";
 ...
-export const incrementUsage = mutation({
+export const incrementUsage = internalMutation({
   args: {
     userId: v.id("users"),
   },

Then update the call site in deployTier1 to use ctx.runMutation(internal.deploymentRouter.incrementUsage, ...).

---

237-256: ⚠️ Potential issue | 🟠 Major

Authorization bypass when agentId is provided.

When args.agentId is set, the query switches to the by_agent index and drops the userId filter entirely (Line 246–248). This allows any authenticated user to view deployment history for any agent. The userId-based query should be intersected with the agentId filter, or ownership of the agent should be verified.

convex/mcpClient.ts (2)

556-556: ⚠️ Potential issue | 🟡 Minor

Inconsistent default model ID — not updated to Haiku 4.5.

The fallback model on Line 556 is "anthropic.claude-3-haiku-20240307-v1:0", but other files in this PR (guardrails, agentBuilderWorkflow, AgentBuilder) have been updated to use the Haiku 4.5 variant (us.anthropic.claude-haiku-4-5-20250514-v1:0). This should be aligned for consistency.

---

488-496: ⚠️ Potential issue | 🟡 Minor

Redundant dynamic import of api and fragile token-fetching pattern.

Line 489 dynamically imports api from "./_generated/api.js", but it's already imported at the top of the file (Line 5) and the local import is never used. The fetch call to CONVEX_SITE_URL on Line 493 to get a Cognito token is fragile — it won't work if the site URL isn't set and silently swallows errors. This entire function runs outside a Convex action context, so consider passing the token from the caller instead.

convex/guardrails.ts (1)

161-177: ⚠️ Potential issue | 🟠 Major

Update pricing constants to match Claude Haiku 4.5 rates.

The model was upgraded to Haiku 4.5 but the pricing constants were not updated. Current values match the older Haiku 3 rates:

• Input: $0.00025 per 1K tokens should be $0.001 (Haiku 4.5 is $1.00 per MTok)
• Output: $0.00125 per 1K tokens should be $0.005 (Haiku 4.5 is $5.00 per MTok)

This causes cost calculations to be ~4× too low. Additionally, update the comment from "as of 2024" to reflect the current 2025 model. Note: reasoning tokens do not have separate pricing; they are billed as output tokens per Anthropic's extended thinking documentation.

convex/awsDeployment.ts (3)

655-658: ⚠️ Potential issue | 🟡 Minor

config.enableMonitoring || true always evaluates to true.

The || operator with true on the right means monitoring can never be disabled. Same issue does not apply to Line 651 (enableAutoScaling || false) but is inconsistent. Use ?? instead.

Proposed fix

     monitoring: {
-      enabled: config.enableMonitoring || true,
+      enabled: config.enableMonitoring ?? true,
       logLevel: "INFO"
     }

---

198-281: ⚠️ Potential issue | 🟠 Major

updateDeploymentStatus is a public mutation with no authorization — any caller can modify any deployment.

This mutation updates status, logs, progress, and result of any deployment by ID without verifying that the caller owns the deployment. Compare with cancelDeployment (Line 408) which properly checks userId. addDeploymentLog (Line 286) has the same problem.

Proposed fix sketch

   handler: async (ctx, args) => {
+    const userId = await getAuthUserId(ctx);
+    if (!userId) throw new Error("Not authenticated");
+
     const deployment = await ctx.db.get(args.deploymentId);
-    if (!deployment) {
-      throw new Error("Deployment not found");
-    }
+    if (!deployment || deployment.userId !== userId) {
+      throw new Error("Deployment not found or not authorized");
+    }

---

49-51: ⚠️ Potential issue | 🔴 Critical

Authorization bypass for unauthenticated users deploying others' agents.

When userId is null (unauthenticated), the ownership check on Line 49 (if (userId && agent.createdBy !== userId)) is skipped entirely — an unauthenticated request can deploy any agent. The check later at Line 77 rejects anonymous users without credentials, but between Lines 49–76, anonymous users pass through unchecked on ownership.

Proposed fix

-    if (userId && agent.createdBy !== userId) {
+    if (!userId) {
+      throw new Error("Authentication required to deploy an agent");
+    }
+    if (agent.createdBy !== userId) {
       throw new Error("Not authorized to deploy this agent");
     }

convex/strandsAgentExecution.ts (2)

126-132: ⚠️ Potential issue | 🟠 Major

Non-null assertions on AWS credentials will produce a confusing runtime error.

Lines 129–130 use ! (non-null assertion) on process.env.AWS_ACCESS_KEY_ID and process.env.AWS_SECRET_ACCESS_KEY. If either is unset, undefined is silently passed to the SDK, causing a cryptic auth error deep in the AWS call. Compare with automatedAgentBuilder.ts (lines 289–291) which validates upfront with a clear error message.

🛡️ Add credential validation

+  const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+  if (!accessKeyId || !secretAccessKey) {
+    throw new Error("Missing AWS credentials: ensure AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY are set");
+  }
+
   const client = new BedrockRuntimeClient({
     region: process.env.AWS_REGION || "us-east-1",
     credentials: {
-      accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
-      secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
+      accessKeyId,
+      secretAccessKey,
     },
   });

---

134-146: ⚠️ Potential issue | 🟠 Major

History may contain "system" or "tool" role messages, which are invalid in Bedrock's messages array.

ConversationMessage.role includes "system" | "tool", but the Bedrock Converse/InvokeModel messages array only accepts "user" and "assistant". System content must go in the separate system field (already handled via agent.systemPrompt at line 161). If the conversation history includes system or tool messages, the API call will return a validation error.

🐛 Filter or remap non-user/assistant messages

   const messages: Array<{ role: string; content: Array<{ text: string }> }> = [];
 
   for (const msg of history) {
+    // Bedrock messages array only supports "user" and "assistant" roles
+    if (msg.role !== "user" && msg.role !== "assistant") continue;
     messages.push({
       role: msg.role,
       content: [{ text: msg.content }],
     });
   }

convex/containerOrchestrator.ts (1)

358-399: ⚠️ Potential issue | 🟠 Major

getTaskStatus is a public action with no authentication check.

Any caller can query ECS task status by taskArn. Unlike other endpoints in this file (which are internalAction), this is exposed publicly. An attacker with a valid-looking ARN could probe your ECS infrastructure.

🛡️ Add auth check or make internal

-export const getTaskStatus = action({
+export const getTaskStatus = internalAction({
   args: {
     taskArn: v.string(),
   },

If it must remain public, add authentication:

   handler: async (ctx, args) => {
+    // Verify authentication
+    const identity = await ctx.auth.getUserIdentity();
+    if (!identity) {
+      throw new Error("Authentication required");
+    }
     try {

convex/codeGenerator.ts (2)

29-29: ⚠️ Potential issue | 🟡 Minor

Inconsistent package name: strandsagents in META_TOOLING_INSTRUCTIONS vs strands_tools in imports.

Line 29 in META_TOOLING_INSTRUCTIONS tells the agent to "Use the @tool decorator from strandsagents", while Line 130 generates an import from strands_tools. If these are different packages, the instructions are misleading; if the package was renamed, the instructions need updating.

🔧 Proposed fix

-1. Use the `@tool` decorator from strandsagents
+1. Use the `@tool` decorator from strands_tools

Also applies to: 130-130

---

264-269: ⚠️ Potential issue | 🔴 Critical

Empty import generated when no tools are selected.

When toolImports is empty (size === 0), Lines 264-269 still emit from strands_tools import (\n), producing an empty Python import statement which is a syntax error. The else branch on Line 267-268 pushes only the closing ) without any import names.

🐛 Proposed fix

   if (toolImports.size > 0) {
     imports.push(...Array.from(toolImports));
     imports.push(")");
   } else {
-    imports.push(")");
+    // Remove the incomplete import statement
+    imports.pop(); // Remove "from strands_tools import ("
   }

convex/schema.ts (1)

126-141: ⚠️ Potential issue | 🔴 Critical

AWS credentials (access keys, secret keys, session tokens) stored in plaintext in the users table.

Lines 129-140 store awsAccessKeyId, awsSecretAccessKey, and a full awsCredentials object (with secretKey and sessionToken) directly in the users table. Anyone with database read access (or a query path that exposes user documents) can extract these credentials.

At minimum, encrypt these at rest using an application-level encryption layer before storage. Better yet, use a secrets manager (AWS Secrets Manager, Vault) and store only a reference. Also ensure no public query inadvertently returns these fields.

🤖 Fix all issues with AI agents

In @.github/workflows/setup-github-projects_Version3.yml:
- Around line 117-122: The gh project item-add call is using the nonexistent
--issue flag in the backfill while loop (variables: PROJECT_ID, issues_json,
issue_num, REPO) and in the issue event handler; replace the invalid --issue
"$REPO#$issue_num" usage with --url pointing to the canonical issue URL (e.g.
"https://github.com/$REPO/issues/$issue_num") so the command becomes gh project
item-add "$PROJECT_ID" --url "<issue-url>"; update the other occurrence that
handles single issue events likewise to construct and pass the correct issue URL
via --url.
- Around line 104-125: The pagination loop uses gh issue list with a
non-existent --page flag (see the while loop using page and the gh issue list
invocation), which will fail; remove the --page argument and either set a
sufficiently large --limit or replace the call with an API-based paginated query
(e.g., gh api --paginate) to retrieve all issues; update the loop that reads
issues_json, the count check, and the inner read loop (the variables
issues_json, count, and page) to work with the chosen pagination approach so the
failure/exit handling and failures incrementing remain correct.
- Around line 40-74: The workflow extracts the GraphQL node id but later
commands expect the numeric project number; update the jq extraction that sets
PROJECT_ID (the two places where PROJECT_ID=$(echo "$proj_list" | jq -r "[.[] |
select(.title == \"$PROJECT_NAME\") | .id] | first // empty") is used) to select
.number instead of .id so PROJECT_ID holds the numeric project number used by gh
project view and gh project item-add; ensure both occurrences are changed and
keep the variable name and subsequent checks unchanged.

In `@convex/agentcoreTestExecution.ts`:
- Around line 374-387: The fetch call uses user-controlled params.ollamaEndpoint
without validation or timeout, creating SSRF and hanging risks; fix by parsing
and allowlisting the endpoint before calling fetch (use URL to extract
hostname/port and only permit configured hosts/IPs/ranges such as
localhost/127.0.0.1 or an explicit allowlist), and enforce a request timeout via
AbortController (or a Promise.race with a timeout) passing the abort signal into
fetch; in practice update the code around the fetch call (reference
params.ollamaEndpoint and the fetch invocation) to validate the URL and throw on
disallowed hosts, then create an AbortController with a sensible timeout and
pass its signal to fetch so the request cannot hang indefinitely.

In `@convex/agentImprovement.ts`:
- Around line 48-56: generateUpdatedAgentConfig mutates and returns
updatedConfig.tools but the update call uses agent.tools, so tool additions are
discarded; change the update invocation that calls
ctx.runMutation(api.agents.update, {...}) to pass updatedConfig.tools (with a
null/undefined guard if needed) instead of agent.tools so the mutated tools are
persisted (refer to generateUpdatedAgentConfig, ctx.runMutation,
api.agents.update, updatedConfig.tools, agent.tools).

In `@convex/automatedAgentBuilder.ts`:
- Around line 305-307: The fallback modelId constant in automatedAgentBuilder.ts
is using an incorrect Claude Haiku 4.5 date stamp; update the default value
assigned to modelId (the variable declared as const modelId =
process.env.AGENT_BUILDER_MODEL_ID || "...") to the correct Bedrock inference
profile ID "us.anthropic.claude-haiku-4-5-20251001-v1:0" so runtime selection
falls back to the valid model string when AGENT_BUILDER_MODEL_ID is undefined.

In `@convex/containerOrchestrator.ts`:
- Around line 131-139: The code double-counts usage by calling
internal.testExecution.incrementUserUsage both when the ECS task starts (the
ctx.runQuery(...) + ctx.runMutation(...) block using args.testId at the top of
containerOrchestrator.ts) and again on log-detected completion; remove the
start-time increment: delete or disable the
ctx.runMutation(internal.testExecution.incrementUserUsage, { userId:
testDetails.userId, testId: args.testId, executionMethod: "fargate" }) call (and
its surrounding testDetails fetch if unused) and keep the single
incrementUserUsage invocation that runs when the "TEST COMPLETED SUCCESSFULLY"
marker is found so each successful execution is counted once.

In `@convex/lib/unifiedModalitySwitching.ts`:
- Around line 340-356: selectUnifiedModel currently throws when passed modality
"multimodal" because UNIFIED_MODEL_CATALOG lacks that key; update
selectUnifiedModel (and similarly getModelExecutionConfig) to handle modality
"multimodal" by mapping it to a safe fallback (e.g., treat "multimodal" as
"text" or pick the text-model entry from UNIFIED_MODEL_CATALOG) before looking
up models, and ensure detectModality callers that may pass "multimodal" are
supported; reference the selectUnifiedModel function, UNIFIED_MODEL_CATALOG
constant, detectModality, and getModelExecutionConfig and implement the fallback
mapping early in those functions so runtime throws are avoided.
- Around line 39-140: The UNIFIED_MODEL_CATALOG entry for text needs three
fixes: update the haiku modelId in the haiku tier from
"anthropic.claude-haiku-4-5-20250514-v1:0" to
"anthropic.claude-haiku-4-5-20251001-v1:0" (symbol:
UNIFIED_MODEL_CATALOG.text.haiku), remove the deprecated Claude 3 Opus tier
(symbol: UNIFIED_MODEL_CATALOG.text.opus) entirely, and correct pricing for
haiku and sonnet by including both input+output token costs (update
UNIFIED_MODEL_CATALOG.text.haiku.costPer1KTokensOrUnit to reflect $1/1M + $5/1M
→ 0.006 per 1K and UNIFIED_MODEL_CATALOG.text.sonnet.costPer1KTokensOrUnit to
reflect $3/1M + $15/1M → 0.018 per 1K).

In `@convex/localModelDetector.ts`:
- Around line 490-565: testLocalModel currently sends fetch requests to the
user-provided args.endpoint (used in the ollama/llamacpp/lmstudio branches)
which creates an SSRF risk; before any fetch calls (inside testLocalModel,
referencing args.endpoint), parse the endpoint with the URL constructor, resolve
its hostname to an IP (or use net.isIP when given directly), and validate it
against an allowlist or block private/internal ranges (RFC1918, link-local
169.254.*, loopback, metadata IPs) — reject the request early with a failure
message if the host/IP is disallowed; alternatively only permit explicit
localhost or configured trusted hostnames, and ensure the validation runs once
before the switch so none of the fetch calls to `${args.endpoint}/...` proceed
with unsafe endpoints.

In `@convex/multiAgentRuntime.ts`:
- Around line 22-168: The public action executeMultiAgentPattern lacks auth
checks, so add caller verification at the start of the handler: ensure ctx.auth
(or equivalent request identity) exists, validate the caller owns or is allowed
to operate on args.parentAgentId and optional parentConversationId (call
existing auth/permissions helpers or internal authorization actions), and reject
with an unauthorized error if checks fail; only proceed to call
internal.swarmTestingOrchestrator.createSwarmFromToolInvocation /
executeSwarmFromTool after permission is confirmed. Ensure error messages and
thrown errors are consistent (unauthorized vs forbidden) and that permission
checks reference executeMultiAgentPattern, args.parentAgentId,
args.parentConversationId, and args.agents.

In `@convex/promptChainExecutor.ts`:
- Around line 248-267: The invokeOllama function currently hardcodes a request
to 127.0.0.1:11434 which will be unreachable from Convex's cloud Node runtime;
change invokeOllama to use a configurable host (read from an environment
variable or Convex secret) or move Ollama calls to client-side code/edge runtime
so they target the user's machine or a reachable Ollama server, update the
function signature/usage in promptChainExecutor.ts accordingly (refer to
invokeOllama and any callers) and add robust error handling/logging when the
configured host is missing or the request fails.

In `@convex/swarmTestingOrchestrator.ts`:
- Around line 480-489: The code passes process.platform (the Convex server's OS)
into setupOllama which causes setup commands to use the server's platform
instead of the end user's; update the call that invokes
internal.localModelDetector.setupOllama to accept a real client-side platform
indicator (e.g., a platform field supplied from the client request or
environment) rather than process.platform, and modify the surrounding flow so
ctx.runAction(internal.localModelDetector.setupOllama, ...) receives that
client-provided platform value (or remove platform entirely if setup should
always assume a specific target), ensuring any platform-dependent logic in
internal.localModelDetector.setupOllama uses the client-supplied platform rather
than process.platform.

In `@convex/testExecution.ts`:
- Around line 204-213: The testsThisMonth counter is being incremented twice for
cloud tests: once in submitTest (uses isOllamaModel check and ctx.db.patch) and
again inside incrementUserUsage; remove the duplicate increment from
incrementUserUsage so only submitTest updates testsThisMonth (or alternatively
move the single increment to incrementUserUsage if you prefer accurate
completion-based counting). Locate incrementUserUsage and delete or disable the
block that patches testsThisMonth/testsThisMonth increment (the ctx.db.patch
that updates testsThisMonth and lastTestAt) to prevent double-counting while
keeping the submitTest increment intact.

In `@convex/unifiedAgentExecution.ts`:
- Around line 325-341: Replace the synchronous InvokeModelCommand use with the
Nova Reel async pattern: call StartAsyncInvoke (using modelId and your JSON
payload plus an outputDataConfig that points to the S3 output location) via
client.send to receive an invocationArn, then poll GetAsyncInvoke with that
invocationArn until the job reaches a terminal state and read the S3 output;
remove the direct JSON.parse(new TextDecoder().decode(response.body)) handling
and any expectation of inline video content from responseBody. Ensure
StartAsyncInvoke/ GetAsyncInvoke calls pass the same modelId and input payload
and that you construct outputDataConfig correctly so the produced video is
stored to S3.

In `@convex/unifiedUserIdentity.ts`:
- Around line 33-43: The current getUnifiedUserId flow allows an authenticated
caller to pass any deviceId and trigger mergeAnonymousUser, enabling account
takeover; to fix it, change the logic in getUnifiedUserId to verify ownership
before calling mergeAnonymousUser: require that the authenticated user's record
(query "users" for authUserId) already contains the same deviceId or validate a
one-time merge token/challenge tied to that device/session, and only then
perform the withIndex("by_device_id") lookup and call
mergeAnonymousUser(anonymousId, authUserId); ensure checks reference the
deviceId field and authUserId and reject the merge if no proven association
exists.

In `@src/components/NodeSettingsDrawer.tsx`:
- Around line 124-128: updateConfig currently closes over the render-time draft
which causes lost updates when multiple calls occur; change updateConfig to use
the functional form of setDraft so each call receives the latest state (e.g.,
call setDraft(prev => { if (!prev) return prev; const nextConfig =
mutator(prev.config as T); return { ...prev, config: nextConfig } as
WorkflowNodeData; })), keeping the same mutator signature and preserving types
for draft, WorkflowNodeData and config.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

process.platform returns the Convex server's platform, not the user's.

Line 487 passes process.platform to setupOllama, but this code runs in Convex's cloud runtime. The value will be the server's OS (likely Linux), not the end user's platform. This compounds the broader localModelDetector design issue — setup commands for the wrong platform will be executed on the wrong host.

🤖 Prompt for AI Agents

In `@convex/swarmTestingOrchestrator.ts` around lines 480 - 489, The code passes
process.platform (the Convex server's OS) into setupOllama which causes setup
commands to use the server's platform instead of the end user's; update the call
that invokes internal.localModelDetector.setupOllama to accept a real
client-side platform indicator (e.g., a platform field supplied from the client
request or environment) rather than process.platform, and modify the surrounding
flow so ctx.runAction(internal.localModelDetector.setupOllama, ...) receives
that client-provided platform value (or remove platform entirely if setup should
always assume a specific target), ensuring any platform-dependent logic in
internal.localModelDetector.setupOllama uses the client-supplied platform rather
than process.platform.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@convex/agentcoreTestExecution.ts`:
- Around line 453-461: Guard against response.Payload being undefined before
calling TextDecoder.decode/JSON.parse in the agent execution flow: in the block
that checks response.FunctionError and the subsequent parsing of result, first
check if response.Payload is present (e.g., if (!response.Payload) return a
clear error object or use a safe default string like '{}' or
'{"errorMessage":"no payload"}' before decoding), then decode and JSON.parse the
payload; update references around response.FunctionError,
TextDecoder().decode(response.Payload), JSON.parse(...), errorPayload and result
so parsing never receives undefined and you return a descriptive failure when
payload is missing.

🧹 Nitpick comments (7)

convex/agentcoreTestExecution.ts (6)

29-45: Agent null check occurs after updateStatus call — acceptable but worth noting.

The test status is set to RUNNING (Line 37) before the agent null check (Line 43). If the agent doesn't exist, the catch block will then mark it FAILED. This works but performs a wasted mutation. Consider moving the null check before the status update if you want to avoid the unnecessary write.

---

196-271: Hardcoded model map duplicates modelRegistry — divergence risk.

Lines 202–269 define a large fallback mapping that overlaps with ALL_MODELS imported from modelRegistry.ts (Line 191). Any model additions or ID corrections must now be kept in sync in two places. Consider deriving the short-name → Bedrock-ID mapping from the registry instead.

---

200-200: Fragile provider-prefix guard clause.

This long chain of !modelId.startsWith(...) checks will silently break if a new provider prefix is introduced. Consider inverting the logic: check whether modelId looks like a fully-qualified ARN/model-ID (e.g., contains . or :) rather than enumerating every known prefix.

---

303-309: setTimeout in Promise.race is never cleared, leaking the timer on success.

When client.send(command) resolves first, the 60-second reject timer continues to run (and will fire a no-op rejection on a settled promise). Use AbortController or clear the timer on resolution.

Proposed fix

-    const response = await Promise.race([
-      client.send(command),
-      new Promise<never>((_, reject) =>
-        setTimeout(() => reject(new Error("Bedrock timeout")), 60000)
-      ),
-    ]);
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), 60000);
+    try {
+      const response = await client.send(command, { abortSignal: controller.signal });
+      clearTimeout(timeoutId);

---

446-451: Same setTimeout timer leak as the Bedrock path.

The 30-second reject timer is never cleared on successful resolution. Same fix applies — use AbortSignal or clear the timer.

---

183-188: Non-null assertions on environment variables will produce opaque SDK errors if unset.

process.env.AWS_ACCESS_KEY_ID! and AWS_SECRET_ACCESS_KEY! will be undefined at runtime if the env vars are missing, but the ! suppresses the TS warning. The resulting SDK error will be confusing. Consider an early guard with a clear message.

requirements.txt (1)

118-119: Consider documenting if strands-utcp is also private.

The strands-utcp package appears to be part of the internal Strands ecosystem (similar naming pattern to strands-agents and strands-agents-tools). If it's also private/internal, consider adding a comment similar to lines 4-9 to maintain documentation consistency and prevent build failures in public environments.

[coderabbitai]

Actionable comments posted: 17

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

convex/awsDeployment.ts (3)

655-662: ⚠️ Potential issue | 🟡 Minor

enableMonitoring: config.enableMonitoring || true is always true.

The || operator returns the first truthy value, so false || true evaluates to true. This means monitoring can never be disabled. Same issue on line 656 with enableAutoScaling. Use ?? (nullish coalescing) instead.

Proposed fix

     monitoring: {
-      enabled: config.enableMonitoring || true,
+      enabled: config.enableMonitoring ?? true,
       logLevel: "INFO"
     }

Also check scaling.enabled on line 655:

-      enabled: config.enableAutoScaling || false,
+      enabled: config.enableAutoScaling ?? false,

---

698-702: ⚠️ Potential issue | 🟡 Minor

Fragile userId type coercion based on string prefix.

args.userId.startsWith('j') is a brittle heuristic for detecting Convex Id<"users">. If the ID format changes or a string userId happens to start with 'j', this silently misclassifies. Consider using a more robust approach or typing the argument more strictly.

---

1544-1558: ⚠️ Potential issue | 🟠 Major

Empty subnets and securityGroups arrays will cause ECS service creation to fail.

The CreateServiceCommand is called with empty arrays for both subnets and securityGroups (lines 1554–1555). Fargate requires at least one subnet to launch tasks. The TODO comments indicate this is known but unresolved — this code path will always error at runtime.

Do you want me to generate the VPC/subnet discovery logic using DescribeVpcsCommand and DescribeSubnetsCommand (already imported on line 1421)?

🤖 Fix all issues with AI agents

In `@convex/agentAsToolGenerator.ts`:
- Around line 44-118: The template uses safeAgentName, safeDescription, and
safeAgentId but still injects raw agentName/description/agentId in many places;
update the template string so every interpolation uses the sanitized variables
(safeAgentName, safeDescription, safeAgentId) instead of
agentName/description/agentId (e.g., in the `@tool` description, docstring lines,
JSON payload "agentId", error messages, and return strings). Also ensure
toolName is sanitized for inclusion in Python string contexts (apply the same
sanitize function or an appropriate string-escaping step to the toolName
identifier before embedding it in the template) so all dynamic values in the
template come from escaped/safe variables.

In `@convex/agentcoreTestExecution.ts`:
- Around line 47-62: The rate limits are duplicated and inconsistent: replace
the hardcoded limits object in agentcoreTestExecution.ts ({ freemium: 10,
personal: 100, enterprise: -1 }) and the constants in testExecution.ts
(FREE_TESTS_PER_MONTH, PERSONAL_TESTS_PER_MONTH) with a single shared export
(e.g., FREE_TESTS_PER_MONTH, PERSONAL_TESTS_PER_MONTH,
ENTERPRISE_TESTS_PER_MONTH or a shared LIMITS map) in a new shared module, then
import and use those shared symbols in both agentcoreTestExecution.ts (where the
limits object is used) and testExecution.ts so both code paths enforce the same
values. Ensure the enterprise value remains -1 for unlimited and update
references such as the limits lookup and any uses of FREE_TESTS_PER_MONTH /
PERSONAL_TESTS_PER_MONTH to the shared names.

In `@convex/agentImprovement.ts`:
- Around line 17-73: The public actions applyImprovementPlan, autoImproveAgent,
and previewImprovements lack authentication/authorization and therefore allow
any caller to read conversations and modify any agent; add the same auth guard
used in submitTest/executeMultiAgentPattern: ensure ctx.auth (or the
authenticated user id) is present, fetch the agent via api.agents.get and verify
the agent.ownerId (or equivalent) matches the caller (or that the caller has
admin rights), and throw an Unauthorized/Error if not allowed before proceeding
(apply this check at the start of applyImprovementPlan, autoImproveAgent, and
previewImprovements and mirror the pattern used in
submitTest/executeMultiAgentPattern).

In `@convex/conversationAnalysis.ts`:
- Around line 132-151: The "Detect misunderstandings" block is using substring
checks like userContent.includes("no") which causes many false positives; update
the detection in that block (the logic that pushes into
analysis.identifiedIssues and analysis.userCorrections) to use
word-boundary/whole-word matching (e.g., a case-insensitive regex like /\bno\b/i
or tokenize userContent and check words/phrases) and apply the same whole-word
approach for the other trigger phrases (e.g., "that's not what i meant", "try
again") to avoid matching inside other words; keep the rest of the handling
(pushing to analysis.identifiedIssues and analysis.userCorrections) unchanged.

In `@convex/interleavedReasoning.ts`:
- Around line 168-209: getUserMessageCount currently reads the deprecated
embedded conversation.messages array (conversation.messages || []) which is no
longer populated; instead query the interleavedMessages table for each
conversation and count messages whose timestamp >= since. In the
getUserMessageCount handler, after resolving conversations from
interleavedConversations (or the single conversation via conversationToken),
replace the inner loop over conversation.messages with a db query against
"interleavedMessages" (e.g.,
ctx.db.query("interleavedMessages").withIndex(...).collect() or a count-style
query) filtering by conversationId (or conversationToken if that’s the key) and
timestamp >= since, then sum those counts into count. Ensure you remove reliance
on the deprecated conversation.messages field when computing the final count.
- Around line 79-85: The exported sendMessage is annotated with ": any", which
erases Convex action type inference; remove the ": any" annotation on the export
(i.e., change "export const sendMessage: any = action(...)" to "export const
sendMessage = action(...)") so TypeScript can infer correct argument/return
types from the action call; also, after removing the top-level any, review the
handler signature (handler: async (ctx: any, args: any) => ...) and tighten or
remove the any types if needed so callers get full type safety from the inferred
action type.
- Line 282: The default Bedrock model ID string assigned to the const modelId is
outdated and must be updated; locate the declaration of modelId (const modelId =
process.env.AGENT_BUILDER_MODEL_ID ||
"us.anthropic.claude-haiku-4-5-20250514-v1:0") and replace the hardcoded default
with the corrected Bedrock model ID that uses the 20251001 stamp (e.g.,
"us.anthropic.claude-haiku-4-5-20251001-v1:0") so it matches the fix applied
elsewhere (automatedAgentBuilder's model ID).
- Around line 289-292: Wrap the Bedrock response decoding/parsing around a
try-catch similar to convex/automatedAgentBuilder.ts: after awaiting
client.send(command) check response.statusCode or any error fields, decode the
body via new TextDecoder().decode(response.body) into a string, then attempt
JSON.parse inside the try block; on failure (malformed JSON or error status)
log/throw a descriptive error including the raw body and response metadata so
callers can debug. Update the code around the symbols response, responseBody,
client.send, and command to return/propagate a clear error when parsing fails
rather than letting JSON.parse throw unhandled.

In `@convex/ollamaMCPIntegration.ts`:
- Around line 22-50: Public actions testAgentWithOllama, listOllamaModels,
checkOllamaStatus, and chatWithOllama expose Ollama operations without
authentication; change their exposure to server-only or add identity checks.
Either replace action({...}) with internalAction({...}) for testAgentWithOllama,
listOllamaModels, checkOllamaStatus, and chatWithOllama so they cannot be
invoked by clients, or at the top of each handler add an explicit auth guard
(e.g., verify ctx.auth or ctx.session / call an internal user lookup) that
throws when the caller is unauthenticated or unauthorized before calling
callOllamaMCP; update exports accordingly and keep the same handler logic names
(testAgentWithOllama, listOllamaModels, checkOllamaStatus, chatWithOllama).
- Around line 162-294: The executeOllamaTest action currently exported as a
public action can be invoked by any client and mutates
internal.testExecution.updateStatus and internal.testExecution.appendLogs;
change its declaration from action({...}) to internalAction({...}) (update the
exported symbol executeOllamaTest accordingly) so only server-side callers can
invoke it, keep the same handler and arg schema, and update any callers to
import/use the internalAction version; ensure references to
internal.testExecution.updateStatus and internal.testExecution.appendLogs remain
unchanged inside the handler.

In `@convex/promptChainExecutor.ts`:
- Around line 215-223: The current creation of a RegExp from user-supplied
extractor in the extraction logic (the block that constructs new
RegExp(extractor) — part of the extractValue/extractOutput flow) is vulnerable
to ReDoS; fix it by validating or restricting the user pattern before
constructing the RegExp: either (1) use a safe-regex check (e.g., integrate a
safe-regex library or algorithm) and reject or sanitize patterns that are
unsafe, (2) restrict extractors to a small allowed set of precompiled patterns
and map extractOutput values to those, or (3) treat extractor as a literal
string (escape it) if freeform regexes aren’t required, or run the regex
evaluation in an isolated worker with a short timeout to prevent event-loop
blocking — implement one of these defenses and ensure the code returns the full
response or a clear error when a user-supplied pattern is rejected.
- Around line 183-193: The renderTemplate function builds a RegExp from
user-controlled keys which can cause ReDoS or incorrect behavior when keys
contain regex metacharacters; update renderTemplate to either escape regex
metacharacters in the constructed placeholder (e.g., add an escapeRegExp utility
and use it when building the RegExp for placeholder) or avoid RegExp entirely by
using String.prototype.replaceAll with the literal placeholder string; ensure
you reference renderTemplate, the variables object, and the placeholder variable
when making the change so replacements are safe for keys like "input(1)" or
"data[0]".

In `@convex/rateLimiter.ts`:
- Around line 167-188: The code is using identity?.subjectId which doesn't exist
on Convex's UserIdentity; update the user ID extraction in the wrapper returned
by the outer function (the code that calls ctx.auth.getUserIdentity()) to use
identity.subject or identity.tokenIdentifier instead of subjectId, and pass that
string into checkRateLimit (i.e., replace occurrences of identity?.subjectId
with identity?.subject || identity?.tokenIdentifier and ensure you cast to
String when calling checkRateLimit and keep the existing authentication error
behavior).

In `@convex/schema.ts`:
- Around line 493-515: The auditLogs table stores raw IP addresses in
metadata.ipAddress which poses GDPR/CCPA risk; update the schema and surrounding
logic so ipAddress is either removed from persisted metadata or stored only in
hashed/anonymized form, and implement a short retention/rotation policy plus
data-subject deletion support: modify the auditLogs.defineTable metadata schema
(metadata -> ipAddress) to reflect hashedIp or remove ipAddress, update any
functions that write to auditLogs (where auditLogs is inserted/updated) to
hash/anonymize the IP before persisting and ensure there is a retention job to
purge raw/old IPs and a deletion pathway that respects data subject requests.
- Around line 133-147: The schema currently defines awsAccessKeyId,
awsSecretAccessKey and awsCredentials as optional plain-text fields (see
awsAccessKeyId, awsSecretAccessKey, awsCredentials, awsCredentialsUpdatedAt,
awsIdentityId) which exposes secrets; change the schema to stop storing raw
secrets by replacing these fields with a secret reference (e.g., awsSecretRef:
v.optional(v.string())) or a boolean/metadata flag and a timestamp (e.g.,
awsCredentialsUpdatedAt) and update any creation/update paths to instead write
secrets to a proper secrets manager (or encrypt with KMS) and persist only the
returned secret ID/reference and expiry metadata; ensure any code that reads
awsCredentials is updated to fetch the secret from the secrets manager using the
stored reference.

In `@convex/strandsAgentExecutionDynamic.ts`:
- Around line 79-132: The public action executeAgentWithDynamicModel lacks an
authentication/authorization check, so add verification at the start of its
handler to ensure the caller is authenticated and authorized to run the agent:
require ctx.auth (or equivalent request identity) and confirm the caller has
permission to execute the agent (for example, compare ctx.auth.userId or role
against agent.createdBy or an ACL) before proceeding; if the caller is
unauthenticated or unauthorized, return a failure result (success: false) and do
not call executeWithModelSwitching or executeDirectBedrock. Also ensure any new
auth failure paths are logged and that permission checks cover both owner and
any allowed roles (admin/service) used elsewhere in the codebase.

In `@convex/workflows.ts`:
- Around line 8-25: getUserScope currently returns arbitrary auth fields and
must be aligned with getAuthUserId() so workflows created by workflowTemplates
match ownership checks; replace the fallback chain in getUserScope with a call
to getAuthUserId from `@convex-dev/auth/server` (import it and return its value as
the resolved scope) so the returned string matches workflows.userId set by
workflowTemplates and fixes the comparisons against userScope in the workflow
handlers (references: getUserScope, getAuthUserId, workflows.userId, and the
ownership checks that compare workflow.userId !== userScope).

🟡 Minor comments (12)

convex/promptChainExecutor.ts-281-312 (1)

281-312: ⚠️ Potential issue | 🟡 Minor

Non-null assertions on AWS credentials will throw unhelpful errors.

Lines 287–288 use process.env.AWS_ACCESS_KEY_ID! and process.env.AWS_SECRET_ACCESS_KEY!. If these environment variables are unset, this produces a runtime error deep in the AWS SDK call rather than a clear message. Add an early check.

Proposed fix

 async function invokeBedrock(model: string, prompt: string): Promise<string> {
   const { BedrockRuntimeClient, InvokeModelCommand } = await import("@aws-sdk/client-bedrock-runtime");
 
+  const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+  if (!accessKeyId || !secretAccessKey) {
+    throw new Error("AWS credentials not configured: set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY");
+  }
+
   const client = new BedrockRuntimeClient({
     region: process.env.AWS_REGION || "us-east-1",
     credentials: {
-      accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
-      secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
+      accessKeyId,
+      secretAccessKey,
     },
   });

convex/localModelDetector.ts-514-563 (1)

514-563: ⚠️ Potential issue | 🟡 Minor

Wrap case declarations in blocks to fix linting errors.

Static analysis correctly flags that const declarations in case clauses without blocks can leak across cases. Wrap each case body in { }.

Proposed fix (showing pattern for one case)

       switch (args.provider) {
-        case 'ollama':
-          const ollamaResponse = await fetch(`${args.endpoint}/api/generate`, {
+        case 'ollama': {
+          const ollamaResponse = await fetch(`${args.endpoint}/api/generate`, {
             ...
           });
           if (ollamaResponse.ok) { ... }
           break;
+        }
 
-        case 'llamacpp':
-          const llamaResponse = await fetch(...);
+        case 'llamacpp': {
+          const llamaResponse = await fetch(...);
           ...
           break;
+        }

Apply the same pattern to the lmstudio case and also to the startLocalModel switch (lines 347–405).

package.json-45-47 (1)

45-47: ⚠️ Potential issue | 🟡 Minor

Upgrade reactflow to @xyflow/react and remove unused stripe dependency.

reactflow v11 is the legacy package line; React Flow v12+ uses @xyflow/react. Since the codebase actively imports and uses reactflow in VisualScriptingBuilder.tsx and VisualWorkflowEditor.tsx, upgrade to the current package name.

stripe is a server-side Node.js SDK and is not imported anywhere in the codebase. It should be removed from the top-level package.json. If a Convex backend function needs Stripe integration, the SDK should be declared in a dedicated backend dependency location, not in a shared root package.json that bundles both frontend and backend code.

convex/swarmTestingOrchestrator.ts-965-971 (1)

965-971: ⚠️ Potential issue | 🟡 Minor

Isolation verification via substring matching is unreliable and trivially bypassed.

Checking result.content?.includes("cross-agent") etc. to verify isolation is not a real isolation check — it merely scans the LLM's text output for specific phrases. An LLM could reference "other agents" in a perfectly valid response, causing a false negative, or it could violate isolation without using those words.

This should either be removed or clearly documented as a placeholder with a TODO.

convex/unifiedAgentExecution.ts-175-181 (1)

175-181: ⚠️ Potential issue | 🟡 Minor

Inconsistent AWS credential/bucket validation across handlers.

executeVideo (lines 316-322) properly validates AWS_S3_BUCKET and credentials before proceeding. But executeText (line 178-179), executeImage (line 254-256), and uploadImagesToS3 (line 502) use non-null assertions (!) without any guard, which will throw cryptic errors at runtime if the env vars are missing.

Consider adding consistent validation

+function requireAwsCredentials(): { accessKeyId: string; secretAccessKey: string } {
+  const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+  if (!accessKeyId || !secretAccessKey) {
+    throw new Error("AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY are required");
+  }
+  return { accessKeyId, secretAccessKey };
+}
+
+function requireS3Bucket(): string {
+  const bucket = process.env.AWS_S3_BUCKET;
+  if (!bucket) throw new Error("AWS_S3_BUCKET is required");
+  return bucket;
+}

Also applies to: 252-258, 496-502

convex/multiAgentRuntime.ts-15-21 (1)

15-21: ⚠️ Potential issue | 🟡 Minor

Contradictory comments about schema table availability.

Line 19 states "Multi-agent sessions and results tables are not yet defined in schema" while line 208 says "NOW ENABLED with schema tables." The mutations at lines 210-285 insert into multiAgentSessions, interleavedConversations, and multiAgentResults, so these tables presumably exist now. Update the stale comment at line 19.

Also applies to: 208-208

convex/agentcoreTestExecution.ts-276-284 (1)

276-284: ⚠️ Potential issue | 🟡 Minor

Conversation history messages with non-user/assistant roles are silently coerced to "user".

Line 280 maps all non-"assistant" roles (including "system" and "tool") to "user". This differs from strandsAgentExecutionDynamic.ts (lines 205-213) which correctly skips these roles. System messages injected as user turns will confuse the model.

Proposed fix

     if (params.conversationHistory) {
       for (const msg of params.conversationHistory.slice(-5)) {
+        if (msg.role !== "user" && msg.role !== "assistant") continue;
         messages.push({
-          role: msg.role === "assistant" ? "assistant" : "user",
+          role: msg.role,
           content: [{ text: msg.content }],
         });
       }
     }

convex/agentcoreTestExecution.ts-36-62 (1)

36-62: ⚠️ Potential issue | 🟡 Minor

Agent null check occurs after test status is set to RUNNING — leaves test in a broken state.

Line 37 updates the test status to "RUNNING", but the agent null check is on line 43. If the agent is null, the test is already marked as RUNNING and then throws, which hits the outer catch and sets it to FAILED. While this ultimately resolves, reordering the checks avoids the unnecessary status flip:

Proposed fix

     // Get agent
     const agent = await ctx.runQuery(internal.agents.getInternal, { id: args.agentId });
+    if (!agent) {
+      throw new Error("Agent not found");
+    }

     // Update status to running
     await ctx.runMutation(internal.testExecution.updateStatus, {
       testId: args.testId,
       status: "RUNNING",
     });

-    // CHECK LIMITS: Only when actually executing
-    if (!agent) {
-      throw new Error("Agent not found");
-    }
+    // CHECK LIMITS: Only when actually executing

convex/testExecution.ts-878-906 (1)

878-906: ⚠️ Potential issue | 🟡 Minor

Double-count fix is in place, but calculateBedrockCost is called with a non-matching model ID.

The testsThisMonth double-increment issue from the past review is resolved — incrementUserUsage no longer increments the counter (lines 882-883 comment confirms this). Good.

However, line 904 calls calculateBedrockCost(args.usage, "claude-3-5-sonnet"). The string "claude-3-5-sonnet" doesn't match any key in the pricing map (line 31-35), so it silently falls back to pricing["anthropic.claude-3-5-sonnet-20241022-v2:0"]. This works by accident but is misleading and will produce incorrect costs if the actual model is Haiku or Titan.

Consider passing the actual model ID from the agent or from args:

Proposed fix

   handler: async (ctx, args) => {
+    const test = await ctx.db.get(args.testId);
     const user = await ctx.db.get(args.userId);
     if (!user) return;
     ...
-        estimatedCost: args.usage ? calculateBedrockCost(args.usage, "claude-3-5-sonnet") : 0,
+        estimatedCost: args.usage && test
+          ? calculateBedrockCost(args.usage, test.modelConfig?.modelId || "anthropic.claude-3-5-sonnet-20241022-v2:0")
+          : 0,

convex/testExecution.ts-22-22 (1)

22-22: ⚠️ Potential issue | 🟡 Minor

Env var read at module scope may not reflect dashboard changes after deployment.

process.env.MAX_CONCURRENT_TESTS is read at module load time (line 22). In Convex, module-scope code runs once during isolate initialization, and the value is cached across invocations. If this env var is changed in the Convex dashboard after deployment, the cached value won't reflect those changes until the code is redeployed. This is standard Node.js behavior and may be intentional for a compile-time constant, but worth noting if dynamic reconfiguration is needed.

convex/rateLimiter.ts-82-158 (1)

82-158: ⚠️ Potential issue | 🟡 Minor

TOCTOU window in checkRateLimit: concurrent requests can bypass the limit.

The function reads the entry via ctx.runQuery (line 97), then writes via ctx.runMutation (lines 124/143). Between the read and write, another concurrent action can also read the same count and both pass. This is inherent to the action-based pattern (actions don't run in a transaction).

For a rate limiter, slight over-admission is typically acceptable, but be aware that under burst load, the effective limit can be ~2× the configured maxRequests. If stricter enforcement is needed, move the check-and-update logic into a single mutation.

convex/tools.ts-384-401 (1)

384-401: ⚠️ Potential issue | 🟡 Minor

sortedAnswers[0] will throw if numPaths is 0 or all answers are empty.

If a caller passes numPaths: 0, the answers array will be empty, and sortedAnswers[0][0] on line 391 will throw a TypeError: Cannot read properties of undefined. Guard against this:

Proposed fix

   const sortedAnswers = Object.entries(voteCounts).sort((a, b) => b[1] - a[1]);
-  const finalAnswer = sortedAnswers[0][0];
-  const confidence = sortedAnswers[0][1] / numPaths;
+  const finalAnswer = sortedAnswers.length > 0 ? sortedAnswers[0][0] : "No answer generated";
+  const confidence = sortedAnswers.length > 0 && numPaths > 0 ? sortedAnswers[0][1] / numPaths : 0;

🧹 Nitpick comments (23)

convex/modelBenchmarks.ts (2)

505-521: New Convex queries are straightforward — consider documenting the sentinel values for consumers.

The queries themselves are correct. However, getAllBenchmarks returns entries with valueScore: -1 and efficiencyScore: -1 for Ollama models. Frontend consumers may naively display or sort on these fields without realizing -1 means "not applicable." A brief note in the JSDoc or a response transformation (e.g., mapping -1 to null) would make the API more self-describing.

---

479-499: maxBudget parameter is unused.

The maxBudget parameter (line 482) is never referenced in the function body. It appears to have been intended to cap maxCost but was never wired in.

Proposed fix — either remove or wire it in

Option A — remove:

 export function recommendModelForComplexity(
   complexity: number,
   taskType: "code" | "reasoning" | "knowledge" | "general" = "general",
-  maxBudget: number = 300
 ): ModelBenchmarks {

Option B — use it to cap the cost lookup:

   const maxCost = complexity < 30 ? 0.5 :
                   complexity < 50 ? 2.0 :
                   complexity < 70 ? 5.0 :
                   15.0;
+  const effectiveCost = Math.min(maxCost, maxBudget);

-  const optimal = findOptimalModel(taskType, maxCost, requiredAbility);
+  const optimal = findOptimalModel(taskType, effectiveCost, requiredAbility);

package.json (1)

20-33: Inconsistent AWS SDK version pins may cause dependency conflicts.

Several AWS SDK packages are pinned at ^3.701.0 (e.g., client-cloudwatch-logs, client-ecs) while others are at ^3.911.0–^3.917.0. AWS SDK v3 packages share core transitive dependencies, and version skew can lead to multiple copies of @smithy/* packages and subtle runtime incompatibilities.

Consider aligning all @aws-sdk/* packages to the same minor range (e.g., ^3.917.0).

requirements.txt (1)

103-108: Both Flask and FastAPI are listed as dependencies — likely redundant.

The file lists both fastapi>=0.109.0 (line 64) and flask>=3.1.2 (line 106). Running two web frameworks simultaneously is unusual and increases the dependency surface. If Flask is only needed transitively, document why; otherwise, consolidate on one framework.

convex/localModelDetector.ts (1)

14-19: Duplicate imports: top-level ESM imports and require() calls inside functions.

execSync, spawn, fs, path, and os are imported at the top level (lines 17–19) but are also re-imported via require() inside detectLlamaCpp (line 135), detectLMStudio (line 169), detectGGUF (lines 203–204), setupOllama (lines 258–259), and startLocalModel (line 345). Remove the redundant require() calls and use the top-level imports throughout.

convex/lib/roles.ts (1)

3-4: MODERATOR_ROLES is identical to ADMIN_ROLES — intentional?

Both arrays contain only [UserRole.ADMIN]. If there's meant to be a separate moderator role (e.g., UserRole.PAID), this should be updated. If intentional, consider adding a brief comment explaining why.

convex/workflowExecutor.ts (2)

664-681: Expression evaluator is fragile and could produce surprising results.

expression.includes("success") matches any expression containing that substring (e.g., "check_unsuccessful" would match). The split(/===?/) approach for equality checks is similarly brittle. The TODO acknowledges this, but as this is wired into live workflow execution, an expression like "unsuccessfulRetry == true" would silently return the wrong result.

Consider at minimum matching with word boundaries or using a simple tokenizer/parser.

---

527-552: uploadVideoToS3 is unused dead code in convex/unifiedAgentExecution.ts.

The uploadVideoToS3 function at lines 527–552 in convex/unifiedAgentExecution.ts is never called anywhere in the codebase and should be removed.

convex/unifiedAgentExecution.ts (1)

527-552: Remove unused uploadVideoToS3 function. The executeVideo function was refactored to use StartAsyncInvokeCommand, which writes output directly to S3, making this helper obsolete.

convex/workflows.ts (3)

413-413: let model is never reassigned — use const.

Static analysis (ESLint prefer-const) correctly flags this.

Proposed fix

-  let model = config.modelId || config.model || "anthropic.claude-3-5-sonnet-20241022-v2:0";
+  const model = config.modelId || config.model || "anthropic.claude-3-5-sonnet-20241022-v2:0";

---

467-498: forEach callback implicitly returns a value from Set.add().

Biome flags that pipPackages.add(pkg) returns the Set, making the .forEach callback return a value. Use a block body to suppress the warning:

Proposed fix

-      (metadata?.additionalPipPackages || []).forEach((pkg) => pipPackages.add(pkg));
+      (metadata?.additionalPipPackages || []).forEach((pkg) => { pipPackages.add(pkg); });

---

343-358: as any cast on api.agents.create bypasses compile-time validation.

If the agents.create mutation args schema changes, this call will silently break at runtime. Consider aligning the argument shape with the actual mutation signature or updating the mutation to accept the new fields.

convex/multiAgentRuntime.ts (1)

34-34: handler: async (ctx: any, args: any) discards all type safety from the args validator.

Convex infers handler argument types from the args schema. The explicit any annotations suppress this, allowing bugs to slip through silently.

Proposed fix

-  handler: async (ctx: any, args: any): Promise<any> => {
+  handler: async (ctx, args) => {

convex/agentImprovement.ts (1)

86-139: Lexical declarations in case blocks can leak across clauses.

Static analysis (Biome/ESLint) correctly flags const toolMatch (line 90) and const modelMatch (line 122) as lexical declarations inside case blocks without braces. These declarations are hoisted to the enclosing switch scope and can be accessed (as undefined) from other clauses.

Proposed fix — wrap each case in a block

       case "add_tool":
+      {
         // Extract tool name from description
         const toolMatch = improvement.description.match(/Add capability: (.+)/i);
         ...
         break;
+      }

       ...

       case "change_model":
+      {
         // Extract recommended model
         const modelMatch = improvement.implementation.match(/(claude-[^\s]+)/i);
         ...
         break;
+      }

convex/strandsAgentExecutionDynamic.ts (1)

195-201: Non-null assertions on AWS credentials will throw unhelpful errors if env vars are missing.

process.env.AWS_ACCESS_KEY_ID! and process.env.AWS_SECRET_ACCESS_KEY! (lines 198-199) will be undefined at runtime if the env vars aren't set, producing cryptic SDK errors downstream rather than a clear failure message.

Proposed fix

+  const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+  if (!accessKeyId || !secretAccessKey) {
+    return { success: false, error: "AWS credentials not configured" };
+  }
+
   const client = new BedrockRuntimeClient({
     region: process.env.AWS_REGION || "us-east-1",
     credentials: {
-      accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
-      secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
+      accessKeyId,
+      secretAccessKey,
     },
   });

convex/agentcoreTestExecution.ts (2)

375-385: SSRF validation is in place — good improvement over the previous version.

The endpoint allowlist (line 376) and URL parsing correctly prevent requests to arbitrary hosts. However, "0.0.0.0" in the allowlist is worth reconsidering — while it binds to all interfaces locally, allowing it as a target host could have unintended behavior on some platforms where it routes to the host's network stack.

Consider removing 0.0.0.0

-    const allowedHosts = ["localhost", "127.0.0.1", "0.0.0.0", "::1"];
+    const allowedHosts = ["localhost", "127.0.0.1", "::1"];

---

388-406: var response should be let — and the scoping pattern can be improved.

ESLint correctly flags the var on line 391. The var is used to hoist the variable out of the try block, but this can be avoided with let declared before the try:

Proposed fix

     const controller = new AbortController();
     const timeoutId = setTimeout(() => controller.abort(), 30000);
+    let response: Response;
     try {
-      var response = await fetch(`${params.ollamaEndpoint}/v1/chat/completions`, {
+      response = await fetch(`${params.ollamaEndpoint}/v1/chat/completions`, {
         method: "POST",
         ...
       });
     } finally {
       clearTimeout(timeoutId);
     }

convex/automatedAgentBuilder.ts (2)

188-207: v.any() for agentRequirements and conversationHistory bypasses Convex validation.

The updateBuildSession internal mutation uses v.any() for both agentRequirements and conversationHistory. Since the schema defines agentRequirements with a precise v.object(...) shape (see convex/schema.ts lines 522–532), any shape mismatch will only surface as a runtime DB error rather than being caught at the mutation boundary.

Consider using the same validator or at least v.object({...}) to get early validation.

---

389-454: generateAgentFromSession reads unvalidated fields from session.generatedAgentConfig.

config at line 408 is typed as any (from v.any() in schema). Properties like config.name, config.model, config.systemPrompt are accessed without null checks or validation. If the AI produces a config missing any of these, the downstream api.codeGenerator.generateAgent or api.agents.create calls will fail with an unhelpful error.

A lightweight guard would improve debuggability:

if (!config.name || !config.model || !config.systemPrompt) {
  throw new Error("Generated agent config is incomplete: missing name, model, or systemPrompt");
}

convex/rateLimiter.ts (1)

353-369: getRateLimitStats collects the entire rateLimitEntries table unbounded.

ctx.db.query("rateLimitEntries").collect() loads every row into memory. As usage grows, this will hit Convex query limits or degrade performance. Consider paginating or adding a .take(limit) with a warning, or computing stats incrementally.

convex/tools.ts (2)

106-127: handoffToUser generates IDs using deprecated substr and non-cryptographic Math.random().

Line 115 uses Math.random().toString(36).substr(2, 9). substr is deprecated — use substring(2, 11) instead. Also, Math.random() is not cryptographically secure; for a handoff ID that might be used for access control, consider crypto.randomUUID().

Proposed fix

-    const handoffId = `handoff_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
+    const handoffId = `handoff_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`;

---

353-401: selfConsistency runs LLM calls sequentially — consider parallelizing.

The for-loop at line 366 invokes invokeLLM sequentially for each path. Since the paths are independent, they could be parallelized (similar to how mapReduce uses Promise.all with concurrency batching). This would significantly reduce latency for the default 3 paths.

convex/conversationAnalysis.ts (1)

317-335: Unused variable total on line 322.

const total = analysis.conversationMetrics.totalMessages is declared but never used.

Proposed fix

-  const total = analysis.conversationMetrics.totalMessages;
   const issues = analysis.identifiedIssues.length;
   const successes = analysis.successfulInteractions.length;

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Sanitization is applied inconsistently — raw values still used in most of the generated Python code.

Lines 44–48 correctly create safeAgentName, safeDescription, and safeAgentId, but they're only used in the docstring (lines 50–51). The rest of the template continues to use the raw unsanitized values:

• Line 61: description="${description || ...}" — uses raw description
• Line 65: ${agentName} — uses raw agentName
• Line 77, 83, 87, 97, 114, 117: same issue with agentName and agentId

Additionally, toolName is never sanitized for Python string contexts (it's only regex-cleaned for identifier use but could still break if embedded in a string).

Proposed fix — use sanitized values throughout the template

 `@tool`(
-    name="${toolName}",
-    description="${description || `Invoke ${agentName} agent to handle specialized tasks`}",
+    name="${toolName}",
+    description="${safeDescription || `Invoke ${safeAgentName} agent to handle specialized tasks`}",
     parameters={
         "task": {
             "type": "string",
-            "description": "The task or question to send to ${agentName}",
+            "description": "The task or question to send to ${safeAgentName}",
             "required": True
         },
...
-                "agentId": "${agentId}",
+                "agentId": "${safeAgentId}",

Apply the same pattern to all remaining raw interpolations of agentName, description, and agentId in the template.

🤖 Prompt for AI Agents

In `@convex/agentAsToolGenerator.ts` around lines 44 - 118, The template uses
safeAgentName, safeDescription, and safeAgentId but still injects raw
agentName/description/agentId in many places; update the template string so
every interpolation uses the sanitized variables (safeAgentName,
safeDescription, safeAgentId) instead of agentName/description/agentId (e.g., in
the `@tool` description, docstring lines, JSON payload "agentId", error messages,
and return strings). Also ensure toolName is sanitized for inclusion in Python
string contexts (apply the same sanitize function or an appropriate
string-escaping step to the toolName identifier before embedding it in the
template) so all dynamic values in the template come from escaped/safe
variables.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Public actions applyImprovementPlan, autoImproveAgent, and previewImprovements have no authentication.

These actions modify agent configuration and access conversation data but never verify the caller's identity. Any unauthenticated client can invoke them to alter any agent's system prompt, tools, and model. Add auth checks similar to those in submitTest or executeMultiAgentPattern.

Also applies to: 195-233, 238-275

🤖 Prompt for AI Agents

In `@convex/agentImprovement.ts` around lines 17 - 73, The public actions
applyImprovementPlan, autoImproveAgent, and previewImprovements lack
authentication/authorization and therefore allow any caller to read
conversations and modify any agent; add the same auth guard used in
submitTest/executeMultiAgentPattern: ensure ctx.auth (or the authenticated user
id) is present, fetch the agent via api.agents.get and verify the agent.ownerId
(or equivalent) matches the caller (or that the caller has admin rights), and
throw an Unauthorized/Error if not allowed before proceeding (apply this check
at the start of applyImprovementPlan, autoImproveAgent, and previewImprovements
and mirror the pattern used in submitTest/executeMultiAgentPattern).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Misunderstanding detection has extremely high false-positive rate.

userContent.includes("no") (line 133) matches any message containing the substring "no" — e.g., "I know", "another", "not sure yet, nothing wrong". This will flag normal positive interactions as misunderstandings. At minimum, use word-boundary matching:

Proposed fix

-      if (userContent.includes("no") || userContent.includes("that's not what i meant") || userContent.includes("try again")) {
+      if (/\bno\b/.test(userContent) || userContent.includes("that's not what i meant") || userContent.includes("try again") || userContent.includes("wrong")) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Detect misunderstandings
	if (userContent.includes("no") || userContent.includes("that's not what i meant") || userContent.includes("try again")) {
	analysis.identifiedIssues.push({
	type: 'misunderstanding',
	message: "User indicated misunderstanding",
	userMessage: msg.content,
	agentResponse: nextMsg.content,
	severity: 'medium',
	timestamp: msg.timestamp,
	});
	// This is likely a correction
	if (i > 0) {
	analysis.userCorrections.push({
	originalMessage: messages[i - 2]?.content || "",
	correctionMessage: msg.content,
	timestamp: msg.timestamp,
	});
	}
	}
	// Detect misunderstandings
	if (/\bno\b/.test(userContent) || userContent.includes("that's not what i meant") || userContent.includes("try again") || userContent.includes("wrong")) {
	analysis.identifiedIssues.push({
	type: 'misunderstanding',
	message: "User indicated misunderstanding",
	userMessage: msg.content,
	agentResponse: nextMsg.content,
	severity: 'medium',
	timestamp: msg.timestamp,
	});
	// This is likely a correction
	if (i > 0) {
	analysis.userCorrections.push({
	originalMessage: messages[i - 2]?.content || "",
	correctionMessage: msg.content,
	timestamp: msg.timestamp,
	});
	}
	}

🤖 Prompt for AI Agents

In `@convex/conversationAnalysis.ts` around lines 132 - 151, The "Detect
misunderstandings" block is using substring checks like
userContent.includes("no") which causes many false positives; update the
detection in that block (the logic that pushes into analysis.identifiedIssues
and analysis.userCorrections) to use word-boundary/whole-word matching (e.g., a
case-insensitive regex like /\bno\b/i or tokenize userContent and check
words/phrases) and apply the same whole-word approach for the other trigger
phrases (e.g., "that's not what i meant", "try again") to avoid matching inside
other words; keep the rest of the handling (pushing to analysis.identifiedIssues
and analysis.userCorrections) unchanged.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

sendMessage typed as any destroys caller-side type safety.

export const sendMessage: any = action(...) erases the inferred Convex action type, so callers (UI, other actions) get no type checking on arguments or return values. Remove the : any annotation to let TypeScript infer the proper type.

Proposed fix

-export const sendMessage: any = action( {
+export const sendMessage = action( {

🤖 Prompt for AI Agents

In `@convex/interleavedReasoning.ts` around lines 79 - 85, The exported
sendMessage is annotated with ": any", which erases Convex action type
inference; remove the ": any" annotation on the export (i.e., change "export
const sendMessage: any = action(...)" to "export const sendMessage =
action(...)") so TypeScript can infer correct argument/return types from the
action call; also, after removing the top-level any, review the handler
signature (handler: async (ctx: any, args: any) => ...) and tighten or remove
the any types if needed so callers get full type safety from the inferred action
type.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Storing AWS secret keys in plaintext in the database is a critical security risk.

Lines 135–136 store awsAccessKeyId and awsSecretAccessKey directly in the users table. If the database is compromised (or accessed by any developer/admin with DB access), all users' AWS credentials are exposed. AWS secret keys should be stored in a secrets manager (e.g., AWS Secrets Manager, Vault) and referenced by ID, or encrypted at rest with a KMS key before storage.

The same concern applies to awsCredentials (lines 141–146), though temporary session tokens are less severe since they expire.

🤖 Prompt for AI Agents

In `@convex/schema.ts` around lines 133 - 147, The schema currently defines
awsAccessKeyId, awsSecretAccessKey and awsCredentials as optional plain-text
fields (see awsAccessKeyId, awsSecretAccessKey, awsCredentials,
awsCredentialsUpdatedAt, awsIdentityId) which exposes secrets; change the schema
to stop storing raw secrets by replacing these fields with a secret reference
(e.g., awsSecretRef: v.optional(v.string())) or a boolean/metadata flag and a
timestamp (e.g., awsCredentialsUpdatedAt) and update any creation/update paths
to instead write secrets to a proper secrets manager (or encrypt with KMS) and
persist only the returned secret ID/reference and expiry metadata; ensure any
code that reads awsCredentials is updated to fetch the secret from the secrets
manager using the stored reference.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

ipAddress in audit log metadata may create GDPR/CCPA compliance risk.

Line 507 stores ipAddress in audit logs. IP addresses are considered personal data under GDPR. Ensure there's a retention policy and data subject access/deletion mechanism, or consider hashing/anonymizing the IP after a short retention window.

🤖 Prompt for AI Agents

In `@convex/schema.ts` around lines 493 - 515, The auditLogs table stores raw IP
addresses in metadata.ipAddress which poses GDPR/CCPA risk; update the schema
and surrounding logic so ipAddress is either removed from persisted metadata or
stored only in hashed/anonymized form, and implement a short retention/rotation
policy plus data-subject deletion support: modify the auditLogs.defineTable
metadata schema (metadata -> ipAddress) to reflect hashedIp or remove ipAddress,
update any functions that write to auditLogs (where auditLogs is
inserted/updated) to hash/anonymize the IP before persisting and ensure there is
a retention job to purge raw/old IPs and a deletion pathway that respects data
subject requests.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Public action executeAgentWithDynamicModel has no authentication check.

This action executes agent workflows against Bedrock (incurring AWS costs) but doesn't verify caller identity. An unauthenticated user could invoke this action repeatedly, causing resource and cost abuse.

🤖 Prompt for AI Agents

In `@convex/strandsAgentExecutionDynamic.ts` around lines 79 - 132, The public
action executeAgentWithDynamicModel lacks an authentication/authorization check,
so add verification at the start of its handler to ensure the caller is
authenticated and authorized to run the agent: require ctx.auth (or equivalent
request identity) and confirm the caller has permission to execute the agent
(for example, compare ctx.auth.userId or role against agent.createdBy or an ACL)
before proceeding; if the caller is unauthenticated or unauthorized, return a
failure result (success: false) and do not call executeWithModelSwitching or
executeDirectBedrock. Also ensure any new auth failure paths are logged and that
permission checks cover both owner and any allowed roles (admin/service) used
elsewhere in the codebase.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how userId is stored in workflows and whether other modules use the same identity resolution
rg -n "getUserScope|identity\.subject|identity\.tokenIdentifier" --type=ts -C2

Repository: MikePfunk28/agent_builder_application

Length of output: 2187

---

🏁 Script executed:

#!/bin/bash
# Find the workflows table schema definition
rg -n "workflows|userId" --type=ts | grep -E "(schema|table|userId)" -A2 -B2

Repository: MikePfunk28/agent_builder_application

Length of output: 43032

---

🏁 Script executed:

#!/bin/bash
# Search for identity resolution patterns across the codebase
rg -n "auth\.getUserIdentity|identity\.subject|identity\.email" --type=ts | head -30

Repository: MikePfunk28/agent_builder_application

Length of output: 1008

---

🏁 Script executed:

#!/bin/bash
# Find getAuthUserId function definition and see how it resolves identity
rg -n "function getAuthUserId|const getAuthUserId|export.*getAuthUserId" --type=ts -A10

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

#!/bin/bash
# Check resolveUserId in tools.ts to see how it differs
rg -n "function resolveUserId|const resolveUserId" --type=ts -A15

Repository: MikePfunk28/agent_builder_application

Length of output: 956

---

🏁 Script executed:

#!/bin/bash
# Search for getAuthUserId imports and definition
rg -n "import.*getAuthUserId|from.*getAuthUserId" --type=ts

Repository: MikePfunk28/agent_builder_application

Length of output: 2841

---

🏁 Script executed:

#!/antml:bash
# Search for getAuthUserId definition more broadly
rg -n "getAuthUserId" --type=ts | grep -E "export|function|const" | head -20

Repository: MikePfunk28/agent_builder_application

Length of output: 1516

---

🏁 Script executed:

#!/antml/bash
# Check the workflowTemplates.ts to see how it inserts into workflows table
rg -n "workflows" convex/workflowTemplates.ts -B2 -A2 | grep -E "insert|userId|scope"

Repository: MikePfunk28/agent_builder_application

Length of output: 161

---

🏁 Script executed:

#!/bash
# Get full context of workflowTemplates.ts around line 313 where it inserts workflow
cat -n convex/workflowTemplates.ts | sed -n '310,320p'

Repository: MikePfunk28/agent_builder_application

Length of output: 383

---

🏁 Script executed:

#!/bin/bash
# Verify if workflows created by workflowTemplates are accessed by workflows.ts handlers
rg -n "updateStatusInternal|getInternal" convex/workflows.ts -B5 -A5 | head -40

Repository: MikePfunk28/agent_builder_application

Length of output: 1354

---

Fix identity resolution inconsistency in getUserScope — workflows created via workflowTemplates.ts are inaccessible via workflows handlers.

The function uses a custom fallback chain (identity.subject || identity.tokenIdentifier || identity.email || identity.provider) that returns auth identity strings. However, workflowTemplates.ts (line 315) populates the same workflows.userId field using getAuthUserId() from @convex-dev/auth/server, which returns Convex user document IDs (e.g., v.id("users")).

Since workflows handlers compare workflow.userId !== userScope (lines 201, 291, 325), workflows created by templates will fail ownership validation and become inaccessible to the user. Align identity resolution across all modules: either use getAuthUserId() consistently or update workflowTemplates.ts to use getUserScope().

🤖 Prompt for AI Agents

In `@convex/workflows.ts` around lines 8 - 25, getUserScope currently returns
arbitrary auth fields and must be aligned with getAuthUserId() so workflows
created by workflowTemplates match ownership checks; replace the fallback chain
in getUserScope with a call to getAuthUserId from `@convex-dev/auth/server`
(import it and return its value as the resolved scope) so the returned string
matches workflows.userId set by workflowTemplates and fixes the comparisons
against userScope in the workflow handlers (references: getUserScope,
getAuthUserId, workflows.userId, and the ownership checks that compare
workflow.userId !== userScope).

[coderabbitai]

Actionable comments posted: 16

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

convex/apiKeys.ts (2)

260-267: ⚠️ Potential issue | 🔴 Critical

Math.random() is not cryptographically secure — API keys will be predictable.

Math.random() uses a PRNG that is not suitable for generating security tokens. An attacker who can observe a few keys (e.g., via the returned keyPrefix) could potentially predict future keys.

Use crypto.getRandomValues() (available in the same runtime where crypto.subtle.digest is used on line 276):

Proposed fix

 function generateSecureToken(length: number): string {
   const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
-  let result = '';
-  for (let i = 0; i < length; i++) {
-    result += chars.charAt(Math.floor(Math.random() * chars.length));
-  }
-  return result;
+  const values = new Uint32Array(length);
+  crypto.getRandomValues(values);
+  return Array.from(values, (v) => chars[v % chars.length]).join('');
 }

---

272-278: ⚠️ Potential issue | 🔴 Critical

Hardcoded 'default-secret' fallback for HMAC key defeats the purpose of keyed hashing.

If JWT_PRIVATE_KEY is unset, all API key hashes use the publicly visible string 'default-secret' as the salt, making them trivially reproducible by anyone who reads this source code. This should either throw an error when the env var is missing, or use a dedicated env var for API key hashing (not JWT_PRIVATE_KEY).

Proposed fix

 async function hashAPIKey(apiKey: string): Promise<string> {
   const encoder = new TextEncoder();
-  const secret = process.env.JWT_PRIVATE_KEY || 'default-secret';
+  const secret = process.env.API_KEY_HASH_SECRET;
+  if (!secret) {
+    throw new Error("API_KEY_HASH_SECRET environment variable is required");
+  }
   const data = encoder.encode(apiKey + secret);

convex/agentcoreSetup.ts (2)

124-140: ⚠️ Potential issue | 🟠 Major

Rate limiting is completely inert — user and usage are hardcoded stubs.

user is hardcoded to { tier: "freemium" } (line 124) and monthlyTests is hardcoded to 0 (line 128), so the rate limit check on line 135 (0 >= limit) will never trigger. The centralized tier config integration on lines 131–133 is fetching real limits, but comparing them against dummy data.

Either wire this up to actual user data (similar to how testExecution.ts lines 87–103 fetch the real user), or clearly mark this entire block as a no-op to avoid giving a false sense of security:

-      const user: any = { tier: "freemium" }; // Simplified for now
-      const tier: string = user?.tier || "freemium";
-      const monthlyTests: any = 0; // Simplified for now
+      // TODO: Fetch real user and monthly usage to enforce rate limits
+      // Rate limiting is currently NOT enforced in this code path
+      const tier = "freemium";
+      const monthlyTests = 0;

---

233-250: ⚠️ Potential issue | 🟠 Major

Same inert rate-limiting pattern in getAgentCoreStatus.

Lines 233 and 236 have the same hardcoded stubs as executeAgentTest. The testLimit and remainingTests values returned to the caller will always show 0 used / full capacity remaining regardless of actual usage.

🤖 Fix all issues with AI agents

In `@convex/agentcoreTestExecution.ts`:
- Line 271: The fallback model ID hardcoded in the modelId assignment (the
expression using modelMap, params.modelId, and
process.env.AGENT_BUILDER_MODEL_ID) has the wrong date stamp; update the date
portion of that hardcoded Anthropic model string from 20250514 to 20251001 so it
matches the corrected fallback used in strandsAgentExecution.ts (the same
modelId fallback should be used across both files).

In `@convex/apiKeys.ts`:
- Around line 247-254: Replace the inline CommonJS require call with a static ES
module import: add a top-level "import { getTierConfig } from
'./lib/tierConfig'" and remove the inline "const { getTierConfig } =
require(...)" so the code that calls getTierConfig(tier) and builds the config
object (testsPerMonth, maxConcurrentTests, maxAgents) uses the imported
function; keep the same logic for monthlyExecutions === -1 mapping to Infinity
and ensure no other references to require remain for tierConfig.

In `@convex/lib/dynamicModelSwitching.ts`:
- Around line 34-37: The haiku model entry in dynamicModelSwitching.ts sets
modelId to "us.anthropic.claude-haiku-4-5-20250514-v1:0" which has the wrong
date stamp; update the haiku object's modelId property to use the correct date
per AWS Bedrock (replace 20250514 with 20251001), mirroring the fix applied in
agentcoreTestExecution.ts so the symbol haiku and its modelId are corrected.

In `@convex/lib/tierConfig.ts`:
- Around line 207-217: The mapping in getRoleForTier currently grants
UserRole.ADMIN to "enterprise" tiers which elevates subscription customers to
system admin (see getRoleForTier and the reciprocal getTierForRole); either add
a concise comment above getRoleForTier explaining the explicit business
rationale and scope for granting ADMIN to enterprise customers (including
references to sensitive gated features like auditLogs.ts), or change the mapping
to a narrower, explicit role (e.g., ENTERPRISE_ADMIN or a scoped role) instead
of UserRole.ADMIN and update any consumers (such as audit log checks) to use the
scoped role; ensure the chosen approach is documented in a short inline comment
near getRoleForTier and that getTierForRole remains consistent.

In `@convex/schema.ts`:
- Around line 274-290: The conversations table currently embeds an unbounded
messages array (field messages on conversations) which can hit Convex's document
size limit; instead normalize messages into a separate table using the existing
interleavedMessages pattern (or create a new conversationMessages table) where
each message is its own document that references conversationId, role, content,
timestamp and metadata, and add indexes on conversationId and timestamp (and
optionally userId/agentId) to preserve ordering and query performance;
alternatively, if keeping an embedded array, enforce a maxMessages cap and
implement an archival/truncation strategy (e.g., trim oldest messages to a fixed
limit and move them to a conversationArchive collection) so the
conversations.defineTable(messages) field no longer grows unbounded.
- Around line 654-664: The requests array in the rateLimitEntries table (symbol:
rateLimitEntries, field: requests) is unbounded and can exceed document size;
replace it with a bounded structure (e.g., fixed-size ring buffer or time-bucket
map) and update the validation/type to enforce that bound (e.g., a map from
bucketStart->count or an array with a max length) instead of
v.array(v.number()). Update any code that writes to or trims requests
(rate-limit update/cleanup functions) to roll timestamps into the chosen buckets
and to trim/rotate entries on every write so the stored document size never
grows unbounded.
- Line 433: The env field currently uses v.optional(v.object({})), which rejects
any non-empty environment maps; change the validation for env (the field where
v.object({}) is used in convex/schema.ts) to use v.optional(v.any()) (matching
the agents table's env usage) so arbitrary environment objects like { "PATH":
"/usr/bin" } are accepted; update the schema entry that defines env accordingly.
- Around line 728-762: The workflows.userId and workflowExecutions.userId fields
currently use v.string() and should be switched to Convex foreign-key types to
restore referential integrity: change workflows definition's userId from
v.string() to v.id("users") and change workflowExecutions definition's userId
from v.string() to v.id("users"); keep existing indexes (by_user) and other
field shapes the same and do not change toolMemory (which intentionally uses
v.string()).

In `@convex/strandsAgentExecution.ts`:
- Around line 88-106: The Bedrock detection logic in the agent gating block (the
isBedrock variable using agent.deploymentType and agent.model) is inverted and
skips tier checks for many Bedrock models; change isBedrock to first honor
agent.deploymentType (treat deploymentType === "bedrock" as Bedrock) and only
when deploymentType is absent/null use the same heuristic used in
extractModelConfig (check model ID prefixes/patterns like "anthropic.",
"amazon.", etc., or other Bedrock-specific patterns) to decide Bedrock; keep the
subsequent calls to isProviderAllowedForTier and isBedrockModelAllowedForTier
unchanged so tier gating runs for true Bedrock cases.

In `@convex/strandsAgentExecutionDynamic.ts`:
- Around line 114-121: The current isBedrock check wrongly treats models with
colons as non-Bedrock; update the logic to positively detect Ollama models
instead of relying on absence of ":" so Bedrock IDs with colons can't bypass the
tier gate. Replace the isBedrock assignment with a check that sets isBedrock =
agent.deploymentType !== "ollama" && !isOllamaModel(agent.model) (or inline
equivalent), where isOllamaModel is a small deterministic check that returns
true for explicit Ollama indicators (e.g., agent.deploymentType === "ollama" OR
model strings that contain "ollama" or match known Ollama patterns/prefixes),
then continue to call isProviderAllowedForTier(userTier, "bedrock") as before.
- Around line 248-261: The current payload variable is built in Anthropic
Messages API format regardless of model, causing failures for non-Anthropic
Bedrock models (e.g., llama3.2:latest); update the construction in the same
block that references payload and isAnthropicModel to branch by provider: keep
the existing Anthropic shape (system, messages, max_tokens, thinking,
anthropic_version) when isAnthropicModel is true, and when false create a
Bedrock-compatible payload for non-Anthropic models (use the
Bedrock/InvokeModelCommand schema: proper top-level fields such as model
identifier and prompt/input structure, correct token/timeout fields and any
model-specific options for Llama/Mistral), or alternatively remove non-Anthropic
entries from MODEL_TIERS; ensure the code path that calls InvokeModelCommand
consumes the correct schema for the chosen model.

In `@convex/stripe.ts`:
- Around line 94-99: The current logic derives frontendUrl from
process.env.CONVEX_SITE_URL and process.env.SITE_URL which causes any
CONVEX_SITE_URL containing ".convex." to force frontendUrl to
"http://localhost:4000"; update the code to read a dedicated FRONTEND_URL env
var instead: replace usage of frontendUrl (and its computation based on siteUrl)
with process.env.FRONTEND_URL ?? process.env.SITE_URL ??
process.env.CONVEX_SITE_URL ?? "http://localhost:4000", and ensure Stripe
redirect URLs use this new FRONTEND_URL value; also add a note to set
FRONTEND_URL in Convex env vars to your real production frontend origin.
- Around line 140-143: The frontend URL derivation is duplicated and incorrectly
forces localhost when the site URL contains ".convex."; extract a single helper
(e.g., getFrontendUrl) that computes FRONTEND_URL by reading
process.env.FRONTEND_URL or falling back to process.env.CONVEX_SITE_URL /
process.env.SITE_URL with correct logic (do not unconditionally replace
".convex." hosts with "http://localhost:4000"); replace the duplicated
siteUrl/frontendUrl logic in createCheckoutSession and the block that defines
frontendUrl (symbols: createCheckoutSession, siteUrl, frontendUrl) to call the
new helper so both locations share the same, correct URL resolution.
- Around line 158-178: Change reportUsage from a public action to an
internal-only action: replace action(...) with internalAction(...) for the
exported symbol reportUsage, ensure internalAction is imported from your
framework (where action was imported), and keep the existing args validation and
handler logic (including the call to getStripeClient() and
(stripe.subscriptionItems as any).createUsageRecord). This ensures reportUsage
cannot be invoked by clients and remains callable internally after cloud
executions.
- Around line 166-176: The code currently calls the removed createUsageRecord on
(stripe.subscriptionItems as any) — replace this with the Stripe Billing Meter
Events API: call stripe.billing.meterEvents.create(...) instead of
createUsageRecord and remove the unsafe any cast; populate event_name with your
meter's event name, set payload to include stripe_customer_id (use the customer
id available in your scope) and value as args.quantity, and set timestamp to
Math.floor(Date.now()/1000); update any surrounding logic that relied on
subscriptionItemId to use meter event semantics and ensure the meter/event_name
and payload keys match your Stripe meter configuration.

In `@convex/testExecution.ts`:
- Around line 81-115: The submitTest flow currently skips enforcing per-tier
allowed provider rules: after computing isOllamaModel in submitTest, call
isProviderAllowedForTier(userTier, agent.deploymentType || /* derive provider
from agent.model similar to strandsAgentExecution.ts logic */) (the
isProviderAllowedForTier import is already available) and if it returns false
throw an Error with a helpful message (use getTierConfig(userTier).displayName
and mention allowedProviders) before performing the anonymous/user-tier rate
checks; mirror the placement and behavior used in strandsAgentExecution.ts and
strandsAgentExecutionDynamic.ts so freemium users cannot submit Bedrock tests
even if under their execution count.

🟡 Minor comments (8)

src/components/ModelSelector.tsx-73-87 (1)

73-87: ⚠️ Potential issue | 🟡 Minor

Potential crash if model.description is undefined for dynamically detected models.

Line 83 calls model.description.toLowerCase() without optional chaining. While tags on line 84 is safely guarded with ?., description is accessed directly. If any dynamically detected local model lacks a description field, this will throw at search time.

🛡️ Proposed fix

       return (
         model.label.toLowerCase().includes(lowerSearch) ||
         model.id.toLowerCase().includes(lowerSearch) ||
-        model.description.toLowerCase().includes(lowerSearch) ||
+        (model.description ?? "").toLowerCase().includes(lowerSearch) ||
         model.tags?.some((tag) => tag.toLowerCase().includes(lowerSearch))
       );

convex/http.ts-440-447 (1)

440-447: ⚠️ Potential issue | 🟡 Minor

Avoid leaking signature-verification details to the caller.

Line 446 echoes err.message back in the HTTP response. Stripe verification errors can contain internal details (e.g., expected vs. received signatures). Return a generic message instead and keep the specifics server-side only.

🛡️ Proposed fix

     } catch (err: any) {
       console.error(`Stripe webhook signature verification failed: ${err.message}`);
-      return new Response(`Webhook signature error: ${err.message}`, { status: 400 });
+      return new Response("Webhook signature verification failed", { status: 400 });
     }

convex/agentcoreTestExecution.ts-111-119 (1)

111-119: ⚠️ Potential issue | 🟡 Minor

Usage tracked on success, but incrementUserUsage is called even for Ollama executions.

executeAgentCoreTest calls incrementUserUsage (line 113) on any successful result, regardless of whether it was an Ollama or Bedrock execution. Since Ollama is free and submitTest doesn't increment testsThisMonth for Ollama, this is fine for the counter. However, the audit log on line 890 of testExecution.ts calls calculateBedrockCost regardless of execution method, which will log misleading cost data for Ollama tests (showing Bedrock pricing for a free local execution).

convex/testExecution.ts-28-40 (1)

28-40: ⚠️ Potential issue | 🟡 Minor

calculateBedrockCost pricing map is incomplete and the fallback is misleading.

The pricing map on lines 29–33 only covers 3 specific model IDs. On line 35, unknown models silently fall back to Sonnet pricing. On line 890, the function is called with "claude-3-5-sonnet" (a short name) which will never match any key in the map, so every invocation uses the fallback.

Either pass the actual modelId from the test execution (available via the agent or result metadata), or expand the map to cover more models. The current implementation gives a false sense of per-model cost tracking.

convex/deploymentRouter.ts-90-98 (1)

90-98: ⚠️ Potential issue | 🟡 Minor

Misleading error message — freemium tier already supports Ollama.

Line 95 says: "Upgrade to Personal tier for Ollama support." But tierConfig.ts (line 61) defines freemium.allowedProviders: ["ollama", "lmstudio"]. Freemium users already have Ollama access; the issue here is that their model string doesn't match the Bedrock format. The message should guide the user to select a valid Bedrock model or use a local Ollama model instead.

Also, upgradeUrl is /settings/aws here (line 96) vs /settings on line 78 — consider being consistent.

Proposed message fix

       return {
         success: false,
         error: "Invalid model format",
-        message: "AgentCore requires AWS Bedrock models (e.g., anthropic.claude-sonnet-4-5). Upgrade to Personal tier for Ollama support.",
-        upgradeUrl: "/settings/aws",
+        message: "AgentCore deployment requires a Bedrock model ID (e.g., anthropic.claude-sonnet-4-5). Switch to a Bedrock model or test locally with Ollama.",
+        upgradeUrl: "/settings",
       };

convex/lib/dynamicModelSwitching.ts-113-113 (1)

113-113: ⚠️ Potential issue | 🟡 Minor

Unnecessary escape characters in regex character class.

ESLint correctly flags \+, \*, and \/ as unnecessary escapes inside [...]. These characters don't need escaping in a character class:

-  const hasMath = /\d+[\+\-\*\/]\d+|equation|formula|calculate/.test(lowerMessage);
+  const hasMath = /\d+[+\-*/]\d+|equation|formula|calculate/.test(lowerMessage);

(\- is kept since it's between other characters in the class and could be interpreted as a range.)

convex/agentcoreTestExecution.ts-376-407 (1)

376-407: ⚠️ Potential issue | 🟡 Minor

Use let/const instead of var (line 392); reconsider 0.0.0.0 in SSRF allowlist.

Two issues in this block:

1. var response on line 392 — ESLint correctly flags this. Use let (since it's assigned in a try block and used afterward, restructure to avoid the var scoping hack):

2. 0.0.0.0 in the SSRF allowlist (line 377) — on some platforms, 0.0.0.0 can route to any local interface. While less exploitable than arbitrary hosts, it's worth removing if localhost/127.0.0.1 already cover the intended use case.

Proposed fix for var and 0.0.0.0

-    const allowedHosts = ["localhost", "127.0.0.1", "0.0.0.0", "::1"];
+    const allowedHosts = ["localhost", "127.0.0.1", "::1"];
     try {
       const endpointUrl = new URL(params.ollamaEndpoint);
       if (!allowedHosts.includes(endpointUrl.hostname)) {
@@ ... @@
-    try {
-      var response = await fetch(`${params.ollamaEndpoint}/v1/chat/completions`, {
+    const response = await fetch(`${params.ollamaEndpoint}/v1/chat/completions`, {
         method: "POST",
         ...
-      });
-    } finally {
-      clearTimeout(timeoutId);
-    }
+    });
+    clearTimeout(timeoutId);

Or keep the try/finally but declare response with let before the try block.

convex/strandsAgentExecutionDynamic.ts-270-271 (1)

270-271: ⚠️ Potential issue | 🟡 Minor

response.body may be undefined — guard before decoding.

If the Bedrock response has no body (e.g., throttling or service error that still returns 200), new TextDecoder().decode(undefined) yields an empty string, and JSON.parse("") throws a SyntaxError with a cryptic message. Add a guard:

🛡️ Proposed fix

   const response = await client.send(command);
+  if (!response.body) {
+    throw new Error(`Empty response from Bedrock model ${modelId}`);
+  }
   const responseBody = JSON.parse(new TextDecoder().decode(response.body)) as BedrockInvokeResponse;

🧹 Nitpick comments (18)

src/App.tsx (4)

20-20: Extract the view union type to avoid duplication and fix the loose currentView typing in Content.

The view union is defined inline on Line 20, then partially repeated on Line 166 for onNavigate, but currentView is typed as plain string. This defeats TypeScript's exhaustiveness checking in the switch and means invalid views won't be caught at compile time.

♻️ Proposed fix

Define the type once near the top of the file and reuse it:

+type AppView = "dashboard" | "builder" | "aiBuilder" | "chat" | "mcp" | "errors" | "audit" | "settings" | "visualScripting";
+
 export default function App() {
-  const [currentView, setCurrentView] = useState<"dashboard" | "builder" | "aiBuilder" | "chat" | "mcp" | "errors" | "audit" | "settings" | "visualScripting">("dashboard");
+  const [currentView, setCurrentView] = useState<AppView>("dashboard");

-function Content({ currentView, onNavigate }: { currentView: string; onNavigate: (view: "dashboard" | "builder" | "aiBuilder" | "chat" | "mcp" | "errors" | "audit" | "settings" | "visualScripting") => void }) {
+function Content({ currentView, onNavigate }: { currentView: AppView; onNavigate: (view: AppView) => void }) {

Also applies to: 166-166

---

124-134: View key "settings" is misleading — it renders PricingPanel and is labeled "Billing".

The internal view identifier "settings" maps to a billing/pricing component, not a general settings page. This will cause confusion if an actual settings view is added later. Consider renaming the view key to "billing" to match the UI label and rendered component.

Also applies to: 195-196

---

36-134: Consider extracting the repeated nav button pattern into a reusable component.

All 9 nav buttons follow the exact same structure (icon + label + active-state styling). With the addition of Visual Scripting and Billing, the nav block is now ~100 lines of near-identical JSX. A small NavButton component would reduce this significantly and make adding/removing views trivial.

♻️ Example extraction

function NavButton({ view, currentView, onClick, icon: Icon, label }: {
  view: AppView; currentView: AppView;
  onClick: (v: AppView) => void; icon: React.ElementType; label: string;
}) {
  return (
    <button
      onClick={() => onClick(view)}
      className={`flex items-center gap-2 px-3 py-2 rounded-lg transition-colors ${
        currentView === view
          ? "bg-green-900/30 text-green-400"
          : "text-green-600 hover:text-green-400"
      }`}
    >
      <Icon className="w-4 h-4" />
      {label}
    </button>
  );
}

Then the nav becomes a concise list of <NavButton> entries.

---

35-136: Nav bar with 9 items may overflow on smaller viewports.

With 9 nav buttons plus the sign-out button, the horizontal nav will likely overflow or wrap awkwardly on tablets and smaller desktops. Consider adding horizontal scroll, a responsive dropdown/hamburger, or grouping secondary items (Errors, Audit, Billing) under a "More" menu.

convex/schema.ts (3)

531-541: Inconsistent nullability: v.union(v.string(), v.null()) vs v.optional(v.string()).

agentRequirements fields use v.union(v.string(), v.null()) (requiring the field to be present with an explicit null), while every other nullable field in the schema uses v.optional(...). This creates two different patterns for "no value" that callers must handle differently. If the intent is the same, standardize on v.optional(v.string()) for consistency; if the explicit-null semantics are deliberate, add a brief comment explaining why.

---

706-716: config: v.any() in workflowNodeValidator disables all runtime validation for node configs.

While the comment says "validated at runtime by sanitizeNode", any data written to this field via a Convex mutation bypasses schema validation entirely. If sanitizeNode has a bug or is bypassed, malformed or malicious data enters the database unchecked. This pattern is repeated across the schema (~20 v.any() fields). Where possible, consider using v.union(...) with known shapes, or at minimum v.record(v.string(), v.any()) to enforce that the value is at least an object.

---

562-569: Deprecated messages field still present in interleavedConversations.

The inline comment marks this field as deprecated in favor of interleavedMessages, but it remains in the schema. If migration is complete, removing it avoids confusion and prevents new code from accidentally writing to it. If migration is still in progress, consider adding a tracking issue.

,

Would you like me to open an issue to track the removal of this deprecated field?

src/components/ModelSelector.tsx (1)

276-297: Custom model input lacks basic validation.

The custom model ID field accepts any string. Consider adding a reasonable length limit and/or a pattern constraint (e.g., no whitespace-only or extremely long strings) to prevent nonsensical model IDs from being propagated upstream.

Also, minor inconsistency: line 280 uses (e) for the event parameter while line 119 uses (event).

convex/stripe.ts (3)

201-231: updateSubscription hardcodes tier: "personal" — not extensible for Enterprise tier.

The PR description and PricingPanel component reference Free, Personal, and Enterprise tiers, but this mutation always sets tier: "personal" and role: "paid" regardless of which subscription was purchased. If an Enterprise tier is added later, this handler will downgrade Enterprise subscribers to Personal on every subscription.updated event.

Consider either passing tier as an argument from the webhook (which can read checkout session metadata) or deriving it from the Stripe price/product ID.

---

320-343: Consider whether stripeCustomerId needs to be exposed to the client.

Line 339 returns the Stripe customer ID to the frontend. If the client doesn't need it (e.g., for portal session creation, you already look it up server-side), omitting it reduces the attack surface. If the client does use it, this is fine.

---

35-45: Consider pinning the Stripe API version for safer upgrades.

Not specifying apiVersion means the client uses the SDK's default version, which can change when you upgrade the stripe package. This can alter response shapes silently. Stripe's documentation explicitly recommends pinning the version your integration was tested against.

♻️ Suggested change

-  return new StripeSDK( secretKey );
+  return new StripeSDK( secretKey, { apiVersion: "2024-09-30.acacia" } );

Replace the example version with the API version you've tested against (check your Stripe Dashboard for the current version).

convex/testExecution.ts (1)

194-203: User document fetched twice in the same mutation.

On line 87, user is already fetched via ctx.db.get(userId) for the rate-limiting check. On line 196, the same document is fetched again. Reuse the existing reference instead:

Proposed refactor

     // RATE LIMITING: Increment user's test count ONLY for cloud models (not Ollama)
     if (!isOllamaModel) {
-      const user = await ctx.db.get(userId);
       if (user) {
         await ctx.db.patch(userId, {
           testsThisMonth: (user.testsThisMonth || 0) + 1,

Note: user is declared inside the if (!isOllamaModel) block at line 87, so you'd need to hoist it or restructure the scoping.

convex/lib/tierConfig.ts (1)

222-229: getUpgradeMessage hardcodes the tier progression, which won't scale if tiers change.

The next-tier logic (freemium → Personal, everything else → Enterprise) and pricing ($5/month / Contact us) are hardcoded strings. If tier names or pricing change, this will silently produce incorrect messages. Consider deriving this from TIER_CONFIGS or at minimum adding a comment noting the coupling.

convex/deploymentRouter.ts (1)

68-72: Dynamic import pattern is fine but could be a top-level import.

deploymentRouter.ts doesn't appear to have a circular dependency with tierConfig.ts. A top-level import { getTierConfig } from "./lib/tierConfig" would be simpler and avoid re-importing on every invocation.

convex/lib/dynamicModelSwitching.ts (1)

316-346: defaultModelId parameter is accepted but never used.

The switchingModelCall wrapper accepts defaultModelId (line 323) but the model ID is always taken from decision.selectedModel.modelId (line 340). Either use defaultModelId as a fallback or remove it from the signature.

convex/strandsAgentExecutionDynamic.ts (3)

11-17: Unused imports: analyzeComplexity, calculateComplexityScore, selectModel

Only decideModelSwitch and ModelSwitchDecision are used in this file. The other three named imports are unused and can be removed to keep the import clean.

♻️ Proposed fix

 import {
-  analyzeComplexity,
-  calculateComplexityScore,
-  selectModel,
   decideModelSwitch,
   type ModelSwitchDecision,
 } from "./lib/dynamicModelSwitching";

#!/bin/bash
# Verify these symbols are not used anywhere in this file
rg -n 'analyzeComplexity|calculateComplexityScore|selectModel' convex/strandsAgentExecutionDynamic.ts

---

169-172: Consider structured logging or a lower verbosity level for production.

Four console.log calls per request will be noisy in production logs. Consider using a structured logger with a debug level, or gating these behind an environment flag.

---

206-212: AWS credentials use non-null assertions — consider a pre-flight check.

Lines 209-210 use ! assertions on process.env.AWS_ACCESS_KEY_ID and process.env.AWS_SECRET_ACCESS_KEY. If these env vars are missing, the SDK will receive undefined credentials and produce a confusing auth error at call time. A quick guard with a descriptive error would improve debuggability.

🛡️ Proposed fix

+  const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+  if (!accessKeyId || !secretAccessKey) {
+    throw new Error("AWS credentials not configured (AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY)");
+  }
+
   const client = new BedrockRuntimeClient({
     region: process.env.AWS_REGION || "us-east-1",
     credentials: {
-      accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
-      secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
+      accessKeyId,
+      secretAccessKey,
     },
   });

[coderabbitai]

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)

convex/containerOrchestrator.ts (1)

352-394: ⚠️ Potential issue | 🟠 Major

getTaskStatus is a public action with no authentication or authorization.

Any caller with a valid taskArn string can query ECS task metadata (status, CPU, memory, container exit codes). Consider making this an internalAction or adding auth checks, consistent with the other actions in this file.

convex/modelRegistry.ts (1)

93-104: ⚠️ Potential issue | 🟡 Minor

Model name mismatch: ID says "4-1" (Claude 4.1) but name says "Claude 4.5 Opus".

The key and id field are anthropic.claude-opus-4-1-20250805-v1:0 (4.1 series), yet name is "Claude 4.5 Opus". This will confuse users in the UI. Should be "Claude 4.1 Opus".

Proposed fix

   "anthropic.claude-opus-4-1-20250805-v1:0": {
     id: "anthropic.claude-opus-4-1-20250805-v1:0",
-    name: "Claude 4.5 Opus",
+    name: "Claude 4.1 Opus",

convex/apiKeys.ts (3)

259-266: ⚠️ Potential issue | 🔴 Critical

Math.random() is not cryptographically secure for API key generation.

Math.random() is predictable and unsuitable for generating security tokens. Use crypto.getRandomValues() instead.

Proposed fix

 function generateSecureToken(length: number): string {
-  const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
-  let result = '';
-  for (let i = 0; i < length; i++) {
-    result += chars.charAt(Math.floor(Math.random() * chars.length));
-  }
-  return result;
+  const bytes = new Uint8Array(length);
+  crypto.getRandomValues(bytes);
+  const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+  return Array.from(bytes, b => chars[b % chars.length]).join('');
 }

---

172-182: ⚠️ Potential issue | 🟠 Major

findByHash is a public query — allows unauthenticated callers to probe for valid API key hashes.

An attacker who obtains a key hash (or can compute one given the weak secret fallback) can verify its existence without authentication. This should be an internalQuery, since the only caller is validateAPIKeyAndUsage (which already uses ctx.runQuery).

Proposed fix

-export const findByHash = query({
+export const findByHash = internalQuery({

Note: Update the caller at Line 119 from api.apiKeys.findByHash to internal.apiKeys.findByHash (using ctx.runQuery with the internal reference).

---

271-278: ⚠️ Potential issue | 🟠 Major

Fallback secret 'default-secret' for API key hashing is dangerous.

If JWT_PRIVATE_KEY is unset, all API key hashes use the same well-known pepper, making them trivially reversible via rainbow tables. At minimum, log a warning or throw in production when the env var is missing.

convex/guardrails.ts (2)

161-177: ⚠️ Potential issue | 🔴 Critical

Inconsistent model ID date stamp — 20250514 should be 20251001.

Other files in this PR (e.g., convex/lib/dynamicModelSwitching.ts line 36, convex/lib/unifiedModalitySwitching.ts line 45, convex/automatedAgentBuilder.ts line 307) already use the corrected date 20251001. This default and the comment on line 167 ("pricing as of 2024") are stale.

Proposed fix

 export function calculateMessageCost(
   inputTokens: number,
   outputTokens: number,
   reasoningTokens: number = 0,
-  modelId: string = "anthropic.claude-haiku-4-5-20250514-v1:0"
+  modelId: string = "anthropic.claude-haiku-4-5-20251001-v1:0"
 ): number {
-  // Claude Haiku 4.5 pricing (as of 2024)
+  // Claude Haiku 4.5 pricing

---

137-156: ⚠️ Potential issue | 🟡 Minor

Unused variable hoursElapsed — dead code.

hoursElapsed on line 144 is computed but never referenced. The rate limit check only compares messagesPerHour (which is just messageCount) against config.maxMessagesPerHour. This suggests either the rate calculation is incomplete (should divide by hours) or the variable is leftover.

If it's intentionally unused, remove it

-  const messagesPerHour = messageCount;
-  const hoursElapsed = timeWindow / ( 1000 * 60 * 60 );
+  const messagesPerHour = messageCount;

convex/agentBuilderWorkflow.ts (1)

386-396: ⚠️ Potential issue | 🟡 Minor

Model divergence between streaming and non-streaming paths should be reconciled or explicitly documented.

streamWorkflowExecution uses Anthropic SDK with claude-3-7-sonnet-20250219, while executeWorkflowStage and executeCompleteWorkflow use Bedrock with Haiku 4.5 (default). This creates inconsistent model behavior and pricing between the streaming and non-streaming workflow paths. If the Sonnet model is intentionally used for streaming quality, this decision should be documented in code comments. Otherwise, consider aligning both paths to use the same model configuration for consistency.

🤖 Fix all issues with AI agents

In `@convex/agentBuilderWorkflow.ts`:
- Around line 15-18: Update the stale model ID in the WORKFLOW_MODEL_ID
constant: replace the hard-coded string
"anthropic.claude-haiku-4-5-20250514-v1:0" with the same model string using the
date suffix 20251001 (i.e., "anthropic.claude-haiku-4-5-20251001-v1:0") so
WORKFLOW_MODEL_ID matches the other updated files; search for the
WORKFLOW_MODEL_ID declaration and adjust the literal accordingly.
- Around line 198-203: The loop variable `ctx` in the block that iterates over
`args.previousContext` is shadowing the outer `ctx` action/context parameter
(and the same shadowing occurs in `streamWorkflowExecution` around the similar
loop), so rename the loop variable (e.g., to `prevCtx` or `prevContextItem`) in
the for-of loops that iterate `args.previousContext` to avoid masking the outer
`ctx` (update both the block that builds `contextPrompt` and the analogous loop
in `streamWorkflowExecution`).

In `@convex/agentcoreTestExecution.ts`:
- Around line 407-417: The allowlist in the Ollama SSRF check currently includes
"0.0.0.0" which is overly permissive; update the validation in the block that
builds allowedHosts (refer to allowedHosts and params.ollamaEndpoint in
agentcoreTestExecution.ts) to remove "0.0.0.0" from the list (leaving localhost,
127.0.0.1, ::1) or replace it with a documented, explicit host if you truly need
that destination, and keep the existing URL parsing/throw logic unchanged so
invalid or non-local hosts still reject.
- Around line 386-394: The loop that appends params.conversationHistory to
messages coerces any non-"assistant" role to "user", losing "system" and "tool";
update the logic in the block that iterates params.conversationHistory.slice(-5)
so it only includes messages whose role is one of the valid conversational roles
(e.g. "user","assistant","system","tool") and uses msg.role directly for
messages that pass that check, skipping any unknown roles instead of forcing
them to "user" (identify the change around params.conversationHistory,
messages.push, and the role assignment).
- Around line 308-316: The loop that appends conversation history (the block
using params.conversationHistory.slice(-5) and messages.push) currently coerces
any non-"assistant" role to "user", which incorrectly maps "system" and "tool"
messages to user; change it to only include messages whose role is "user" or
"assistant" (preserving "assistant" as "assistant" and "user" as "user") and
skip any other roles (e.g., "system" or "tool"), mirroring the filtering
behavior used in strandsAgentExecutionDynamic.ts to avoid sending system/tool
content as user messages.
- Around line 234-302: The modelMap contains incorrect Bedrock model identifier
strings for Claude 4.x/4.5 causing runtime lookup failures; update the values
inside modelMap for the keys "claude-sonnet-4.5"/"claude-4.5-sonnet" to
"anthropic.claude-sonnet-4-5-20250929-v1:0", for
"claude-opus-4.1"/"claude-4.1-opus" to
"anthropic.claude-opus-4-1-20250805-v1:0", and for
"claude-opus-4"/"claude-4-opus" to "anthropic.claude-opus-4-20250514-v1:0"
(leave the rest of modelMap and the modelId lookup as-is).
- Around line 422-438: Replace the function-scoped `var response` with
block-scoped `let response` inside the try where the fetch to
`${params.ollamaEndpoint}/v1/chat/completions` occurs so the `response` binding
does not leak outside the try block; update any subsequent uses within the same
block to reference `response` (e.g., the variable declared around the fetch call
in agentcoreTestExecution.ts) and ensure no other code relies on a hoisted
`response` variable.

In `@convex/automatedAgentBuilder.ts`:
- Around line 374-383: The JSON-parse fallback currently returns an empty object
for requirements which wipes session state; change the catch block so it
preserves state by returning requirements: null (or undefined) instead of {}
(keep the other fields: thinking, nextQuestion, readyToGenerate, agentConfig),
and then update the code that applies patches (the place that sets
agentRequirements: requirements) to skip patching when requirements is
null/undefined so existing agentRequirements are not overwritten; refer to the
variables requirements and agentRequirements and the fallback return object
(nextQuestion, readyToGenerate, agentConfig) to locate the changes.
- Around line 300-306: The Bedrock/Anthropic message content blocks built in the
conversationHistory mapping (variable messages) omit the required type field;
change the mapping that constructs messages (the arrow function producing {
role: ..., content: [...] }) so each content object is { type: "text", text:
msg.content } instead of { text: msg.content }, matching the format used in
agentBuilderWorkflow.ts and ensuring compatibility with anthropic_version:
"bedrock-2023-05-31".

In `@convex/deploymentRouter.ts`:
- Around line 204-213: The incrementUsage mutation currently accepts any userId
and can trigger Stripe overage reports; restrict it by verifying the caller is
the authenticated user or making it internal-only: in the mutation handler for
incrementUsage, get the caller id via getAuthUserId(ctx) (or equivalent on ctx)
and either (a) throw/return unauthorized if args.userId !== getAuthUserId(ctx)
before calling incrementUsageAndReportOverageImpl, or (b) remove the userId arg
and call incrementUsageAndReportOverageImpl with the auth user id only (or mark
the mutation as internal/private). Ensure you reference incrementUsage, its
handler, and incrementUsageAndReportOverageImpl when making the change.

In `@convex/http.ts`:
- Around line 514-517: The catch block in the Stripe webhook handler currently
returns a 500 (causing Stripe retries) which can re-run mutations like
updateSubscription, cancelSubscription, and resetMonthlyUsage; change the
handler so it always acknowledges the webhook with a 200/2xx to Stripe and
surface failures internally (log via processLogger/error reporting and alerting)
or make the mutations fully idempotent (ensure updateSubscription and
cancelSubscription are safe to run multiple times and that resetMonthlyUsage
only sets state to a deterministic value or checks a processed-event marker
before mutating). Specifically, modify the catch around the webhook processing
(the try/catch handling handlerError) to return a successful Response to Stripe
while capturing handlerError details for retries/alerts, or add idempotency
checks inside updateSubscription, cancelSubscription, and resetMonthlyUsage to
avoid double effects on retry.
- Around line 460-466: The code is using an unnecessary cast "(internal as
any).stripeMutations" which bypasses TS checks; replace all occurrences of that
pattern with the type-safe "internal.stripeMutations" and call the mutation via
ctx.runMutation(internal.stripeMutations.updateSubscription, { ... }) (and the
other mutation names found) so TypeScript can validate exported signatures;
update every spot where "(internal as any).stripeMutations" appears to use
"internal.stripeMutations" directly.

In `@convex/lib/dynamicModelSwitching.ts`:
- Around line 195-231: The selectModel function currently ignores the
destructured preference flags (preferCost, preferSpeed, preferCapability) —
update selectModel to factor these into the selection after determining
candidate tiers: use preferCost to bias toward lower-index MODEL_TIERS (cheaper)
when complexityScore borders thresholds, preferSpeed to prefer faster tiers
(e.g., lower-latency candidate like haiku/sonnet), and preferCapability to
prefer higher-capability tiers (e.g., opus) by adjusting which index is chosen
from availableModels; implement a small scoring or tie-breaker step (e.g.,
compute candidateIndices based on complexityScore, then modify scores for each
candidate based on the preference flags) and return the highest-scoring
availableModels entry so the function (selectModel) actually respects
preferCost/preferSpeed/preferCapability while still honoring complexityScore and
userTier.
- Around line 56-64: The "opus" model entry (key: opus) uses the retired Bedrock
modelId "anthropic.claude-3-opus-20240229-v1:0" which will fail at runtime;
remove the entire opus object or replace its modelId with a currently supported
Anthropic Claude modelId and update associated metadata (costPer1KInput,
costPer1KOutput, maxTokens, speedRating, capabilityRating) accordingly; also
search for any code that references the opus key (e.g., dynamicModelSwitching
lookups) and update callers to use the new model key or handle absence.
- Around line 206-211: The Ollama detection (isOllamaAgent) incorrectly treats
versioned names like "llama3.2:latest" as non-Ollama because it rejects models
containing "."; replace this with a safer check: consider a model Ollama if it
contains ":" unless it matches a known Bedrock/Claude prefix (e.g., entries in a
small list of Bedrock prefixes), so update the logic that sets isOllamaAgent
(used to compute availableModels and referencing agent.model and MODEL_TIERS) to
test for ":" and then exclude only models that start with known Bedrock provider
prefixes rather than rejecting any name with a ".".

In `@convex/strandsAgentExecution.ts`:
- Around line 184-191: The modelMap contains a typo in the Bedrock model string
for the key "claude-3-5-sonnet-20241022" — the value
"anthropicaude-3-5-sonnet-20241022-v2:0" is missing the dot between "anthropic"
and "claude"; update the mapping in the modelMap used to compute modelId (refer
to variables modelMap, modelId and agent.model) to the correct Bedrock format
"anthropic.claude-3-5-sonnet-20241022-v2:0" and scan the other map entries to
ensure they follow the same "anthropic.claude-...:..." pattern.
- Around line 104-109: Fix two issues: correct the model ID typo from
"anthropicaude-3-5-sonnet-20241022-v2:0" to
"anthropic.claude-3-5-sonnet-20241022-v2:0" wherever that model mapping is
defined so model resolution succeeds, and update the Bedrock detection regex
used in the isBedrock computation (the const using agent.deploymentType and
/^(us\.)?(anthropic|amazon|meta|mistral|cohere|ai21)\./.test(agent.model)) to
also accept other region prefixes such as "eu.", "apac.", and "global." (e.g.,
allow optional (us|eu|apac|global)\. ), ensuring the check still falls back to
deploymentType and continues to use isProviderAllowedForTier(userTier,
"bedrock") for gating.

In `@convex/strandsAgentExecutionDynamic.ts`:
- Around line 320-341: The parsing currently assumes Anthropic-style blocks
(responseBody.content) which yields empty content for other Bedrock providers;
update the response handling after client.send(command) to branch by
provider/model and extract text accordingly: keep the existing Anthropic block
parsing for when responseBody.content exists, add a branch to read
responseBody.generation (or responseBody.generation[0] if applicable) for Llama
models, extract and concatenate responseBody.outputs[].text for Mistral, and add
handlers for Cohere/AI21 shapes as needed; populate the same variables (content,
reasoning, toolCalls) so downstream logic using those symbols stays unchanged
and ensure you detect provider/model via the same selector used when building
the request.

In `@convex/stripeMutations.ts`:
- Around line 237-246: The getSubscriptionStatus query is returning the internal
stripeCustomerId to the client; remove stripeCustomerId from the returned object
in the getSubscriptionStatus resolver (the property currently set at
stripeCustomerId: user.stripeCustomerId ?? null) so that the client no longer
receives Stripe internal IDs, and instead only return tier, role,
subscriptionStatus, executionsThisMonth, currentPeriodEnd, and
hasActiveSubscription; if any client code depends on stripeCustomerId, update it
to use a server-only endpoint or remove the dependency.

In `@convex/testExecution.ts`:
- Around line 906-913: The code calls calculateBedrockCost with a hardcoded
"claude-3-5-sonnet" which never matches the pricing keys; change the call in the
test execution creation to pass the real model identifier from the execution
record (e.g., use args.modelId, args.model, or the execution's stored modelId
field) instead of the hardcoded string so calculateBedrockCost can look up the
correct pricing key, or if the hardcode is intentional, update the surrounding
comment to state why a fallback/default is expected; target the
calculateBedrockCost call in this block to make the change.

🧹 Nitpick comments (7)

convex/containerOrchestrator.ts (1)

210-219: Redundant re-query: test is already fetched and null-checked at Line 179.

testDetails at Line 211 duplicates the query made at Line 179. The test variable (which was null-checked at Line 181) already contains userId. This avoids an unnecessary round-trip.

Proposed fix

          if (logsText.includes("TEST COMPLETED SUCCESSFULLY")) {
-            // Get test details for usage tracking
-            const testDetails = await ctx.runQuery(internal.testExecution.getTestByIdInternal, { testId: args.testId });
-            if (testDetails) {
-              // TRACK USAGE: On successful completion
-              await ctx.runMutation(internal.testExecution.incrementUserUsage, {
-                userId: testDetails.userId,
-                testId: args.testId,
-                executionMethod: "fargate",
-              });
-            }
+            // TRACK USAGE: On successful completion
+            await ctx.runMutation(internal.testExecution.incrementUserUsage, {
+              userId: test.userId,
+              testId: args.testId,
+              executionMethod: "fargate",
+            });

convex/lib/roles.ts (2)

3-4: MODERATOR_ROLES is identical to ADMIN_ROLES — intentional placeholder?

Both arrays contain only UserRole.ADMIN. If a separate moderator role is planned, consider adding a TODO. If not, consolidating to a single constant avoids confusion.

---

10-18: roleHierarchy is re-created on every hasPermission call.

This is fine for the current usage volume but could be hoisted to module scope as a constant.

convex/modelRegistry.ts (1)

987-1035: getModelConfig throws for providers listed in the ModelProvider type but not handled.

ModelProvider includes "openai" | "anthropic" | "azure" | "google", but getModelConfig only handles "bedrock" and "ollama", throwing for anything else. If those additional providers are added to the registry in the future, this will break at runtime.

convex/lib/dynamicModelSwitching.ts (1)

113-114: Fix unnecessary escape characters in regex (ESLint no-useless-escape).

Static analysis flags \+, \*, and \/ as unnecessary inside a character class.

Proposed fix

-  const hasMath = /\d+[\+\-\*\/]\d+|equation|formula|calculate/.test( lowerMessage );
+  const hasMath = /\d+[+\-*/]\d+|equation|formula|calculate/.test( lowerMessage );

convex/agentcoreTestExecution.ts (1)

36-45: Status updated to RUNNING before agent null-check.

Line 37 sets status to "RUNNING", but the agent null-check happens at line 43. If the agent is null, the test is left in "RUNNING" state permanently (the thrown error at line 44 is caught, but status is set to "FAILED" with error.message — so it does recover). However, reordering would be cleaner and avoid the unnecessary mutation.

Suggested reorder

+     const agent = await ctx.runQuery( internal.agents.getInternal, { id: args.agentId } );
+     if ( !agent ) {
+       throw new Error( "Agent not found" );
+     }
+
      // Update status to running
      await ctx.runMutation( internal.testExecution.updateStatus, {
        testId: args.testId,
        status: "RUNNING",
      } );

-     // CHECK LIMITS: Only when actually executing
-     if ( !agent ) {
-       throw new Error( "Agent not found" );
-     }

convex/strandsAgentExecutionDynamic.ts (1)

253-260: Model normalization map is minimal compared to other execution paths.

This map contains only 3 entries (claude-3-5-sonnet, claude-3-5-haiku, claude-3-opus), while agentcoreTestExecution.ts has ~40+ entries covering Nova, Titan, Llama, Mistral, AI21, Cohere, and newer Claude 4.x models. If decideModelSwitch selects a short-name model not in this map, it silently falls through to the env var / hardcoded default instead of the intended model.

Consider extracting the model map to a shared module (or reusing the one from modelRegistry.ts) to keep both execution paths consistent.

#!/bin/bash
# Check if modelRegistry.ts has a reusable mapping that could replace these inline maps
rg -n "modelMap|shortName|normalize" convex/modelRegistry.ts | head -20
# Also check what models decideModelSwitch can return
rg -n "modelId" convex/lib/dynamicModelSwitching.ts | head -30

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Same role coercion issue in Ollama path — "system" and "tool" history messages become "user".

Same issue as the Bedrock path: non-assistant history messages are coerced to "user". Filter to only valid conversational roles.

🤖 Prompt for AI Agents

In `@convex/agentcoreTestExecution.ts` around lines 386 - 394, The loop that
appends params.conversationHistory to messages coerces any non-"assistant" role
to "user", losing "system" and "tool"; update the logic in the block that
iterates params.conversationHistory.slice(-5) so it only includes messages whose
role is one of the valid conversational roles (e.g.
"user","assistant","system","tool") and uses msg.role directly for messages that
pass that check, skipping any unknown roles instead of forcing them to "user"
(identify the change around params.conversationHistory, messages.push, and the
role assignment).

[coderabbitai]

Actionable comments posted: 15

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

convex/deploymentRouter.ts (1)

248-257: ⚠️ Potential issue | 🟠 Major

Restrict deployment history queries to the caller’s records.

When agentId is provided, the query drops the user filter and can leak other users’ deployments.

Suggested fix

     if (args.agentId) {
       query = ctx.db
         .query("deployments")
-        .withIndex("by_agent", (q) => q.eq("agentId", args.agentId!));
+        .withIndex("by_agent", (q) => q.eq("agentId", args.agentId!))
+        .filter((q) => q.eq(q.field("userId"), userId));
     }

convex/awsDeployment.ts (1)

659-679: ⚠️ Potential issue | 🟡 Minor

enableMonitoring can never be turned off.

config.enableMonitoring || true always resolves to true. Use nullish coalescing to respect explicit false.

🛠️ Fix

-    monitoring: {
-      enabled: config.enableMonitoring || true,
+    monitoring: {
+      enabled: config.enableMonitoring ?? true,
       logLevel: "INFO"
     }

convex/guardrails.ts (1)

161-176: ⚠️ Potential issue | 🔴 Critical

Update Claude Haiku 4.5 pricing constants—current values are significantly outdated.

The code uses Claude Haiku 4.5 pricing from 2024, but the default model ID targets a 2025 release. Current pricing (as of October 2025) is $1 per million input tokens and $5 per million output tokens. The code currently has:

• inputCostPer1K = 0.00025 (should be 0.001)
• outputCostPer1K = 0.00125 (should be 0.005)

This 4x underpricing means cost guardrails will allow approximately 4x more token spending than intended before enforcing limits. Update these constants and the comment to reflect current pricing.

🤖 Fix all issues with AI agents

In `@convex/agentAsToolGenerator.ts`:
- Around line 24-25: Add a helper (e.g., makeValidPythonIdentifier) that takes a
string and returns a valid Python identifier by: replacing
non-alphanumeric/underscore chars with '_', ensuring it does not start with a
digit (prefix with '_' if it does), and returning '_' if the result would be
empty; then replace the existing raw derivations with calls to this helper so
that toolName (derived from agent.name) and the two other generated
identifier/filename derivations used in generateToolCode and any import/function
names all use makeValidPythonIdentifier consistently (update usages around
generateToolCode, the initial const toolName, and the other generation sites
currently producing Python identifiers).

In `@convex/automatedAgentBuilder.ts`:
- Around line 116-129: Replace the loose identity check in both processResponse
and generateAgentFromSession by calling ctx.auth.getAuthUserId() instead of
getUserIdentity(), then after fetching the session via
internal.automatedAgentBuilder.getBuildSessionInternal (using args.sessionId)
verify session.userId === authUserId and throw an authorization error if it does
not match; apply this ownership check in both action handlers to ensure only the
session owner can access or modify the agentBuildSessions record.

In `@convex/awsDeployment.ts`:
- Around line 601-618: The forEach callbacks in generateAgentCoreRequirements
implicitly return the value from packages.add which triggers the lint error;
change both callbacks to use block bodies and call packages.add without
returning (e.g., tools.forEach(tool => { if (tool.requiresPip &&
tool.pipPackages) { tool.pipPackages.forEach((pkg: string) => {
packages.add(pkg); }); } });) so the callbacks have explicit statements and no
implicit return; keep the same symbols (generateAgentCoreRequirements, tools,
tool.requiresPip, tool.pipPackages, packages.add).
- Around line 620-641: The Ollama detection in generateAgentCoreDockerfile is
too strict (it uses absence of a dot in agent.model); update the detection to
prefer an explicit provider flag and fall back to safer heuristics: treat the
agent as Ollama when agent.modelProvider === "ollama" (or a similar explicit
property on the agent) and only if that is absent fall back to checking known
Ollama model prefixes (e.g., strings starting with "ollama" or "llama") or a
registry lookup, instead of the current dot/no-dot test; keep the existing
safeModelPattern and modelName sanitization (and the rest of the Dockerfile
generation) but replace the isOllamaModel assignment with this provider-first
logic so Ollama IDs like "llama3.1:8b" are correctly detected.

In `@convex/deploymentRouter.ts`:
- Around line 96-103: The current Bedrock check in deploymentRouter.ts uses a
weak test (agent.model?.includes(".")) and should be replaced with the robust
Bedrock regex used in strandsAgentExecution.ts or moved to a shared helper;
update the validation for AgentCore (the block returning { success:false,
error:"Invalid model format", ... }) to use the regex
/^(us\.|eu\.|apac\.|global\.)?(anthropic|amazon|meta|mistral|cohere|ai21)\./ or,
better, implement a reusable function (e.g., isBedrockModel in modelRegistry.ts)
and call that function here so validation is consistent across
strandsAgentExecution.ts and deploymentRouter.ts.

In `@convex/interleavedReasoning.ts`:
- Around line 338-348: addMessage currently sets sequenceNumber to Date.now()
which can collide; change addMessage to compute a deterministic sequenceNumber
like addMessageBatch does by querying the current max sequenceNumber for the
conversation and using max + 1 (or a conversation-specific counter) before
inserting. Concretely, in addMessage locate the insertion block and replace the
timestamp-as-sequence logic with a pre-insert read (e.g., ctx.db.query or
ctx.db.table("interleavedMessages").filter(...).max("sequenceNumber")) to get
the highest sequenceNumber for args.conversationId, set sequenceNumber = (max ||
0) + 1, and then insert using that computed sequenceNumber so ordering matches
addMessageBatch.
- Around line 27-29: The anonymous conversation token construction (variable
conversationToken) uses predictable Date.now() and Math.random(), so replace
that logic with a cryptographically secure UUID or random bytes (e.g.,
crypto.randomUUID() or crypto.randomBytes converted to hex) to generate the anon
token; update the creation site where conversationToken is assigned and ensure
any consumers such as getConversationInternal still accept the new format (keep
the "anon_" prefix if desired) so the token has sufficient entropy and is not
brute-forceable.

In `@convex/lib/dynamicModelSwitching.ts`:
- Around line 100-103: The hasMath regex (assigned to variable hasMath)
currently uses unnecessary backslashes inside the character class for +, *, and
/ which ESLint flags; update the pattern so the character class contains +, *,
and / without escaping them (retain escaping for the hyphen if used) and keep
the rest of the alternation (e.g., "equation", "formula", "calculate") intact so
hasMath still tests lowerMessage correctly.

In `@convex/modelRegistry.ts`:
- Around line 876-968: The registry lookups miss entries when model IDs include
geo prefixes (e.g., "us.", "eu.") because BEDROCK_MODELS keys are unprefixed;
normalize IDs before lookup. Add a small helper (e.g., normalizeBedrockId) or
reuse resolveBedrockModelId logic to strip known geo prefixes (from
BEDROCK_PREFIXES or a subset like "us.","eu.","apac.","global.") and return the
canonical registry key, then update getUnitsForModel to call that normalizer
(use the normalized id when checking BEDROCK_MODELS and ALL_MODELS) and apply
the same normalization wherever metadata lookups occur (e.g., any functions that
query BEDROCK_MODELS/ALL_MODELS) so short names and geo‑prefixed/full IDs
resolve consistently.

In `@convex/strandsAgentExecution.ts`:
- Around line 83-133: The executeAgentWithStrandsAgents action lacks an
auth/ownership guard: after fetching the agent (agent variable) and the
agentOwner lookup, verify the caller identity from ctx (e.g., ctx.auth or
equivalent request user id) and ensure it matches agent.createdBy (or allow via
ACL/role check), returning a failure result (success:false,
error:"Unauthorized") if not; place this check before rate limiting and
model/gating logic so paid Bedrock calls cannot be triggered by non-owners, and
use the same action name executeAgentWithStrandsAgents and agent.createdBy /
internal.users.getInternal references to locate where to add the guard.
- Around line 183-188: Filter out non-supported roles before building the
messages array when sending to Anthropic: in the loop that iterates over history
(used to populate messages in strandsAgentExecution.ts), check the
isAnthropicModel condition and only push entries whose msg.role is "user" or
"assistant" (skip "system" and "tool"); keep system content handled separately
via the top-level system field and ensure the code that constructs messages (the
messages.push block) only runs for allowed roles when isAnthropicModel is true.

In `@convex/strandsAgentExecutionDynamic.ts`:
- Around line 136-161: The code computes isOllamaModel but always routes to
Bedrock; fix by branching on isOllamaModel before calling
executeDirectBedrock/executeWithModelSwitching: if isOllamaModel, either call an
Ollama-specific executor (e.g., executeDirectOllama or executeWithModelSwitching
for Ollama) passing the same parameters (ctx, agent, args.message, history and
the preference object), or explicitly return a safe error blocking Ollama
models; ensure you check args.enableModelSwitching the same way as the Bedrock
path and preserve the preference object
(preferCost/preferSpeed/preferCapability/userTier) when invoking the Ollama
execution path so Ollama IDs are not misrouted to executeDirectBedrock.

In `@convex/stripe.ts`:
- Around line 167-206: The reportUsage internalAction currently creates
meterEvents via stripe.billing.meterEvents.create without an identifier,
allowing retries to double-bill; modify the handler in reportUsage to generate
and pass a deterministic identifier (e.g., derive an eventId from execution
context or args like a stable hash of stripeCustomerId + timestamp or a provided
eventId param) into the create call's identifier field before calling
stripe.billing.meterEvents.create so Stripe can deduplicate retries; ensure the
identifier generation is stable across retries for the same logical operation
and add eventId as an arg to reportUsage if needed.

In `@convex/testExecution.ts`:
- Around line 220-223: The code currently increments usage optimistically at
submission time by calling incrementUsageAndReportOverageImpl (in
convex/testExecution.ts) for non-Ollama models, but cancelTest does not reverse
that increment when a test in state QUEUED is cancelled; update cancelTest to
detect cancellations of QUEUED tests and decrement executionsThisMonth (or call
a new rollback function mirroring incrementUsageAndReportOverageImpl) to undo
the optimistic charge, referencing the same modelId/userId metadata and ensuring
updateLastTestAt behavior is preserved, or alternatively add a clear comment in
cancelTest explaining the intentional non-rollback policy if this behavior is
deliberate.

In `@Security_checkup.py`:
- Around line 1-3: This file currently only imports Agent, tool,
LocalChromiumBrowser, think, and readfile but contains no implementation; either
delete Security_checkup.py to remove dead code or implement the intended
security checkup logic—if implementing, create a clear entrypoint function
(e.g., run_security_checkup) that uses Agent and tool for orchestration and
LocalChromiumBrowser/think/readfile where needed, and consolidate the two
strands_tools imports into a single line importing LocalChromiumBrowser, think,
readfile to remove duplication; ensure unused imports are removed if not used.

🧹 Nitpick comments (9)

convex/testExecution.ts (3)

36-45: calculateBedrockCost usage parameter is typed as any.

Consider using a typed interface consistent with the incrementUserUsage args to get compile-time safety and avoid silent property-name mismatches.

Suggested typing

-function calculateBedrockCost(usage: any, modelId: string): number {
+function calculateBedrockCost(
+  usage: { inputTokens?: number; outputTokens?: number },
+  modelId: string,
+): number {

---

22-22: process.env access at module top-level in Convex.

Convex serverless functions may not have process.env available at module-evaluation time. If MAX_CONCURRENT_TESTS isn't set, parseInt("10") provides a safe fallback here, but parseInt without a radix can surprise with certain inputs.

-const MAX_CONCURRENT_TESTS = parseInt(process.env.MAX_CONCURRENT_TESTS || "10");
+const MAX_CONCURRENT_TESTS = parseInt(process.env.MAX_CONCURRENT_TESTS || "10", 10);

---

191-193: userId: effectiveUserId as any — unnecessary cast.

effectiveUserId is just userId (assigned on line 143 / 443) which is already typed as Id<"users">. The as any bypasses the type system. If the schema expects a different type for testExecutions.userId, fix the schema; otherwise remove the cast.

Also applies to: 446-448

convex/rateLimiter.ts (1)

256-259: Prefer tokenIdentifier to avoid cross-provider collisions.

Using identity.subject alone can collide across IdPs; tokenIdentifier is globally unique and safer as the rate-limit key.

Suggested fix

-      const userId = identity?.subject;
+      const userId = identity?.tokenIdentifier ?? identity?.subject;

convex/agentcoreSetup.ts (1)

123-143: Clarify “tests vs executions” naming to avoid misreporting.

monthlyTests is compared against tierConfig.monthlyExecutions, which can confuse limits/messages. Consider aligning variable names (or the limit source) so the error text and counters match.

🔎 Suggested naming alignment

-      const monthlyTests: any = 0; // Simplified for now
+      const monthlyExecutions: number = 0; // Simplified for now
...
-      if ( limit !== -1 && monthlyTests >= limit ) {
+      if ( limit !== -1 && monthlyExecutions >= limit ) {
         return {
           success: false,
-          error: `Monthly test limit reached (${monthlyTests}/${limit}). Upgrade for more tests.`,
+          error: `Monthly test limit reached (${monthlyExecutions}/${limit}). Upgrade for more tests.`,
         };
       }

convex/stripe.ts (1)

31-41: Pin Stripe API version for stability.

Relying on the SDK default can introduce breaking changes over time. Consider explicitly setting apiVersion (possibly via env) when instantiating the Stripe client.

🔧 Example pin (adjust version to your dashboard setting)

-  return new StripeSDK( secretKey );
+  const apiVersion = process.env.STRIPE_API_VERSION;
+  return new StripeSDK( secretKey, apiVersion ? { apiVersion } : undefined );

convex/interleavedReasoning.ts (3)

23-23: Remove ctx: any to preserve Convex's MutationCtx type safety.

The explicit ctx: any annotation overrides the type Convex infers for mutation handlers. This disables type-checking on all ctx.db.* calls within the handler. Let Convex infer the context type:

-  handler: async ( ctx: any, args: { title?: string; systemPrompt?: string } ): Promise<{ conversationId: string; conversationToken?: string }> => {
+  handler: async ( ctx, args ): Promise<{ conversationId: string; conversationToken?: string }> => {

The args type is already enforced by the args validator, so the explicit annotation is also redundant.

---

559-583: S3 offload is not atomic — partial failures leave orphaned objects.

The three sequential steps (S3 upload → updateS3Reference → trimMessages) are not transactional. If the process fails between steps, you get orphaned S3 objects or a stale reference. Since Convex retries actions and each retry generates a new s3Key (line 559 uses Date.now()), repeated failures will accumulate orphaned S3 objects.

This is acceptable for a background task, but consider adding a cleanup/reconciliation mechanism if S3 costs become a concern.

---

454-470: Context size calculation ignores toolCalls payload.

getContextSize sums content.length and reasoning.length but omits toolCalls, which can contain arbitrarily large tool inputs/outputs. This means the S3 offload threshold may be reached later than expected. Acceptable as a heuristic, but worth noting if tool-heavy conversations grow large.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the file structure and locate the specific file
find . -name "agentAsToolGenerator.ts" -type f

Repository: MikePfunk28/agent_builder_application

Length of output: 113

---

🏁 Script executed:

# Read the file to examine the mentioned lines
wc -l convex/agentAsToolGenerator.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 115

---

🏁 Script executed:

# Get the context around the three mentioned locations
cat -n convex/agentAsToolGenerator.ts | sed -n '20,30p'
cat -n convex/agentAsToolGenerator.ts | sed -n '138,148p'
cat -n convex/agentAsToolGenerator.ts | sed -n '276,286p'

Repository: MikePfunk28/agent_builder_application

Length of output: 1776

---

🏁 Script executed:

# Search for all toolName derivations and usages
rg "replaceAll.*\[.*a-zA-Z0-9_" convex/agentAsToolGenerator.ts -A 2 -B 2

Repository: MikePfunk28/agent_builder_application

Length of output: 331

---

🏁 Script executed:

# Look for generateToolCode and generateCoordinatorCode functions and see how they use the parameters
rg "function generateToolCode|function generateCoordinatorCode" convex/agentAsToolGenerator.ts -A 15

Repository: MikePfunk28/agent_builder_application

Length of output: 1171

---

🏁 Script executed:

# Verify toolName usage in templates more thoroughly
cat -n convex/agentAsToolGenerator.ts | sed -n '25,35p'
cat -n convex/agentAsToolGenerator.ts | sed -n '155,165p'
cat -n convex/agentAsToolGenerator.ts | sed -n '50,85p'

Repository: MikePfunk28/agent_builder_application

Length of output: 1812

---

🏁 Script executed:

# Check if there's any other sanitization or validation for toolName
rg "toolName" convex/agentAsToolGenerator.ts -B 1 -A 1 | head -40

Repository: MikePfunk28/agent_builder_application

Length of output: 1226

---

🏁 Script executed:

# Final check: verify exact Python syntax impact for edge cases
python3 << 'EOF'
# Test what Python would accept/reject
test_cases = [
    "123abc",  # starts with digit
    "",        # empty
    "_",       # single underscore
    "__",      # double underscore
    "tool_name"  # valid
]

for name in test_cases:
    # Check if it's a valid Python identifier
    is_valid = name.isidentifier() if name else False
    print(f"'{name}' -> valid={is_valid}")
EOF

Repository: MikePfunk28/agent_builder_application

Length of output: 185

---

Fix invalid Python identifiers in generated tool names.

toolName can start with a digit (e.g., "123test" → "123test") or collapse to an empty string (e.g., "!!!" → ""), both invalid as Python identifiers in function definitions and imports. Add a validation helper and apply it consistently across all three derivations.

Suggested fix

+function toPythonIdentifier(raw: string): string {
+  let name = raw.replaceAll(/[^a-zA-Z0-9_]/g, "_").toLowerCase();
+  if (!name || /^[0-9]/.test(name)) name = `tool_${name || "agent"}`;
+  return name;
+}
+
-const toolName: string = agent.name.replaceAll(/[^a-zA-Z0-9_]/g, '_').toLowerCase();
+const toolName = toPythonIdentifier(agent.name);

Apply the same helper at lines 142 and 280.

🤖 Prompt for AI Agents

In `@convex/agentAsToolGenerator.ts` around lines 24 - 25, Add a helper (e.g.,
makeValidPythonIdentifier) that takes a string and returns a valid Python
identifier by: replacing non-alphanumeric/underscore chars with '_', ensuring it
does not start with a digit (prefix with '_' if it does), and returning '_' if
the result would be empty; then replace the existing raw derivations with calls
to this helper so that toolName (derived from agent.name) and the two other
generated identifier/filename derivations used in generateToolCode and any
import/function names all use makeValidPythonIdentifier consistently (update
usages around generateToolCode, the initial const toolName, and the other
generation sites currently producing Python identifiers).

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

wc -l convex/automatedAgentBuilder.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 116

---

🏁 Script executed:

cat -n convex/automatedAgentBuilder.ts | sed -n '110,135p'

Repository: MikePfunk28/agent_builder_application

Length of output: 987

---

🏁 Script executed:

cat -n convex/automatedAgentBuilder.ts | sed -n '400,425p'

Repository: MikePfunk28/agent_builder_application

Length of output: 1114

---

🏁 Script executed:

grep -n "agentBuildSessions" convex/schema.ts | head -20

Repository: MikePfunk28/agent_builder_application

Length of output: 121

---

🏁 Script executed:

rg "getBuildSessionInternal" -A 10 convex/automatedAgentBuilder.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 1075

---

🏁 Script executed:

rg "getAuthUserId" convex/

Repository: MikePfunk28/agent_builder_application

Length of output: 10501

---

🏁 Script executed:

grep -n "generateAgentFromSession" -A 15 convex/automatedAgentBuilder.ts | head -40

Repository: MikePfunk28/agent_builder_application

Length of output: 614

---

🏁 Script executed:

sed -n '545,570p' convex/schema.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 1128

---

🏁 Script executed:

grep -n "userId" convex/schema.ts | grep -i "buildSession\|agentBuildSession" -A 5 -B 5

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🏁 Script executed:

sed -n '545,580p' convex/schema.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 1581

---

🏁 Script executed:

head -30 convex/automatedAgentBuilder.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 986

---

🏁 Script executed:

grep -n "export const.*action" convex/automatedAgentBuilder.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 179

---

🏁 Script executed:

sed -n '116,130p' convex/automatedAgentBuilder.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 497

---

🏁 Script executed:

sed -n '406,420p' convex/automatedAgentBuilder.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 614

---

Enforce session ownership verification in both actions.

The processResponse and generateAgentFromSession actions verify authentication but fail to check that the session belongs to the authenticated user. This allows any authenticated user to access or modify sessions belonging to other users by providing a valid session ID.

The agentBuildSessions table includes a userId field that can be used for ownership verification. Replace the getUserIdentity() check with getAuthUserId() and add an ownership verification to each session validation:

Suggested fix

-    const identity = await ctx.auth.getUserIdentity();
-    if ( !identity ) {
+    const userId = await getAuthUserId( ctx );
+    if ( !userId ) {
       throw new Error( "Authentication required" );
     }
@@
-    if ( !session ) {
+    if ( !session || session.userId !== userId ) {
       throw new Error( "Build session not found" );
     }

Apply this same ownership check in both processResponse (lines 116-129) and generateAgentFromSession (lines 406-418).

🤖 Prompt for AI Agents

In `@convex/automatedAgentBuilder.ts` around lines 116 - 129, Replace the loose
identity check in both processResponse and generateAgentFromSession by calling
ctx.auth.getAuthUserId() instead of getUserIdentity(), then after fetching the
session via internal.automatedAgentBuilder.getBuildSessionInternal (using
args.sessionId) verify session.userId === authUserId and throw an authorization
error if it does not match; apply this ownership check in both action handlers
to ensure only the session owner can access or modify the agentBuildSessions
record.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Avoid implicit return in forEach callback (lint error).

The callback returns the Set.add result; Biome flags this. Use a block body to make it explicit.

🧹 Lint-friendly fix

-    if ( tool.requiresPip && tool.pipPackages ) {
-      tool.pipPackages.forEach( ( pkg: string ) => packages.add( pkg ) );
-    }
+    if ( tool.requiresPip && tool.pipPackages ) {
+      tool.pipPackages.forEach( ( pkg: string ) => {
+        packages.add( pkg );
+      } );
+    }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	function generateAgentCoreRequirements( tools: any[] ): string {
	const packages = new Set( [
	"strands-agents>=1.0.0",
	"bedrock-agentcore>=0.1.6",
	"bedrock-agentcore-starter-toolkit>=0.1.25",
	"boto3>=1.28.0",
	"pyjwt>=2.8.0",
	]);
	] );
	// Add tool-specific packages
	tools.forEach(tool => {
	if (tool.requiresPip && tool.pipPackages) {
	tool.pipPackages.forEach((pkg: string) => packages.add(pkg));
	tools.forEach( tool => {
	if ( tool.requiresPip && tool.pipPackages ) {
	tool.pipPackages.forEach( ( pkg: string ) => packages.add( pkg ) );
	}
	});
	} );
	return Array.from(packages).join("\\n");
	return Array.from( packages ).join( String.raw`\n` );
	}
	function generateAgentCoreRequirements( tools: any[] ): string {
	const packages = new Set( [
	"strands-agents>=1.0.0",
	"bedrock-agentcore>=0.1.6",
	"bedrock-agentcore-starter-toolkit>=0.1.25",
	"boto3>=1.28.0",
	"pyjwt>=2.8.0",
	] );
	// Add tool-specific packages
	tools.forEach( tool => {
	if ( tool.requiresPip && tool.pipPackages ) {
	tool.pipPackages.forEach( ( pkg: string ) => {
	packages.add( pkg );
	} );
	}
	} );
	return Array.from( packages ).join( String.raw`\n` );
	}

🧰 Tools

🪛 Biome (2.3.14)

[error] 613-613: This callback passed to forEach() iterable method should not return a value.

Either remove this return or remove the returned value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents

In `@convex/awsDeployment.ts` around lines 601 - 618, The forEach callbacks in
generateAgentCoreRequirements implicitly return the value from packages.add
which triggers the lint error; change both callbacks to use block bodies and
call packages.add without returning (e.g., tools.forEach(tool => { if
(tool.requiresPip && tool.pipPackages) { tool.pipPackages.forEach((pkg: string)
=> { packages.add(pkg); }); } });) so the callbacks have explicit statements and
no implicit return; keep the same symbols (generateAgentCoreRequirements, tools,
tool.requiresPip, tool.pipPackages, packages.add).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Ollama detection is too strict (dot heuristic breaks common IDs).

Many Ollama IDs include dots (e.g., llama3.1:8b). This will misclassify them as non‑Ollama and generate the wrong Dockerfile. Prefer agent.modelProvider or a registry lookup.

✅ Safer detection

-import { sanitizeAgentName } from "./constants";
+import { sanitizeAgentName } from "./constants";
+import { OLLAMA_MODELS } from "./modelRegistry";

...
-  const isOllamaModel = typeof agent.model === "string" && !agent.model.includes( "." );
+  const isOllamaModel =
+    agent.modelProvider === "ollama" ||
+    ( typeof agent.model === "string" && OLLAMA_MODELS[agent.model] );

🤖 Prompt for AI Agents

In `@convex/awsDeployment.ts` around lines 620 - 641, The Ollama detection in
generateAgentCoreDockerfile is too strict (it uses absence of a dot in
agent.model); update the detection to prefer an explicit provider flag and fall
back to safer heuristics: treat the agent as Ollama when agent.modelProvider ===
"ollama" (or a similar explicit property on the agent) and only if that is
absent fall back to checking known Ollama model prefixes (e.g., strings starting
with "ollama" or "llama") or a registry lookup, instead of the current
dot/no-dot test; keep the existing safeModelPattern and modelName sanitization
(and the rest of the Dockerfile generation) but replace the isOllamaModel
assignment with this provider-first logic so Ollama IDs like "llama3.1:8b" are
correctly detected.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find and read the deploymentRouter.ts file
fd -t f "deploymentRouter.ts" | head -5

Repository: MikePfunk28/agent_builder_application

Length of output: 107

---

🏁 Script executed:

# Check file size and read the relevant section
if [ -f "convex/deploymentRouter.ts" ]; then
  wc -l "convex/deploymentRouter.ts"
  echo "=== Lines 90-110 ==="
  sed -n '90,110p' "convex/deploymentRouter.ts"
fi

Repository: MikePfunk28/agent_builder_application

Length of output: 887

---

🏁 Script executed:

# Search for other model validation patterns in the codebase
rg "model.*includes\(|\.test\(" convex/ -A 2 -B 2 --max-count 20

Repository: MikePfunk28/agent_builder_application

Length of output: 17333

---

🏁 Script executed:

# Check for Bedrock-related patterns or documentation
rg -i "bedrock|anthropic|amazon.*model|stability\.cohere" convex/ --max-count 20

Repository: MikePfunk28/agent_builder_application

Length of output: 48272

---

Bedrock model validation is too weak and inconsistent.

includes(".") allows non-Bedrock models like llama3.2:latest. Also, a more comprehensive validation pattern already exists in strandsAgentExecution.ts:

/^(us\.|eu\.|apac\.|global\.)?(anthropic|amazon|meta|mistral|cohere|ai21)\./.test(agent.model)

This accounts for:

• Regional prefixes (us., eu., apac., global.)
• All actual Bedrock providers: anthropic, amazon, meta, mistral, cohere, ai21

Either reuse that pattern here or refactor into a shared validation helper in modelRegistry.ts to keep this logic consistent across the codebase.

🤖 Prompt for AI Agents

In `@convex/deploymentRouter.ts` around lines 96 - 103, The current Bedrock check
in deploymentRouter.ts uses a weak test (agent.model?.includes(".")) and should
be replaced with the robust Bedrock regex used in strandsAgentExecution.ts or
moved to a shared helper; update the validation for AgentCore (the block
returning { success:false, error:"Invalid model format", ... }) to use the regex
/^(us\.|eu\.|apac\.|global\.)?(anthropic|amazon|meta|mistral|cohere|ai21)\./ or,
better, implement a reusable function (e.g., isBedrockModel in modelRegistry.ts)
and call that function here so validation is consistent across
strandsAgentExecution.ts and deploymentRouter.ts.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n convex/strandsAgentExecution.ts | head -200 | tail -50

Repository: MikePfunk28/agent_builder_application

Length of output: 1755

---

🏁 Script executed:

rg "import.*anthropic|from.*anthropic" convex/ -i -A 2

Repository: MikePfunk28/agent_builder_application

Length of output: 501

---

🏁 Script executed:

rg "system.*role|tool.*role" convex/ -i

Repository: MikePfunk28/agent_builder_application

Length of output: 63

---

🌐 Web query:

Anthropic Messages API supported roles documentation

💡 Result:

Anthropic Messages API supports these role values:

• Input messages (messages[].role): "user" or "assistant" (no other roles are accepted). [1]
• System prompt: there is no "system" message role in messages[]; instead, provide system instructions via the top-level system parameter. [2]
• Response (message.role): the generated message’s role is always "assistant". [1]

Docs: [1], [2]

---

🏁 Script executed:

sed -n '195,230p' convex/strandsAgentExecution.ts

Repository: MikePfunk28/agent_builder_application

Length of output: 1407

---

🏁 Script executed:

rg "ConversationMessage|role.*system|role.*tool" convex/ -A 2 -B 2

Repository: MikePfunk28/agent_builder_application

Length of output: 10063

---

🌐 Web query:

AWS Bedrock Anthropic model Messages API role restrictions

💡 Result:

For Anthropic Claude “Messages” style requests on Amazon Bedrock, the key role restrictions are:

• Only two message roles are allowed in messages[]: user and assistant (no system role inside messages). Use the top-level system field for system instructions instead. [1][2]
• Claude is trained for alternating user / assistant turns; if you send consecutive turns of the same role, they may be combined into a single turn. [2]
• If your final input message is an assistant message (used for “prefill”), Claude’s response continues from that assistant content. [1][2]
• Images/documents can only appear in content when the message role is user (i.e., don’t attach images/docs to assistant messages). [3]

Sources: Amazon Bedrock Anthropic Claude Messages parameters [1], Anthropic Messages API docs [2], Bedrock doc note on multimodal content restrictions (images/docs only with user) [3].

---

Filter history roles before calling Anthropic Messages API.

Anthropic Messages API (via AWS Bedrock) only supports user/assistant roles in the messages array; system and tool entries trigger API errors. The ConversationMessage type allows these unsupported roles, but they must be filtered before sending to Bedrock when isAnthropicModel is true. System content is already handled correctly via the top-level system field.

✅ Minimal safe filter

-  for ( const msg of history ) {
-    messages.push( {
-      role: msg.role,
-      content: [{ text: msg.content }],
-    } );
-  }
+  for ( const msg of history ) {
+    if ( msg.role !== "user" && msg.role !== "assistant" ) continue;
+    messages.push( {
+      role: msg.role,
+      content: [{ text: msg.content }],
+    } );
+  }

🤖 Prompt for AI Agents

In `@convex/strandsAgentExecution.ts` around lines 183 - 188, Filter out
non-supported roles before building the messages array when sending to
Anthropic: in the loop that iterates over history (used to populate messages in
strandsAgentExecution.ts), check the isAnthropicModel condition and only push
entries whose msg.role is "user" or "assistant" (skip "system" and "tool"); keep
system content handled separately via the top-level system field and ensure the
code that constructs messages (the messages.push block) only runs for allowed
roles when isAnthropicModel is true.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Ollama models are detected but still routed to Bedrock.

isOllamaModel is computed, but the execution path always calls executeDirectBedrock, which will fail for Ollama IDs. Either implement an Ollama branch or explicitly block Ollama here.

🛡️ Minimal safe guard

-      // Execute with or without dynamic model switching
+      // Execute with or without dynamic model switching
+      if ( isOllamaModel ) {
+        return {
+          success: false,
+          error: "Dynamic model switching currently supports Bedrock models only.",
+        };
+      }
       if ( args.enableModelSwitching === false ) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Model gating: Block freemium users from Bedrock models.
	// Positively detect Ollama models so Bedrock IDs with colons (e.g.
	// "anthropic.claude-haiku-4-5-20251001-v1:0") are not misclassified.
	const isOllamaModel = agent.deploymentType === "ollama"
	|| agent.model.toLowerCase().includes( "ollama" )
	|| ( !agent.deploymentType && !agent.model.includes( "." ) && agent.model.includes( ":" ) );
	const isBedrock = !isOllamaModel;
	if ( isBedrock && !isProviderAllowedForTier( userTier, "bedrock" ) ) {
	return {
	success: false,
	error: "Bedrock models require a Personal subscription ($5/month). " +
	"Use local Ollama models for free, or upgrade in Settings → Billing.",
	};
	}
	// Execute with or without dynamic model switching
	if ( args.enableModelSwitching === false ) {
	return await executeDirectBedrock( ctx, agent, args.message, history );
	} else {
	return await executeWithModelSwitching( ctx, agent, args.message, history, {
	preferCost: args.preferCost,
	preferSpeed: args.preferSpeed,
	preferCapability: args.preferCapability,
	userTier,
	} );
	}
	// Model gating: Block freemium users from Bedrock models.
	// Positively detect Ollama models so Bedrock IDs with colons (e.g.
	// "anthropic.claude-haiku-4-5-20251001-v1:0") are not misclassified.
	const isOllamaModel = agent.deploymentType === "ollama"
	|| agent.model.toLowerCase().includes( "ollama" )
	|| ( !agent.deploymentType && !agent.model.includes( "." ) && agent.model.includes( ":" ) );
	const isBedrock = !isOllamaModel;
	if ( isBedrock && !isProviderAllowedForTier( userTier, "bedrock" ) ) {
	return {
	success: false,
	error: "Bedrock models require a Personal subscription ($5/month). " +
	"Use local Ollama models for free, or upgrade in Settings → Billing.",
	};
	}
	// Execute with or without dynamic model switching
	if ( isOllamaModel ) {
	return {
	success: false,
	error: "Dynamic model switching currently supports Bedrock models only.",
	};
	}
	if ( args.enableModelSwitching === false ) {
	return await executeDirectBedrock( ctx, agent, args.message, history );
	} else {
	return await executeWithModelSwitching( ctx, agent, args.message, history, {
	preferCost: args.preferCost,
	preferSpeed: args.preferSpeed,
	preferCapability: args.preferCapability,
	userTier,
	} );
	}

🤖 Prompt for AI Agents

In `@convex/strandsAgentExecutionDynamic.ts` around lines 136 - 161, The code
computes isOllamaModel but always routes to Bedrock; fix by branching on
isOllamaModel before calling executeDirectBedrock/executeWithModelSwitching: if
isOllamaModel, either call an Ollama-specific executor (e.g.,
executeDirectOllama or executeWithModelSwitching for Ollama) passing the same
parameters (ctx, agent, args.message, history and the preference object), or
explicitly return a safe error blocking Ollama models; ensure you check
args.enableModelSwitching the same way as the Bedrock path and preserve the
preference object (preferCost/preferSpeed/preferCapability/userTier) when
invoking the Ollama execution path so Ollama IDs are not misrouted to
executeDirectBedrock.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the actual file and the specific lines mentioned
find . -name "stripe.ts" -type f | head -5

Repository: MikePfunk28/agent_builder_application

Length of output: 99

---

🏁 Script executed:

# List the repository structure to understand the codebase
git ls-files | head -20

Repository: MikePfunk28/agent_builder_application

Length of output: 742

---

🏁 Script executed:

# Read the stripe.ts file to verify the code at lines 167-206
cat -n ./convex/stripe.ts | sed -n '160,210p'

Repository: MikePfunk28/agent_builder_application

Length of output: 2052

---

🏁 Script executed:

# Search for any references to "identifier" in the codebase
rg "identifier" --type ts

Repository: MikePfunk28/agent_builder_application

Length of output: 227

---

🏁 Script executed:

# Check for any existing retry or idempotency mechanisms in the codebase
rg "idempoten" --type ts

Repository: MikePfunk28/agent_builder_application

Length of output: 167

---

🏁 Script executed:

# Let's look at the http.ts file to see the idempotency context
cat -n ./convex/http.ts | grep -A 10 -B 10 "idempotent"

Repository: MikePfunk28/agent_builder_application

Length of output: 983

---

🏁 Script executed:

# Search for any Stripe-related documentation or comments about meter events
rg "meter|metered" --type ts -B 2 -A 2

Repository: MikePfunk28/agent_builder_application

Length of output: 50393

---

🌐 Web query:

Stripe billing meter events API identifier field idempotency

💡 Result:

For Stripe Billing “meter events”, the field that acts as the event-level idempotency/deduplication identifier is identifier (not the event object’s id).

• identifier (request param): Provide a globally unique value (UUID-like) per real-world usage event. Stripe will enforce uniqueness within a rolling ~24-hour window, primarily to protect against accidental retries/duplicate submissions. If you resend the same identifier in that window, Stripe treats it as a duplicate (v2 returns duplicate_meter_event). [1], [2]
• If you don’t provide identifier, Stripe auto-generates one, which means you lose the ability to safely dedupe by re-sending the same event identifier yourself. [1], [3]
• Stripe also recommends using HTTP idempotency keys to avoid double-reporting due to retries/latency; in this context, Stripe’s docs explicitly tie this to meter events by noting each meter event corresponds to an identifier you can set. [3]

Practical guidance: set both:

1. identifier = <stable UUID per usage event> (event-level dedupe, ~24h), and
2. Idempotency-Key: <stable key per create request> (request-level retry safety). [1], [3]

Sources: Stripe API reference for creating meter events (v1/v2) and “Record usage… / Idempotency” documentation. [1], [2], [3]

---

Add idempotency key to metered usage events to prevent double-billing on retries.

Internal actions can retry on failure; without an identifier field, Stripe will process duplicate meter events and double-charge usage. Pass a deterministic event ID (e.g., derived from execution context) to the identifier field so Stripe deduplicates retries within its ~24-hour window.

🧾 Example (requires passing an eventId)

 export const reportUsage = internalAction( {
   args: {
     stripeCustomerId: v.string(),
     quantity: v.number(),
+    eventId: v.string(),
   },
   handler: async ( _ctx, args ) => {
@@
     await stripe.billing.meterEvents.create( {
       event_name: meter.event_name,
+      identifier: args.eventId,
       payload: {
         stripe_customer_id: args.stripeCustomerId,
         value: String( args.quantity ),
       },
       timestamp: Math.floor( Date.now() / 1000 ),
     } );

🤖 Prompt for AI Agents

In `@convex/stripe.ts` around lines 167 - 206, The reportUsage internalAction
currently creates meterEvents via stripe.billing.meterEvents.create without an
identifier, allowing retries to double-bill; modify the handler in reportUsage
to generate and pass a deterministic identifier (e.g., derive an eventId from
execution context or args like a stable hash of stripeCustomerId + timestamp or
a provided eventId param) into the create call's identifier field before calling
stripe.billing.meterEvents.create so Stripe can deduplicate retries; ensure the
identifier generation is stable across retries for the same logical operation
and add eventId as an arg to reportUsage if needed.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

No usage rollback on cancellation of queued tests.

Usage is incremented optimistically at submission time (here), but cancelTest does not decrement executionsThisMonth when a QUEUED test is cancelled before it ever runs. Users lose an execution unit for work that was never performed. If this is intentional (to prevent gaming), a comment clarifying the design choice would help.

🤖 Prompt for AI Agents

In `@convex/testExecution.ts` around lines 220 - 223, The code currently
increments usage optimistically at submission time by calling
incrementUsageAndReportOverageImpl (in convex/testExecution.ts) for non-Ollama
models, but cancelTest does not reverse that increment when a test in state
QUEUED is cancelled; update cancelTest to detect cancellations of QUEUED tests
and decrement executionsThisMonth (or call a new rollback function mirroring
incrementUsageAndReportOverageImpl) to undo the optimistic charge, referencing
the same modelId/userId metadata and ensuring updateLastTestAt behavior is
preserved, or alternatively add a clear comment in cancelTest explaining the
intentional non-rollback policy if this behavior is deliberate.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

File contains only unused imports with no implementation.

This file imports Agent, tool, LocalChromiumBrowser, think, and readfile but never uses any of them. As-is, it serves no purpose and looks like an accidentally committed stub. Either add the intended security checkup logic or remove the file to avoid dead code in the repository.

Additionally, the two strands_tools imports on lines 2–3 can be consolidated:

♻️ Merge imports (if file is kept)

 from strands import Agent, tool
-from strands_tools import LocalChromiumBrowser
-from strands_tools import think, readfile
+from strands_tools import LocalChromiumBrowser, think, readfile

,

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	from strands import Agent, tool
	from strands_tools import LocalChromiumBrowser
	from strands_tools import think, readfile
	from strands import Agent, tool
	from strands_tools import LocalChromiumBrowser, think, readfile

🤖 Prompt for AI Agents

In `@Security_checkup.py` around lines 1 - 3, This file currently only imports
Agent, tool, LocalChromiumBrowser, think, and readfile but contains no
implementation; either delete Security_checkup.py to remove dead code or
implement the intended security checkup logic—if implementing, create a clear
entrypoint function (e.g., run_security_checkup) that uses Agent and tool for
orchestration and LocalChromiumBrowser/think/readfile where needed, and
consolidate the two strands_tools imports into a single line importing
LocalChromiumBrowser, think, readfile to remove duplication; ensure unused
imports are removed if not used.

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

assistant/main.py (1)

28-34: ⚠️ Potential issue | 🟡 Minor

allow_origins=["*"] with allow_credentials=True is ineffective per the CORS spec.

Browsers reject Access-Control-Allow-Origin: * when credentials are included. Either restrict origins to your actual domain(s) or set allow_credentials=False.

convex/agentBuilderWorkflow.ts (1)

395-457: ⚠️ Potential issue | 🟠 Major

streamWorkflowExecution has no token billing — usage goes unmetered.

The streaming path enforces the gate check (lines 385-393) but never calls incrementUsageAndReportOverage. Since it uses Claude 3.7 Sonnet (line 427) — which is significantly more expensive than Haiku 4.5 — this creates a billing blind spot where users can consume expensive LLM calls without any usage tracking.

Additionally, this path uses the Anthropic API directly (@anthropic-ai/sdk) rather than Bedrock, with a different model (claude-3-7-sonnet-20250219 vs WORKFLOW_MODEL_ID). This means:

1. Costs are billed to your Anthropic account, not AWS
2. The Bedrock gate check is semantically wrong (gating Bedrock access but invoking Anthropic)
3. Token usage is never reported to Stripe

Consider adding billing after the streaming loop completes, or harmonizing the model/provider with the non-streaming path.

convex/mcpClient.ts (1)

576-612: ⚠️ Potential issue | 🔴 Critical

Critical: Payload format incompatibility with default DeepSeek model.

The function defaults to "deepseek.v3-v1:0" (line 576) but constructs an Anthropic-specific payload (lines 599–605) with hardcoded anthropic_version, system, and messages fields. DeepSeek on Bedrock uses a prompt-based format, not the Anthropic Messages API. This will fail at runtime.

Additionally, the response parsing at line 626 assumes Anthropic's content[0].text structure, which also won't match DeepSeek responses.

Branch the payload construction and response parsing by model provider, as done in strandsAgentExecution.ts (lines 210–272), or change the default to an Anthropic model like other functions in the codebase.

🤖 Fix all issues with AI agents

In `@assistant/main.py`:
- Around line 139-141: The exception handler currently logs and returns str(e)
to the client; change it to log the full error details server-side using
logger.exception or logger.error with the exception, but return a generic
message to the client when raising HTTPException (e.g., "Internal server error"
or "An internal error occurred") instead of str(e); update the except block
around the assistant invocation (the except Exception as e handler that calls
logger.error and raises HTTPException) to use logger.exception(e) or
logger.error(..., exc_info=True) and raise HTTPException(status_code=500,
detail="Internal server error") so internal details are not leaked.

In `@convex/automatedAgentBuilder.ts`:
- Around line 158-165: The code casts gateResult.userId to any before passing
into incrementUsageAndReportOverage, masking a type mismatch: gateResult.userId
is a string but the mutation expects Id<"users">; fix by ensuring the original
Convex Id is preserved and passed instead of the stringified value—update the
gate producer (bedrockGate) to return the Convex Id type or add a field carrying
the original Id, then in automatedAgentBuilder replace the userId:
gateResult.userId as any with the preserved Id (e.g., userId:
gateResult.userIdOriginal or userId: gateResult.userId if you changed its type)
when calling
ctx.runMutation(internal.stripeMutations.incrementUsageAndReportOverage).

In `@convex/http.ts`:
- Around line 519-534: In the "charge.dispute.created" handler the logic sets
disputeCustomer to undefined when event.data.object.charge is only an ID string,
which silently skips restricting the account; update the handler (case
"charge.dispute.created") to resolve the customer when charge is an ID by
fetching the charge from Stripe (or at minimum emit a warning log) and then call
internal.stripeMutations.restrictAccountForDispute with the resolved
stripeCustomerId; ensure you reference the disputeCustomer variable and the
ctx.runMutation(internal.stripeMutations.restrictAccountForDispute, ...) call so
the new flow either fetches the charge via the Stripe client to get
charge.customer or logs a warning when resolution fails.

In `@convex/modelRegistry.ts`:
- Around line 884-903: resolveBedrockModelId currently reads
process.env.AGENT_BUILDER_MODEL_ID (unavailable in Convex runtime), so the env
fallback is unreliable; change the function signature to accept an optional
fallback/default parameter (e.g., fallbackModelId?: string) and use that (or the
hardcoded "deepseek.v3-v1:0" if the parameter is undefined) instead of reading
process.env inside resolveBedrockModelId, and update all callers
(mutations/queries and server-only code) to pass the environment value where
available (process.env.AGENT_BUILDER_MODEL_ID) from Node-only entrypoints so
runtime code never accesses process.env directly.
- Around line 97-109: The entry for the Bedrock model uses an invalid ID with a
trailing ":0"—update the object key and its id field from
"anthropic.claude-opus-4-6-v1:0" to "anthropic.claude-opus-4-6-v1" (i.e., change
both the property name and the id value inside the object) so the model ID used
by the registry (refer to the object keyed by anthropic.claude-opus-4-6-v1:0 and
its id property) matches the AWS Bedrock model name without the ":0" suffix.

In `@convex/promptChainExecutor.ts`:
- Around line 44-66: The current gating/billing logic in executePromptChain and
executeParallelPrompts only inspects the first Bedrock model found (gateModelId)
which misses other distinct Bedrock models; update both functions to collect all
distinct bedrock model ids from args.prompts (inspect p.model for "bedrock:"
prefix), then for each distinct model call requireBedrockAccess(ctx, modelId,
...) and aggregate results—if any requireBedrockAccess returns allowed=false
return its reason; for billing, stop using a single gateModelId and instead
compute token costs per prompt using each prompt's actual bedrock model id (or
choose the most expensive model if you prefer) so charges reflect the correct
model-specific pricing.
- Around line 365-415: invokeBedrock is always building an Anthropic-style
payload (anthropic_version, messages, content) which breaks for non-Anthropic
Bedrock models; update invokeBedrock to branch on the model identifier (e.g.,
inspect the model string like model.startsWith("anthropic") or maintain a
mapping for Anthropic vs other providers) and construct the InvokeModelCommand
body accordingly (Anthropic: keep anthropic_version/messages/content;
non-Anthropic: use the generic Bedrock schema the target model expects, e.g.,
input/parameters or a text prompt field and appropriate max_tokens key), then
parse responseBody differently per branch before calling
extractTokenUsage/estimateTokenUsage so token extraction matches the
model-specific response shape; keep function name invokeBedrock, variables
command/responseBody/outputText and reuse extractTokenUsage/estimateTokenUsage
as shown.

In `@convex/strandsAgentExecution.ts`:
- Around line 186-192: Replace the non-null assertions on
process.env.AWS_ACCESS_KEY_ID and process.env.AWS_SECRET_ACCESS_KEY used when
constructing the BedrockRuntimeClient with explicit validation: check that these
environment variables (and optionally AWS_REGION) are set before creating the
client (referencing BedrockRuntimeClient and the env names AWS_ACCESS_KEY_ID,
AWS_SECRET_ACCESS_KEY, AWS_REGION), and if any are missing throw a clear,
descriptive error (similar to the checks in mcpClient.ts and
automatedAgentBuilder.ts) so undefined credentials are not passed to the SDK.

In `@convex/strandsAgentExecutionDynamic.ts`:
- Around line 165-173: The billing currently passes agent.model to
internal.stripeMutations.incrementUsageAndReportOverage which misattributes
costs when executeWithModelSwitching chose a different model; change the modelId
argument to use the actually-used model from result.metadata.model (or
result.metadata?.model) when present, falling back to agent.model if not, and
keep passing result.tokenUsage.inputTokens/outputTokens to ensure correct
metering (identify this change around the ctx.runMutation call to
internal.stripeMutations.incrementUsageAndReportOverage).
- Around line 283-294: The payload for Anthropic models currently always
includes the thinking block which breaks older Claude variants; update the logic
around isAnthropicModel so the thinking object is only added when the model
identifier explicitly supports extended thinking (e.g., check the model string
for supported versions like "claude-3.5-sonnet" / "claude-3.5" or maintain a
small allowlist of supported model names) before adding thinking to payload;
leave agent.systemPrompt, messages, and other fields unchanged and ensure models
like "claude-3" / "claude-instant" do not receive the thinking property to avoid
API errors.

In `@convex/stripeMutations.ts`:
- Around line 95-108: The current fire-and-forget call using
ctx.scheduler.runAfter(0, internalStripe.reportUsage, ...) after db.patch can
silently drop overage reports if scheduling fails; update the logic around
ctx.scheduler.runAfter and internalStripe.reportUsage to ensure failures are
observable and retried: wrap the scheduling in a small retry/backoff loop or use
a retry helper so scheduling is attempted multiple times before giving up, and
emit a clear warning log (via processLogger or ctx.logger) including
user.stripeCustomerId, overageUnits, and the caught error when scheduling
ultimately fails; apply this change where user.tier === "personal" and
user.stripeCustomerId are checked so overageUnits and the scheduling call are
covered.

In `@convex/tools.ts`:
- Line 723: The schema accepts an inputData parameter (inputData:
v.optional(v.record(v.string(), v.any()))) but the handler never uses it; either
remove the field or apply its values to template interpolation. Fix by locating
the code in convex/tools.ts that reads the template string (look for variables
named template, templateString, or the function that builds the output) and
implement substitution using inputData keys (e.g., replace {{key}} or chosen
placeholder format with inputData[key]), or remove inputData from the schema and
callers; ensure the handler references inputData when producing the final output
so passed values take effect.
- Around line 411-419: The code can crash when answers is empty (e.g., numPaths
=== 0); update the aggregation in the block that builds voteCounts /
sortedAnswers to handle the empty case before indexing sortedAnswers[0]: check
if answers.length === 0 (or Object.keys(voteCounts).length === 0) and return or
set a safe default (for example set finalAnswer to an empty string or null and
confidence to 0, or throw a clear error) instead of accessing sortedAnswers[0];
modify the variables finalAnswer and confidence assignment to use this guard so
functions that use voteCounts, sortedAnswers, finalAnswer, confidence, and
numPaths do not throw.

🧹 Nitpick comments (12)

convex/lib/dynamicModelSwitching.ts (1)

309-339: defaultModelId parameter is accepted but never used.

Line 316 accepts defaultModelId: string but the wrapper always uses decision.selectedModel.modelId (line 333). Either remove the unused parameter or use it as a fallback when the decision engine can't determine a model.

convex/stripeMutations.ts (1)

39-48: Loose any types on ctx and userId weaken type safety.

incrementUsageAndReportOverageImpl is the billing source of truth — bugs here have financial impact. Consider typing userId as Id<"users"> and ctx with the actual Convex mutation context type to catch misuse at compile time.

Suggested improvement

+import type { Id } from "./_generated/dataModel";
+import type { MutationCtx } from "./_generated/server";
+
 export async function incrementUsageAndReportOverageImpl(
-  ctx: { db: any; scheduler: any },
-  userId: any,
+  ctx: Pick<MutationCtx, "db" | "scheduler">,
+  userId: Id<"users">,
   options?: {

convex/interleavedReasoning.ts (1)

148-156: gateResult.userId as any bypasses type safety on the billing path.

gateResult.userId is String(userId) (a plain string), but incrementUsageAndReportOverage expects v.id("users"). While this works at runtime because Convex IDs are strings, the as any cast hides the type boundary. If the Convex validator is tightened later, this will break silently.

convex/agentBuilderWorkflow.ts (1)

317-356: Redundant gate check in executeCompleteWorkflow — each stage already gates.

executeCompleteWorkflow calls requireBedrockAccess (lines 318-326) and then delegates to executeWorkflowStage via ctx.runAction (line 345), which runs its own gate check. This results in N+1 auth lookups (1 here + 1 per stage). Not a correctness issue, but it adds unnecessary latency and DB reads for a 6-stage workflow.

convex/schema.ts (1)

674-687: Rate limit cap enforced by application code, not schema—lacks dedicated tests.

The 200-entry cap documented at lines 675-677 is correctly enforced in rateLimiter.ts via .slice(-MAX_RATE_LIMIT_REQUESTS) at two write points (lines 153 and 229). The implementation prevents unbounded growth as intended. However, no dedicated unit tests exist for the trim logic; adding tests would guard against regression if the trim is modified.

convex/lib/bedrockGate.ts (1)

198-211: Dead branch: overageAllowed is always false when !limitResult.allowed.

Looking at checkExecutionLimit in tierConfig.ts, when overage is allowed the function returns { allowed: true, ... }. The gate here only enters this block when !limitResult.allowed, meaning overageAllowed is always false. The "Overage billing applies at $0.05/unit" message on line 206 is unreachable.

This isn't a bug since the fallback message is correct, but the ternary is misleading. Consider simplifying.

convex/lib/tokenBilling.ts (1)

212-228: Consider replacing require() with a synchronous lookup to satisfy ESLint.

The require() on line 217 triggers @typescript-eslint/no-require-imports. Since getModelPricing needs to be synchronous, consider having callers pass the BEDROCK_MODELS registry in, or caching it at module level via a top-level import (the "circular dependency" concern could be addressed by having modelRegistry.ts export only the data, which it already does).

♻️ Alternative: top-level import with lazy access

+// Import at module level — modelRegistry.ts is a pure data file,
+// so no circular dependency risk at runtime.
+import { BEDROCK_MODELS } from "../modelRegistry";
+
 function getModelPricing( modelId: string ): { input: number; output: number } {
-  try {
-    // eslint-disable-next-line `@typescript-eslint/no-var-requires`
-    const { BEDROCK_MODELS } = require( "../modelRegistry" );
-    const model = BEDROCK_MODELS[modelId];
-    if ( model?.costPer1MTokens ) {
-      return model.costPer1MTokens;
-    }
-  } catch {
-    // Module not available (e.g., in unit tests) — use fallback
-  }
+  const model = BEDROCK_MODELS?.[modelId];
+  if ( model?.costPer1MTokens ) {
+    return model.costPer1MTokens;
+  }
   return { input: 1.0, output: 5.0 };
 }

convex/promptChainExecutor.ts (1)

329-360: Ollama localhost fallback will silently fail in Convex cloud runtime.

While OLLAMA_ENDPOINT env var support was added (addressing the prior review), the default http://127.0.0.1:11434 will still fail silently in production since localhost targets the Convex server, not the user's machine. Consider logging a warning or throwing when the env var is unset to avoid confusing timeouts.

convex/strandsAgentExecutionDynamic.ts (2)

248-254: AWS credentials fail silently with non-null assertions.

Lines 251–252 use ! on process.env.AWS_ACCESS_KEY_ID and process.env.AWS_SECRET_ACCESS_KEY. If either is unset, the value is undefined and the SDK will throw a confusing error. The codebase already has a better pattern in convex/lib/messageExecutor.ts (lines 30–33) that validates both are set.

🛡️ Suggested validation

+  const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+  if ( !accessKeyId || !secretAccessKey ) {
+    throw new Error( "AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY must both be set" );
+  }
+
   const client = new BedrockRuntimeClient( {
     region: process.env.AWS_REGION || "us-east-1",
     credentials: {
-      accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
-      secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
+      accessKeyId,
+      secretAccessKey,
     },
   } );

---

374-385: Redundant double-decode of response.body in the fallback branch.

Line 336 already decodes response.body into responseBody. The fallback at line 376 decodes response.body again and re-parses the JSON. Use the already-parsed responseBody instead.

♻️ Proposed fix

   } else {
-    // Fallback: try to extract text from any string field in the response
-    const raw = new TextDecoder().decode( response.body );
-    console.warn( `Unrecognized Bedrock response format for model ${modelId}. Raw preview: ${raw.slice( 0, 200 )}` );
-    // Attempt to use the raw body as text if it looks like a plain string
-    try {
-      const parsed = JSON.parse( raw );
-      content = typeof parsed === "string" ? parsed : JSON.stringify( parsed );
-    } catch {
-      content = raw;
-    }
+    // Fallback: stringify the already-parsed response
+    console.warn( `Unrecognized Bedrock response format for model ${modelId}. Keys: ${Object.keys( responseBody ).join( ", " )}` );
+    content = JSON.stringify( responseBody );
   }

convex/tools.ts (2)

116-116: substr is deprecated — use substring instead.

String.prototype.substr is deprecated per the ECMAScript spec. Replace with substring.

-    const handoffId = `handoff_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
+    const handoffId = `handoff_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`;

---

362-373: Extract the duplicated Bedrock gate check into a shared helper.

The exact same ~12-line pattern (Ollama detection → dynamic import → requireBedrockAccess → throw on deny) is repeated in all five reasoning tools. A helper would reduce duplication and ensure consistent behavior if the gating logic evolves.

♻️ Suggested helper

async function requireBedrockGate(
  ctx: any,
  model: string
): Promise<{ userId: string; tier: string } | undefined> {
  const isOllama = model.includes(":") && !model.includes(".");
  if (isOllama) return undefined;
  const { requireBedrockAccess } = await import("./lib/bedrockGate");
  const gate = await requireBedrockAccess(
    ctx, model,
    async (lookupArgs) => ctx.runQuery(internal.users.getInternal, lookupArgs),
  );
  if (!gate.allowed) throw new Error(gate.reason);
  return gate as { userId: string; tier: string };
}

Then each tool simply calls:

const gateResult = await requireBedrockGate(ctx, args.model);

Also applies to: 442-454, 533-545, 631-643, 726-738

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Raw exception detail forwarded to the client may leak internals.

str(e) can contain AWS SDK errors, stack info, or account identifiers. Return a generic message and log the detail server-side.

Suggested fix

     except Exception as e:
         logger.error(f"Error invoking assistant: {str(e)}")
-        raise HTTPException(status_code=500, detail=str(e))
+        raise HTTPException(status_code=500, detail="Internal error processing request")

🧰 Tools

🪛 Ruff (0.15.0)

[warning] 139-139: Do not catch blind exception: Exception

(BLE001)

---

[warning] 140-140: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

[warning] 140-140: Use explicit conversion flag

Replace with conversion flag

(RUF010)

---

[warning] 141-141: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents

In `@assistant/main.py` around lines 139 - 141, The exception handler currently
logs and returns str(e) to the client; change it to log the full error details
server-side using logger.exception or logger.error with the exception, but
return a generic message to the client when raising HTTPException (e.g.,
"Internal server error" or "An internal error occurred") instead of str(e);
update the except block around the assistant invocation (the except Exception as
e handler that calls logger.error and raises HTTPException) to use
logger.exception(e) or logger.error(..., exc_info=True) and raise
HTTPException(status_code=500, detail="Internal server error") so internal
details are not leaked.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

userId type mismatch: string cast to any for Id<"users"> parameter.

gateResult.userId is typed as string (from bedrockGate.ts line 85: String(userId)), but incrementUsageAndReportOverage likely expects Id<"users">. The as any cast on line 160 silences the type error but could cause runtime issues if the mutation validates the ID type.

Consider passing the original Convex Id<"users"> instead of re-casting the stringified version.

🤖 Prompt for AI Agents

In `@convex/automatedAgentBuilder.ts` around lines 158 - 165, The code casts
gateResult.userId to any before passing into incrementUsageAndReportOverage,
masking a type mismatch: gateResult.userId is a string but the mutation expects
Id<"users">; fix by ensuring the original Convex Id is preserved and passed
instead of the stringified value—update the gate producer (bedrockGate) to
return the Convex Id type or add a field carrying the original Id, then in
automatedAgentBuilder replace the userId: gateResult.userId as any with the
preserved Id (e.g., userId: gateResult.userIdOriginal or userId:
gateResult.userId if you changed its type) when calling
ctx.runMutation(internal.stripeMutations.incrementUsageAndReportOverage).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Dispute handler silently skips cases where customer can't be resolved.

Line 527 returns undefined when only a charge ID string is present (no expanded customer). This means disputes on charges without an inline customer reference will leave the account unrestricted. Consider fetching the charge via the Stripe API to resolve the customer, or at minimum log a warning so these cases surface in monitoring.

Suggested improvement

               ? disputeData.customer
               : typeof disputeData.charge === "string"
-                ? undefined // charge ID only — need to look up; skip for now
+                ? undefined // TODO: look up charge to resolve customer
                 : disputeData.charge?.customer;
+          if ( !disputeCustomer ) {
+            console.warn(`Stripe webhook: Could not resolve customer for dispute ${event.id}. Manual review required.`);
+          }
           if ( disputeCustomer ) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	case "charge.dispute.created": {
	// Stripe Dispute: customer is on the linked charge, not top-level.
	// Use the raw event data which includes customer as a string.
	const disputeData = event.data.object as any;
	const disputeCustomer: string | undefined =
	typeof disputeData.customer === "string"
	? disputeData.customer
	: typeof disputeData.charge === "string"
	? undefined // charge ID only — need to look up; skip for now
	: disputeData.charge?.customer;
	if ( disputeCustomer ) {
	await ctx.runMutation(internal.stripeMutations.restrictAccountForDispute, {
	stripeCustomerId: disputeCustomer,
	});
	}
	break;
	case "charge.dispute.created": {
	// Stripe Dispute: customer is on the linked charge, not top-level.
	// Use the raw event data which includes customer as a string.
	const disputeData = event.data.object as any;
	const disputeCustomer: string | undefined =
	typeof disputeData.customer === "string"
	? disputeData.customer
	: typeof disputeData.charge === "string"
	? undefined // TODO: look up charge to resolve customer
	: disputeData.charge?.customer;
	if ( !disputeCustomer ) {
	console.warn(`Stripe webhook: Could not resolve customer for dispute ${event.id}. Manual review required.`);
	}
	if ( disputeCustomer ) {
	await ctx.runMutation(internal.stripeMutations.restrictAccountForDispute, {
	stripeCustomerId: disputeCustomer,
	});
	}
	break;

🤖 Prompt for AI Agents

In `@convex/http.ts` around lines 519 - 534, In the "charge.dispute.created"
handler the logic sets disputeCustomer to undefined when
event.data.object.charge is only an ID string, which silently skips restricting
the account; update the handler (case "charge.dispute.created") to resolve the
customer when charge is an ID by fetching the charge from Stripe (or at minimum
emit a warning log) and then call
internal.stripeMutations.restrictAccountForDispute with the resolved
stripeCustomerId; ensure you reference the disputeCustomer variable and the
ctx.runMutation(internal.stripeMutations.restrictAccountForDispute, ...) call so
the new flow either fetches the charge via the Stripe client to get
charge.customer or logs a warning when resolution fails.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

AWS Bedrock Claude 4.6 Opus model ID anthropic.claude-opus-4-6-v1

💡 Result:

• Amazon Bedrock base model ID for Claude Opus 4.6: anthropic.claude-opus-4-6-v1 [1][2]
• Bedrock inference profile ID (US): us.anthropic.claude-opus-4-6-v1 (used when calling an inference profile instead of the base model ID) [3]
• AWS announced Claude Opus 4.6 availability in Bedrock on Feb 5, 2026. [4]

Sources:
[1] AWS Bedrock supported models list (Claude Opus 4.6) [2] Anthropic “Claude on Amazon Bedrock” model ID table [3] AWS Bedrock inference profiles support page [4] AWS “What’s New” announcement (Feb 5, 2026)

---

Remove the :0 suffix from the Bedrock model ID.

The model ID anthropic.claude-opus-4-6-v1:0 is invalid. AWS Bedrock uses anthropic.claude-opus-4-6-v1 (without the :0 suffix). This will cause runtime failures when invoking the model.

Correction needed:

- "anthropic.claude-opus-4-6-v1:0": {
+ "anthropic.claude-opus-4-6-v1": {
-   id: "anthropic.claude-opus-4-6-v1:0",
+   id: "anthropic.claude-opus-4-6-v1",

🤖 Prompt for AI Agents

In `@convex/modelRegistry.ts` around lines 97 - 109, The entry for the Bedrock
model uses an invalid ID with a trailing ":0"—update the object key and its id
field from "anthropic.claude-opus-4-6-v1:0" to "anthropic.claude-opus-4-6-v1"
(i.e., change both the property name and the id value inside the object) so the
model ID used by the registry (refer to the object keyed by
anthropic.claude-opus-4-6-v1:0 and its id property) matches the AWS Bedrock
model name without the ":0" suffix.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

process.env fallback on Line 902 is unreliable in Convex's standard runtime.

resolveBedrockModelId is a plain exported function that can be called from mutations/queries (non-Node context), where process.env is unavailable. The fallback to "deepseek.v3-v1:0" will always apply in those contexts regardless of the env var being set. Consider accepting a fallback parameter or documenting this constraint.

🤖 Prompt for AI Agents

In `@convex/modelRegistry.ts` around lines 884 - 903, resolveBedrockModelId
currently reads process.env.AGENT_BUILDER_MODEL_ID (unavailable in Convex
runtime), so the env fallback is unreliable; change the function signature to
accept an optional fallback/default parameter (e.g., fallbackModelId?: string)
and use that (or the hardcoded "deepseek.v3-v1:0" if the parameter is undefined)
instead of reading process.env inside resolveBedrockModelId, and update all
callers (mutations/queries and server-only code) to pass the environment value
where available (process.env.AGENT_BUILDER_MODEL_ID) from Node-only entrypoints
so runtime code never accesses process.env directly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Token metering bills against agent.model instead of the actually-used model.

When model switching is enabled, executeWithModelSwitching may select a different model (stored in result.metadata.model), but line 169 always passes agent.model as the modelId for billing. This misattributes token costs to the wrong model.

🐛 Proposed fix

       if ( result.tokenUsage ) {
         await ctx.runMutation( internal.stripeMutations.incrementUsageAndReportOverage, {
           userId: agent.createdBy,
-          modelId: agent.model,
+          modelId: result.metadata?.model ?? agent.model,
           inputTokens: result.tokenUsage.inputTokens,
           outputTokens: result.tokenUsage.outputTokens,
         } );
       }

🤖 Prompt for AI Agents

In `@convex/strandsAgentExecutionDynamic.ts` around lines 165 - 173, The billing
currently passes agent.model to
internal.stripeMutations.incrementUsageAndReportOverage which misattributes
costs when executeWithModelSwitching chose a different model; change the modelId
argument to use the actually-used model from result.metadata.model (or
result.metadata?.model) when present, falling back to agent.model if not, and
keep passing result.tokenUsage.inputTokens/outputTokens to ensure correct
metering (identify this change around the ctx.runMutation call to
internal.stripeMutations.incrementUsageAndReportOverage).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

thinking block is unconditionally included for all Anthropic models — older models will reject it.

isAnthropicModel matches any model containing "anthropic" or "claude", but extended thinking is only supported on Claude 3.5 Sonnet and newer. Sending thinking: { type: "enabled" } to Claude 3 Haiku or Claude Instant will cause an API error. Gate this on specific model versions that support it.

🛡️ Proposed fix sketch

+  const supportsThinking = isAnthropicModel && (
+    modelId.includes("claude-3-5") || modelId.includes("claude-4") || modelId.includes("claude-sonnet-4")
+  );
+
   if ( isAnthropicModel ) {
     payload = {
       anthropic_version: "bedrock-2023-05-31",
       max_tokens: 4096,
       system: agent.systemPrompt,
       messages: messages,
-      temperature: 1,
-      thinking: {
+      temperature: supportsThinking ? 1 : 0.7,
+      ...(supportsThinking && { thinking: {
         type: "enabled",
         budget_tokens: 3000,
-      },
+      } }),
     };

🤖 Prompt for AI Agents

In `@convex/strandsAgentExecutionDynamic.ts` around lines 283 - 294, The payload
for Anthropic models currently always includes the thinking block which breaks
older Claude variants; update the logic around isAnthropicModel so the thinking
object is only added when the model identifier explicitly supports extended
thinking (e.g., check the model string for supported versions like
"claude-3.5-sonnet" / "claude-3.5" or maintain a small allowlist of supported
model names) before adding thinking to payload; leave agent.systemPrompt,
messages, and other fields unchanged and ensure models like "claude-3" /
"claude-instant" do not receive the thinking property to avoid API errors.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fire-and-forget overage reporting may silently lose usage events.

ctx.scheduler.runAfter(0, ...) on line 103 is non-transactional with the preceding db.patch. If the scheduled action fails (e.g., Stripe is down), the local counter is incremented but overage is never reported to Stripe — leading to under-billing. Consider adding a retry mechanism or at minimum logging a warning when this path is reached so it can be monitored.

🤖 Prompt for AI Agents

In `@convex/stripeMutations.ts` around lines 95 - 108, The current fire-and-forget
call using ctx.scheduler.runAfter(0, internalStripe.reportUsage, ...) after
db.patch can silently drop overage reports if scheduling fails; update the logic
around ctx.scheduler.runAfter and internalStripe.reportUsage to ensure failures
are observable and retried: wrap the scheduling in a small retry/backoff loop or
use a retry helper so scheduling is attempted multiple times before giving up,
and emit a clear warning log (via processLogger or ctx.logger) including
user.stripeCustomerId, overageUnits, and the caught error when scheduling
ultimately fails; apply this change where user.tier === "personal" and
user.stripeCustomerId are checked so overageUnits and the scheduling call are
covered.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

sortedAnswers[0] crashes with TypeError if numPaths is 0 or all answers are empty.

If a caller passes numPaths: 0, the loop at line 381 never executes, answers is empty, and sortedAnswers[0] is undefined — causing an unhandled TypeError at line 418.

🐛 Proposed fix

     const sortedAnswers = Object.entries(voteCounts).sort((a, b) => b[1] - a[1]);
-    const finalAnswer = sortedAnswers[0][0];
-    const confidence = sortedAnswers[0][1] / numPaths;
+    const finalAnswer = sortedAnswers.length > 0 ? sortedAnswers[0][0] : "";
+    const confidence = sortedAnswers.length > 0 && numPaths > 0
+      ? sortedAnswers[0][1] / numPaths
+      : 0;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Count votes
	const voteCounts: Record<string, number> = {};
	answers.forEach((answer) => {
	voteCounts[answer] = (voteCounts[answer] || 0) + 1;
	});
	const sortedAnswers = Object.entries(voteCounts).sort((a, b) => b[1] - a[1]);
	const finalAnswer = sortedAnswers[0][0];
	const confidence = sortedAnswers[0][1] / numPaths;
	// Count votes
	const voteCounts: Record<string, number> = {};
	answers.forEach((answer) => {
	voteCounts[answer] = (voteCounts[answer] || 0) + 1;
	});
	const sortedAnswers = Object.entries(voteCounts).sort((a, b) => b[1] - a[1]);
	const finalAnswer = sortedAnswers.length > 0 ? sortedAnswers[0][0] : "";
	const confidence = sortedAnswers.length > 0 && numPaths > 0
	? sortedAnswers[0][1] / numPaths
	: 0;

🤖 Prompt for AI Agents

In `@convex/tools.ts` around lines 411 - 419, The code can crash when answers is
empty (e.g., numPaths === 0); update the aggregation in the block that builds
voteCounts / sortedAnswers to handle the empty case before indexing
sortedAnswers[0]: check if answers.length === 0 (or
Object.keys(voteCounts).length === 0) and return or set a safe default (for
example set finalAnswer to an empty string or null and confidence to 0, or throw
a clear error) instead of accessing sortedAnswers[0]; modify the variables
finalAnswer and confidence assignment to use this guard so functions that use
voteCounts, sortedAnswers, finalAnswer, confidence, and numPaths do not throw.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

inputData argument is accepted but never used.

The inputData parameter (line 723) is defined in the schema and accepted from callers, but the handler never references it. Callers expecting template variable substitution will get no effect. Either remove it or implement template interpolation.

🤖 Prompt for AI Agents

In `@convex/tools.ts` at line 723, The schema accepts an inputData parameter
(inputData: v.optional(v.record(v.string(), v.any()))) but the handler never
uses it; either remove the field or apply its values to template interpolation.
Fix by locating the code in convex/tools.ts that reads the template string (look
for variables named template, templateString, or the function that builds the
output) and implement substitution using inputData keys (e.g., replace {{key}}
or chosen placeholder format with inputData[key]), or remove inputData from the
schema and callers; ensure the handler references inputData when producing the
final output so passed values take effect.