Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature plugin (GPTs like action based on function call) #5331

Merged
merged 41 commits into from
Sep 6, 2024

Conversation

lloydzhou
Copy link
Member

@lloydzhou lloydzhou commented Aug 28, 2024

💻 变更类型 | Change Type

  • feat
  • fix
  • refactor
  • perf
  • style
  • test
  • docs
  • ci
  • chore
  • build

🔀 变更说明 | Description of Change

img_v3_02e7_848d79e9-0bf4-4de5-8343-7a300491864g
img_v3_02e7_57a72ee6-20a5-4bac-a05b-af5ddf20fc4g

📝 补充信息 | Additional Information

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced enhanced tool handling in the chat application, allowing for dynamic execution of functions based on server responses.
    • Added new callback functions for greater flexibility in tool interactions.
    • Implemented a comprehensive plugin management interface, enabling users to create, edit, and delete plugins dynamically.
    • Enhanced decision-making for plugin availability based on service providers.
    • Added a new streaming function to manage real-time chat interactions with improved response handling.
    • Introduced a feature to enable or disable artifacts within the mask configuration.
    • Expanded localization support for plugin functionalities in both English and Chinese.
  • Bug Fixes

    • Refined logic for displaying the typing indicator based on tool presence.
  • Style

    • Added new CSS classes to improve the layout and visual consistency of chat message tools.
  • Chores

    • Updated dependencies to include axios and openapi-client-axios for improved API communication.
    • Enhanced documentation with a reference to a new issue regarding plugin functionality.

Copy link

vercel bot commented Aug 28, 2024

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

Name Status Preview Comments Updated (UTC)
nextchat ❌ Failed (Inspect) Aug 28, 2024 4:03pm

Copy link
Contributor

coderabbitai bot commented Aug 28, 2024

Walkthrough

The changes across the application enhance API interactions and chat functionalities. Key modifications include refining request structures, introducing new callback functionalities, and expanding tool representations within chat messages. A new proxy handler has been added for improved routing, while logging features enhance monitoring capabilities. Additionally, new dependencies have been integrated to modernize HTTP requests and OpenAPI interactions, alongside updates to localization files for better user experience. A new streaming function for chat interactions has also been introduced.

Changes

Files Change Summary
app/api/common.ts Modified requestOpenai function to use only req.nextUrl.pathname, removing query parameters from the path.
app/api/[provider]/[...path]/route.ts Added proxyHandler to enhance routing logic for OpenAI API requests.
app/api/auth.ts Introduced a logging statement for modelProvider in the auth function.
app/api/proxy.ts Implemented a new proxy route handler for managing HTTP requests.
app/client/api.ts Enhanced ChatOptions interface with onBeforeTool and onAfterTool callbacks; updated imports.
app/client/platforms/openai.ts Added tool handling in ChatGPTApi class; introduced running flag and runTools array for tool execution.
app/client/platforms/anthropic.ts Refactored ClaudeApi class for plugin integration and streaming responses.
app/client/platforms/moonshot.ts Streamlined response handling in MoonshotApi class for chat interactions.
app/utils/chat.ts Introduced stream function for managing streaming responses in chat interactions.
app/store/chat.ts Defined ChatMessageTool type; updated ChatMessage type to include tools; added onBeforeTool and onAfterTool methods.
app/store/mask.ts Changed plugin property in Mask type from an array of Plugin types to an array of strings.
app/store/plugin.ts Implemented a plugin management system with methods for adding, retrieving, updating, and deleting plugins.
app/utils.ts Added showPlugins function to determine plugin availability based on provider and model.
package.json Added new dependencies: axios and openapi-client-axios for improved HTTP handling and API interaction.
app/locales/cn.ts Expanded Plugin localization structure with new nested properties for enhanced translations.
app/locales/en.ts Enhanced Plugin localization with new properties and nested objects for improved user interface strings.
README.md Added reference to a new issue (#5353) in the plugins section for documentation enhancement.
next.config.mjs Commented out existing proxy route for OpenAI API, indicating a shift in routing strategy.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ChatAPI
    participant ToolHandler

    User->>ChatAPI: Send message
    ChatAPI->>ToolHandler: Check for tools
    ToolHandler-->>ChatAPI: Return tool status
    ChatAPI->>User: Display message with tool status
Loading

🐇 "In a world of chat so bright,
Tools now dance in pure delight.
With callbacks added, oh what fun,
A seamless flow for everyone!
Errors handled, styles anew,
Hoppy changes, just for you!" 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


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

Name Status Preview Comments Updated (UTC)
nextchat ❌ Failed (Inspect) Aug 28, 2024 4:03pm

@lloydzhou lloydzhou marked this pull request as draft August 28, 2024 16:03
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b6bb167 and f5209fc.

Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (7)
  • app/api/common.ts (1 hunks)
  • app/client/api.ts (2 hunks)
  • app/client/platforms/openai.ts (2 hunks)
  • app/components/chat.module.scss (2 hunks)
  • app/components/chat.tsx (2 hunks)
  • app/store/chat.ts (2 hunks)
  • package.json (2 hunks)
Additional context used
Biome
app/client/platforms/openai.ts

[error] 339-342: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

app/store/chat.ts

[error] 405-405: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

Additional comments not posted (13)
package.json (2)

27-27: LGTM!

The addition of axios is appropriate for handling HTTP requests.

The code changes are approved.


37-37: LGTM!

The addition of openapi-client-axios is appropriate for structured and type-safe API interactions.

The code changes are approved.

app/api/common.ts (1)

35-35: LGTM!

The change simplifies the path extraction process and removes potential complications introduced by query parameters.

The code changes are approved.

app/client/api.ts (2)

8-14: LGTM!

The import statement is correct and necessary for the new functionality.

The code changes are approved.


65-66: LGTM!

The additions enhance the control flow related to tool usage in the chat context.

The code changes are approved.

app/components/chat.module.scss (2)

416-420: LGTM!

The .chat-message-tools class is correctly implemented and follows standard CSS practices.

The code changes are approved.


421-429: LGTM!

The nested .chat-message-tool class is correctly implemented and follows standard CSS practices.

The code changes are approved.

app/store/chat.ts (3)

31-40: LGTM!

The ChatMessageTool type declaration is correctly implemented and follows TypeScript best practices.

The code changes are approved.


48-48: LGTM!

The ChatMessage type declaration is correctly updated to include tools?: ChatMessageTool[] and follows TypeScript best practices.

The code changes are approved.


410-420: LGTM!

The onAfterTool method is correctly implemented and follows TypeScript best practices.

The code changes are approved.

app/components/chat.tsx (3)

31-31: Import statement looks good.

The import statement for CloseIcon is correctly added.

The code changes are approved.


1577-1579: Conditional check for typing status looks good.

The logic for displaying the typing status has been correctly updated to check if the message?.tools array is empty.

The code changes are approved.


1582-1600: Conditional rendering for chat message tools looks good.

The new conditional rendering block for displaying chat message tools is correctly implemented. The logic for rendering each tool's status based on its isError property is clear and enhances the user interface.

The code changes are approved.

app/client/platforms/openai.ts Outdated Show resolved Hide resolved
app/client/platforms/openai.ts Outdated Show resolved Hide resolved
Copy link

vercel bot commented Aug 28, 2024

@lloydzhou is attempting to deploy a commit to the NextChat Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

Your build has completed!

Preview deployment

@lloydzhou lloydzhou marked this pull request as ready for review September 2, 2024 14:22
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c7bc93b and 7180ed9.

Files selected for processing (2)
  • app/client/platforms/anthropic.ts (3 hunks)
  • app/utils/chat.ts (2 hunks)
Additional context used
Biome
app/utils/chat.ts

[error] 257-257: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)


[error] 161-161: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)

Additional comments not posted (5)
app/utils/chat.ts (1)

257-257: The static analysis hint "This code is unreachable" is a false positive and can be ignored.

The code at line 257 is reachable because the finish function is called recursively. If runTools is empty and running is false, the function will reach the code at line 257 and continue executing.

Tools
Biome

[error] 257-257: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

app/client/platforms/anthropic.ts (4)

3-9: LGTM!

The additional imports from @/app/store are necessary to support the new functionality related to plugin integration and streaming responses.


20-20: LGTM!

The import of the stream function from @/app/utils/chat is necessary to handle the streaming responses from the API.


203-208: LGTM!

The code segment correctly retrieves the necessary tools and functions from the plugin store based on the current chat session's mask to support plugin integration.


310-320: LGTM!

The code segment correctly handles the case when streaming is not enabled by sending a regular, non-streaming request to the API using the fetch function. The code is straightforward and doesn't require any changes.

app/utils/chat.ts Outdated Show resolved Hide resolved
app/utils/chat.ts Show resolved Hide resolved
app/client/platforms/anthropic.ts Outdated Show resolved Hide resolved
app/client/platforms/anthropic.ts Show resolved Hide resolved
app/client/platforms/anthropic.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7180ed9 and 6ab6b3d.

Files selected for processing (3)
  • app/client/platforms/anthropic.ts (3 hunks)
  • app/client/platforms/moonshot.ts (2 hunks)
  • app/store/chat.ts (2 hunks)
Additional context used
Biome
app/store/chat.ts

[error] 407-407: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

Additional comments not posted (13)
app/client/platforms/moonshot.ts (4)

11-17: LGTM!

The new imports are necessary for the expanded functionality related to plugin integration and state management.


19-19: LGTM!

The new imports are necessary for the changes made to the stream function usage.


125-184: Excellent refactoring!

The changes made to the stream function usage significantly improve the efficiency and maintainability of the code:

  • The use of usePluginStore and useChatStore allows for more efficient handling of tool calls and response parsing.
  • The refactoring eliminates the need for manual text animation and state management, reducing complexity and improving the flow of data handling.
  • The streaming function now directly updates the response based on incoming messages, parsing JSON data to extract tool calls and their arguments, which enhances the clarity and maintainability of the code.

Overall, these changes reflect a move towards a more modular and efficient architecture for handling chat interactions, with an emphasis on real-time processing and integration of external tools.


169-181: LGTM!

The processToolMessage function is correctly implemented and is necessary for the proper functioning of the stream function.

app/client/platforms/anthropic.ts (6)

208-266: The past review comment reminding to add tests for the new streaming and plugin functionality is still valid and applicable to the current code changes.


268-305: The past review comment suggesting to avoid modifying the requestPayload object directly is still valid and applicable to the current code changes.


3-9: LGTM!

The code changes are approved.


20-20: LGTM!

The code changes are approved.


22-22: LGTM!

The code changes are approved.


309-319: LGTM!

The code changes are approved.

app/store/chat.ts (3)

32-42: LGTM!

The new ChatMessageTool type declaration looks good. It has all the necessary properties to represent a tool associated with a chat message.


50-50: LGTM!

The ChatMessage type has been correctly updated to include an optional tools property, which is an array of ChatMessageTool instances. This allows chat messages to accommodate associated tools.


412-421: LGTM!

The onAfterTool method looks good. It correctly updates the corresponding tool in the tools array based on its id and updates the current session's messages.

app/client/platforms/anthropic.ts Show resolved Hide resolved
app/store/chat.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6ab6b3d and 04e1ab6.

Files selected for processing (1)
  • README.md (2 hunks)
Additional context used
Markdownlint
README.md

135-135: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level

(MD005, list-indent)


133-133: Expected: 2; Actual: 3
Unordered list indentation

(MD007, ul-indent)


134-134: Expected: 2; Actual: 3
Unordered list indentation

(MD007, ul-indent)


135-135: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)

Additional comments not posted (3)
README.md (3)

94-96: LGTM!

The code changes are approved.


101-102: LGTM!

The code changes are approved.


139-140: LGTM!

The code changes are approved.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Outside diff range, codebase verification and nitpick comments (2)
package.json (2)

27-27: Approve the addition of axios dependency, but suggest using the latest available version.

The addition of axios dependency is consistent with the AI-generated summary and will provide a promise-based HTTP client for making requests.

However, the version constraint ^1.7.5 might be using a future version that doesn't exist yet. The latest version of axios is 1.4.0 as of April 2024.

Consider using the latest available version ^1.4.0 instead:

-    "axios": "^1.7.5",
+    "axios": "^1.4.0",

38-38: Approve the addition of openapi-client-axios dependency, but suggest using the latest available version.

The addition of openapi-client-axios dependency is consistent with the AI-generated summary and will facilitate the integration with OpenAPI specifications, allowing for more structured and type-safe API interactions.

However, the version constraint ^7.5.5 might be using a future version that doesn't exist yet. The latest version of openapi-client-axios is 7.0.0 as of April 2024.

Consider using the latest available version ^7.0.0 instead:

-    "openapi-client-axios": "^7.5.5",
+    "openapi-client-axios": "^7.0.0",
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 04e1ab6 and 9820193.

Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (8)
  • app/components/plugin.tsx (1 hunks)
  • app/global.d.ts (1 hunks)
  • app/store/chat.ts (2 hunks)
  • app/store/plugin.ts (1 hunks)
  • app/utils.ts (2 hunks)
  • package.json (3 hunks)
  • src-tauri/Cargo.toml (1 hunks)
  • src-tauri/tauri.conf.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • app/utils.ts
Additional context used
Biome
app/store/plugin.ts

[error] 55-55: Avoid redundant double-negation.

It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation

(lint/complexity/noExtraBooleanCast)


[error] 57-57: Avoid redundant double-negation.

It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation

(lint/complexity/noExtraBooleanCast)


[error] 74-142: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 28-28: Don't use 'Object' as a type.

Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.

(lint/complexity/noBannedTypes)


[error] 36-36: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)


[error] 87-87: Use Array.isArray() instead of instanceof Array.

instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.

(lint/suspicious/useIsArray)


[error] 120-120: Use Array.isArray() instead of instanceof Array.

instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.

(lint/suspicious/useIsArray)

app/components/plugin.tsx

[error] 340-340: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

app/store/chat.ts

[error] 407-407: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

Additional comments not posted (11)
app/global.d.ts (2)

24-28: Skipping comment on the spacing change.

The spacing change is a minor formatting adjustment and does not affect the functionality.


29-34: LGTM!

The code changes are approved.

Verify the definition of the Response interface.

Ensure that the Response interface is defined correctly.

Run the following script to verify the interface definition:

src-tauri/Cargo.toml (1)

Line range hint 20-36: LGTM!

The changes to the tauri dependency in Cargo.toml are approved. The addition of the "http-all" feature and the retention of other existing features align with the PR objective of introducing a new feature to enable actions similar to those of GPTs based on function calls. The expanded features provided by the Tauri framework may be necessary for implementing the new functionality.

src-tauri/tauri.conf.json (1)

54-57: Verify the security implications of allowing all HTTP requests.

Setting "all" and "request" to true without any restrictions could potentially introduce security risks. Please ensure that allowing all HTTP requests aligns with the application's security requirements.

To verify the security implications, run the following script:

Consider restricting the "scope" to specific domains if possible.

The current configuration allows all HTTPS and HTTP URLs, which is very permissive. If the application only needs to make requests to specific domains, consider restricting the "scope" accordingly.

For example, if the application only needs to make requests to https://api.example.com, update the configuration like this:

-      "http": {
-        "all": true,
-        "request": true,
-        "scope": ["https://*", "http://*"]
-      }
+      "http": {
+        "all": false,
+        "request": true,
+        "scope": ["https://api.example.com"]
+      }
Verification successful

No HTTP requests found in the codebase using common libraries.

The search for HTTP request-related code using various libraries (reqwest, http_request, hyper, surf, std::net) did not yield any results. This suggests that the current configuration in tauri.conf.json does not pose an immediate security risk, as there are no HTTP requests being made using these libraries in the Rust files. Please ensure that any future implementations align with the application's security requirements.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the application is making any external HTTP requests to untrusted domains.

# Test: Search for HTTP request code. Expect: Only requests to trusted domains.
rg --type rust $'reqwest|http_request' -g '*.rs'

Length of output: 49


Script:

#!/bin/bash
# Description: Verify if the application is making any external HTTP requests using various libraries.

# Test: Search for HTTP request code using different libraries. Expect: Only requests to trusted domains.
rg --type rust $'reqwest|http_request|hyper|surf|std::net' -g '*.rs'

Length of output: 69

app/components/plugin.tsx (3)

32-117: LGTM!

The state variables and handlers in the PluginPage component are implemented correctly. The onSearch handler filters the plugins based on the search text, the onChangePlugin handler updates the plugin content and parses the YAML definition, and the loadFromUrl handler loads the plugin content from a URL while handling CORS issues.


118-210: LGTM!

The rendering logic in the PluginPage component is implemented correctly. The component renders a list of plugins with their details, including the plugin title, version, and action buttons. The delete action is handled safely with a confirmation prompt. The component also conditionally renders the edit and delete buttons based on the plugin type.


214-362: LGTM!

The plugin editing modal in the PluginPage component is implemented correctly. The modal is rendered conditionally based on the editingPluginId state and allows editing the plugin authentication details and content. The authentication details are handled based on the selected auth type, and the code editor is implemented using a contenteditable pre tag. The modal also lists the functions defined in the plugin.

Tools
Biome

[error] 340-340: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

app/store/chat.ts (4)

32-42: LGTM!

The new ChatMessageTool type declaration looks good. It has all the necessary properties to represent a tool associated with a chat message.


50-50: LGTM!

The ChatMessage type is correctly updated to include an optional tools property, which is an array of ChatMessageTool instances. This allows chat messages to accommodate associated tools.


412-421: LGTM!

The onAfterTool method looks good. It correctly updates the corresponding tool in the tools array based on its id and updates the current session's messages.


406-411: LGTM! But address the assignment in the expression.

The onBeforeTool method is correctly implemented. It handles pushing a new tool into the tools array of a botMessage and updates the current session's messages.

However, the assignment should not be in an expression at line 407, as flagged by the static analysis tool. It can be confusing and should be avoided.

Apply this diff to move the assignment out of the expression:

- (botMessage.tools = botMessage?.tools || []).push(tool);
+ if (!botMessage.tools) {
+   botMessage.tools = [];  
+ }
+ botMessage.tools.push(tool);

Likely invalid or redundant comment.

Tools
Biome

[error] 407-407: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/store/plugin.ts Show resolved Hide resolved
app/store/plugin.ts Show resolved Hide resolved
app/store/plugin.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9820193 and b590d08.

Files selected for processing (1)
  • next.config.mjs (1 hunks)
Additional comments not posted (2)
next.config.mjs (2)

Line range hint 72-87: Review and approve the configuration of remaining proxy routes.

The proxy routes for Azure, Google, and Anthropic are correctly configured. However, it's important to conduct a performance review to ensure that these routes handle traffic efficiently and securely.


68-71: Verify the impact of commenting out the OpenAI API v1 proxy route.

The commented-out proxy route previously directed requests to the OpenAI API v1 endpoint. It's crucial to verify that this change does not affect any existing functionalities or integrations that depend on this endpoint.

Run the following script to verify if any part of the application still references the old API route:

Verification successful

Verification successful: No active references to the old OpenAI API v1 route.

The search confirmed that the only reference to the old API route /api/proxy/v1/:path* is commented out in the next.config.mjs file, with no active references elsewhere in the codebase. This indicates that commenting out the route does not impact existing functionalities or integrations.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for references to the old OpenAI API v1 route.

# Test: Search for the route usage. Expect: No occurrences of the old route.
rg --type javascript --type mjs --type jsx $'/api/proxy/v1/:path*'

Length of output: 102


Script:

#!/bin/bash
# Description: Search for references to the old OpenAI API v1 route.

# Test: Search for the route usage. Expect: No occurrences of the old route.
rg -g '*.js' -g '*.mjs' -g '*.jsx' '/api/proxy/v1/:path*'

Length of output: 118

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b590d08 and f32dd69.

Files selected for processing (8)
  • README.md (2 hunks)
  • app/components/chat.tsx (7 hunks)
  • app/components/markdown.tsx (3 hunks)
  • app/components/mask.tsx (1 hunks)
  • app/constant.ts (3 hunks)
  • app/locales/cn.ts (3 hunks)
  • app/locales/en.ts (3 hunks)
  • app/store/mask.ts (3 hunks)
Files skipped from review as they are similar to previous changes (4)
  • app/components/chat.tsx
  • app/components/markdown.tsx
  • app/constant.ts
  • app/store/mask.ts
Additional context used
Markdownlint
README.md

133-133: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level

(MD005, list-indent)


132-132: Expected: 2; Actual: 3
Unordered list indentation

(MD007, ul-indent)


133-133: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)

Additional comments not posted (3)
app/locales/cn.ts (1)

533-571: Review of the Plugin section enhancements

The modifications to the Plugin section introduce a more structured approach to handling plugin-related functionalities. The addition of nested properties such as Page, Item, Auth, and EditModal significantly enriches the localization capabilities for these elements.

Detailed Observations:

  • Page and Item: These objects now include properties for basic operations like Create, Edit, Delete, and viewing details, which aligns well with typical CRUD operations in web applications.
  • Auth: The expanded authentication options (None, Basic, Bearer, Custom) along with detailed descriptions for proxy usage (ProxyDescription) enhance the flexibility and clarity for end-users regarding authentication mechanisms.
  • EditModal: This object now supports localization for modal dialog elements, which is crucial for maintaining consistency in user interfaces across different languages.

Recommendations:

  • Consistency Check: Ensure that all keys in these objects are consistently used across the application to avoid missing translations.
  • Verification of Proxy Settings: Given the addition of proxy settings in the Auth object, it's important to verify that these settings are correctly implemented and tested in the application to handle CORS issues effectively.

Overall, these changes are well-structured and provide a comprehensive approach to localizing plugin-related functionalities. However, it is crucial to ensure that these new keys are integrated and used consistently throughout the application.

README.md (1)

94-95: Documentation Enhancement Approved

The addition of detailed plugin features and relevant issue references enhances the clarity and usefulness of the documentation.

app/locales/en.ts (1)

541-580: Review of the Plugin Localization Object

The Plugin object has been significantly expanded to include new properties and nested objects, enhancing the localization capabilities for plugin features. Here are some specific observations and suggestions:

  1. Structure and Readability:

    • The structure of the new Plugin object is well-organized, with clear separation of concerns among Page, Item, Auth, and EditModal.
    • The use of functions for dynamic content (e.g., SubTitle in Page and Title in EditModal) is consistent with other parts of the localization file, which is good for maintainability.
  2. Consistency with Existing Patterns:

    • The localization keys are consistent with the existing naming conventions used throughout the file. This consistency is crucial for maintainability and ease of use.
  3. Potential Improvements:

    • Documentation: While the keys are descriptive, adding comments to explain less obvious keys (e.g., ProxyDescription in Auth) could improve clarity for future developers or translators.
    • Error Handling: The Error key in EditModal could include more specific error messages tailored to different failure scenarios, enhancing the user experience by providing more informative feedback.
  4. Verification:

    • It's important to verify that all new keys are used in the application and that there are no missing keys that might be required for new features. This can be done by checking the usage in the codebase or adding tests to ensure all keys are rendered correctly in the UI.

Overall, the changes to the Plugin object significantly enhance the localization capabilities for the plugin features and improve the user experience by providing comprehensive and contextually relevant strings.

README.md Show resolved Hide resolved
app/components/mask.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f32dd69 and 7c0acc7.

Files selected for processing (1)
  • app/utils/chat.ts (2 hunks)
Additional context used
Biome
app/utils/chat.ts

[error] 257-257: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)


[error] 280-280: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 161-161: Don't use 'Function' as a type.

Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.

(lint/complexity/noBannedTypes)

app/utils/chat.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7c0acc7 and 7455978.

Files selected for processing (1)
  • app/components/mask.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • app/components/mask.tsx

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7455978 and 9275f2d.

Files selected for processing (5)
  • app/components/chat.tsx (7 hunks)
  • app/components/plugin.tsx (1 hunks)
  • app/constant.ts (4 hunks)
  • app/locales/cn.ts (3 hunks)
  • app/locales/en.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
  • app/components/chat.tsx
  • app/constant.ts
Additional context used
Biome
app/components/plugin.tsx

[error] 371-371: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

Additional comments not posted (3)
app/components/plugin.tsx (1)

1-4: Imports and Constants: Approved

All imports and constants are correctly used and relevant to the functionality of the component.

Also applies to: 20-33

app/locales/cn.ts (1)

533-572: Localization: Enhanced Plugin Section

The enhancements to the Plugin section in the localization file are comprehensive and align with the intended functionality. Ensure that all keys are consistently used in the UI components to maintain localization accuracy.

Verification successful

Localization Keys Verified in UI Components

The localization keys for the Plugin section are effectively integrated into the UI components, confirming their proper usage and alignment with the intended functionality. No issues were found in the verification process.

  • Files where keys are used:
    • app/components/chat.tsx
    • app/components/plugin.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all localization keys are used in the UI components.

# Test: Search for localization keys in the UI components. Expect: All keys should be used.
rg --type typescript -A 5 $'Locale.Plugin'

Length of output: 76


Script:

#!/bin/bash
# Description: Verify that all localization keys are used in the UI components.

# Test: Search for localization keys in the UI components. Expect: All keys should be used.
rg 'Locale\.Plugin' -A 5 --type ts

Length of output: 9679

app/locales/en.ts (1)

541-580: Localization: Enhanced Plugin Section

The enhancements to the Plugin section in the localization file are comprehensive and align with the intended functionality. Ensure that all keys are consistently used in the UI components to maintain localization accuracy.

Verification successful

Localization Keys Verified in UI Components

The localization keys from the Plugin section are successfully integrated and used in the UI components, ensuring consistency and alignment with the intended functionality. The keys are utilized in various contexts such as titles, subtitles, button texts, and error messages.

  • Files Verified:
    • app/components/plugin.tsx
    • app/components/chat.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all localization keys are used in the UI components.

# Test: Search for localization keys in the UI components. Expect: All keys should be used.
rg --type typescript -A 5 $'Locale.Plugin'

Length of output: 76


Script:

#!/bin/bash
# Description: Verify that all localization keys are used in the UI components.

# Test: Search for localization keys in the UI components. Expect: All keys should be used.
rg --type ts -A 5 $'Locale.Plugin'

Length of output: 9676

Comment on lines +34 to +393
<option value="body">
{Locale.Plugin.Auth.LocationBody}
</option>
</select>
</ListItem>
)}
{editingPlugin.authType == "custom" && (
<ListItem title={Locale.Plugin.Auth.CustomHeader}>
<input
type="text"
value={editingPlugin?.authHeader}
onChange={(e) => {
pluginStore.updatePlugin(editingPlugin.id, (plugin) => {
plugin.authHeader = e.target.value;
});
}}
></input>
</ListItem>
)}
{["bearer", "basic", "custom"].includes(
editingPlugin.authType as string,
) && (
<ListItem title={Locale.Plugin.Auth.Token}>
<PasswordInput
type="text"
value={editingPlugin?.authToken}
onChange={(e) => {
pluginStore.updatePlugin(editingPlugin.id, (plugin) => {
plugin.authToken = e.currentTarget.value;
});
}}
></PasswordInput>
</ListItem>
)}
{!getClientConfig()?.isApp && (
<ListItem
title={Locale.Plugin.Auth.Proxy}
subTitle={Locale.Plugin.Auth.ProxyDescription}
>
<input
type="checkbox"
checked={editingPlugin?.usingProxy}
style={{ minWidth: 16 }}
onChange={(e) => {
pluginStore.updatePlugin(editingPlugin.id, (plugin) => {
plugin.usingProxy = e.currentTarget.checked;
});
}}
></input>
</ListItem>
)}
</List>
<List>
<ListItem title={Locale.Plugin.EditModal.Content}>
<div style={{ display: "flex", justifyContent: "flex-end" }}>
<input
type="text"
style={{ minWidth: 200, marginRight: 20 }}
onInput={(e) => setLoadUrl(e.currentTarget.value)}
></input>
<IconButton
icon={<ReloadIcon />}
text={Locale.Plugin.EditModal.Load}
bordered
onClick={() => loadFromUrl(loadUrl)}
/>
</div>
</ListItem>
<ListItem
subTitle={
<div
className={`markdown-body ${pluginStyles["plugin-content"]}`}
dir="auto"
>
<pre>
<code
contentEditable={true}
dangerouslySetInnerHTML={{
__html: editingPlugin.content,
}}
onBlur={onChangePlugin}
></code>
</pre>
</div>
}
></ListItem>
{editingPluginTool?.tools.map((tool, index) => (
<ListItem
key={index}
title={tool?.function?.name}
subTitle={tool?.function?.description}
/>
))}
</List>
</Modal>
</div>
)}
</ErrorBoundary>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Review: PluginPage Function

The PluginPage function is well-structured and uses React best practices. However, the complexity of the function could be reduced by breaking it into smaller components or hooks. Consider refactoring to improve readability and maintainability.

Tools
Biome

[error] 371-371: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

Comment on lines +371 to +373
dangerouslySetInnerHTML={{
__html: editingPlugin.content,
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Security Issue: Sanitize Content with DOMPurify

As previously noted, using dangerouslySetInnerHTML can expose the application to XSS attacks. It is crucial to sanitize the content using DOMPurify before rendering it to prevent security vulnerabilities.

Tools
Biome

[error] 371-371: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

@Dogtiti Dogtiti merged commit 2bd799f into ChatGPTNextWeb:main Sep 6, 2024
1 of 2 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Sep 25, 2024
10 tasks
@lloydzhou lloydzhou deleted the feature/plugin branch September 26, 2024 13:35
@coderabbitai coderabbitai bot mentioned this pull request Oct 23, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants