Skip to content

feat(chat-deploy)#277

Merged
emir-karabeg merged 18 commits intomainfrom
feat/chat-deploy
Apr 27, 2025
Merged

feat(chat-deploy)#277
emir-karabeg merged 18 commits intomainfrom
feat/chat-deploy

Conversation

@emir-karabeg
Copy link
Copy Markdown
Collaborator

Description

Deploy a shareable chat.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Security enhancement
  • Performance improvement
  • Code refactoring (no functional changes)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (npm test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

Additional Information:

Any additional information, configuration or data that might be necessary to reproduce the issue or use the feature.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 27, 2025 2:05am
sim ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 27, 2025 2:05am

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mrge found 32 issues across 16 files. View them in mrge.io

"description" text,
"is_active" boolean DEFAULT true NOT NULL,
"customizations" json DEFAULT '{}',
"auth_type" text DEFAULT 'public' NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auth_type column lacks a CHECK constraint to restrict values to only valid options

--> statement-breakpoint
ALTER TABLE "chatbot" ADD CONSTRAINT "chatbot_workflow_id_workflow_id_fk" FOREIGN KEY ("workflow_id") REFERENCES "public"."workflow"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "chatbot" ADD CONSTRAINT "chatbot_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
CREATE UNIQUE INDEX "subdomain_idx" ON "chatbot" USING btree ("subdomain"); No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing indexes on foreign keys workflow_id and user_id

return
}

const checkChatbotDeployment = async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API polling effect lacks cleanup that could lead to race conditions if component unmounts during API call

}

// This endpoint returns information about the chatbot
export async function GET(request: NextRequest, { params }: { params: Promise<{ subdomain: string }> }) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect parameter typing: params should not be a Promise in Next.js route handlers

const apiKey = apiKeyResult[0].key

// Forward the message to the workflow execution endpoint
const response = await fetch(`${process.env.NEXT_PUBLIC_APP_URL}/api/workflows/${deployment.workflowId}/execute`, {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using NEXT_PUBLIC_ environment variable on the server side

Comment thread sim/app/api/chatbot/utils.ts Outdated
}

// Handle OPTIONS requests for CORS preflight
export async function OPTIONS(request: NextRequest) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Enforce Utility Function Placement by Usage Pattern

  Utility function placed incorrectly - only used in one location

Comment thread sim/app/api/chatbot/utils.ts Outdated
}

// Validate authentication for chatbot access
export async function validateChatbotAuth(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Enforce Utility Function Placement by Usage Pattern

  Utility function placed incorrectly - only used in one location

"is_active" boolean DEFAULT true NOT NULL,
"customizations" json DEFAULT '{}',
"auth_type" text DEFAULT 'public' NOT NULL,
"password" text,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Prevent Default Empty Values for Required Security Parameters

  Security-related password field implicitly defaults to NULL which could create authentication vulnerabilities

Comment thread sim/app/api/chatbot/utils.ts Outdated
request: NextRequest,
parsedBody?: any
): Promise<{ authorized: boolean, error?: string }> {
const authType = deployment.authType || 'public'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Prevent Default Empty Values for Required Security Parameters

  Authentication type defaults to 'public' when deployment.authType is undefined/null/empty, potentially allowing unauthorized access.

welcomeMessage: z.string(),
}),
authType: z.enum(["public", "password", "email"]),
password: z.string().optional(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Prevent Default Empty Values for Required Security Parameters

  Password validation schema allows optional password when authType is 'password'

@emir-karabeg emir-karabeg marked this pull request as draft April 18, 2025 08:42
@emir-karabeg emir-karabeg force-pushed the main branch 4 times, most recently from df6ba40 to 02657ec Compare April 20, 2025 19:40
@waleedlatif1 waleedlatif1 force-pushed the main branch 2 times, most recently from d758819 to 185977b Compare April 23, 2025 03:28
waleedlatif1 and others added 18 commits April 26, 2025 18:57
… access it at subdomain. still need to fix the actual chat request to match the same format as the chat in the panel
… a response from subdomain. need to fix UI + form submission of deploy modal but the core functionality works
…the ability to delete/view chat deployment, and test emails/email domain
@emir-karabeg emir-karabeg marked this pull request as ready for review April 27, 2025 02:16
@emir-karabeg emir-karabeg merged commit fc101c3 into main Apr 27, 2025
6 checks passed
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mrge found 47 issues across 38 files. View them in mrge.io

@@ -0,0 +1,128 @@
'use client'

import { useState } from 'react'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import: 'useState' is imported but never used in the component

'use client'

import { useState } from 'react'
import { Info, Loader2 } from 'lucide-react'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import: 'Info' icon is imported but never used in the component

interface DeploymentInfoProps {
isLoading: boolean
deploymentInfo: {
isDeployed: boolean
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused property: 'isDeployed' is defined in interface but not used

</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>Cancel</AlertDialogCancel>
<AlertDialogAction
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing loading state in dialog action button while parent button shows loading

>
{showKey ? apiKey : maskApiKey(apiKey)}
</pre>
<CopyButton text={apiKey} />
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API key directly passed to CopyButton without security safeguards, exposing sensitive data in the DOM.

})

// Send OTP endpoint
export async function POST(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No rate limiting for OTP generation requests

// ChatGPT-style message component
function ClientChatMessage({ message }: { message: ChatMessage }) {
// Check if content is a JSON object
const isJsonObject = useMemo(() => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type inconsistency: ChatMessage interface defines content as string, but this code checks if it's an object

}

const responseData = await response.json()
console.log('Message response:', responseData)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Console.log statement should be removed from production code

// Extract content from the response - could be in content or output
let messageContent = responseData.output

// Handle different response formats from API
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complex, nested conditional logic for handling API responses indicates an inconsistent API contract

import { db } from '@/db'
import { chat, workflow } from '@/db/schema'
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
import { addCorsHeaders, validateChatAuth, setChatAuthCookie, validateAuthToken, executeWorkflowForChat } from '../utils'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Enforce Utility Function Placement by Usage Pattern

  Utility function 'executeWorkflowForChat' is only used in this file but is defined in a utils file

@waleedlatif1 waleedlatif1 deleted the feat/chat-deploy branch April 28, 2025 17:51
RadoBoiii pushed a commit to RadoBoiii/sim-fork that referenced this pull request May 1, 2025
* added chatbot table with fk to workflows, added modal to deploy and delpoy to subdomain of *.simstudio.ai

* fixed styling, added delete and edit routes for chatbot

* use loading-agent animation for editing existing chatbot

* add base_url so that we can delpoy in dev as well

* fixed CORS issue, fixed password verification, can deploy chatbot and access it at subdomain. still need to fix the actual chat request to match the same format as the chat in the panel

* fix: renamed chatbot to chat and changed chat to copilot

* feat(chat-deploy): refactored api deploy flow

* feat(chat-deploy): added chat to deploy flow

* added output selector to chat deploy, deployment works and we can get a response from subdomain. need to fix UI + form submission of deploy modal but the core functionality works

* add missing dependencies, fix build errors, remove old unused route

* error disappeared for block output selection, need to update UI, add the ability to delete/view chat deployment, and test emails/email domain

* added otp for email verification on chat deploy

* feat(chat-deploy): ux improvements with chat-deploy modal

* improvement(ui/ux): chat display improvement

* improvement(ui/ux): deploy modal

* added logging category for chat panel & chat deploy executions

* improvement(ui/ux): finished chat-deploy flow

* fix: deleted migrations

---------

Co-authored-by: Waleed Latif <walif6@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants