Skip to content

feat: support onboarding ui - #753

Merged
fatelei merged 3 commits into
langgenius:mainfrom
hjlarry:p20
Jun 15, 2026
Merged

feat: support onboarding ui#753
fatelei merged 3 commits into
langgenius:mainfrom
hjlarry:p20

Conversation

@hjlarry

@hjlarry hjlarry commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Description

Please provide a brief description of the changes made in this pull request.
Please also include the issue number if this is related to an issue using the format Fixes #123 or Closes #123.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Performance improvement
  • Other

Essential Checklist

Testing

  • I have tested the changes locally and confirmed they work as expected
  • I have added unit tests where necessary and they pass successfully

Bug Fix (if applicable)

  • I have used GitHub syntax to close the related issue (e.g., Fixes #123 or Closes #123)

Additional Information

Please provide any additional context that would help reviewers understand the changes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new endpoint to list plugins by category, adds the primary task object to the plugin installation response, and includes corresponding unit tests. The review feedback suggests several improvements: refactoring the ListPlugins function to reuse the new pluginInstallationResponse struct to eliminate code duplication, guarding against negative values for skippedMatches in the service layer, making the category listing more robust by skipping plugins that fail to load instead of failing the entire request, and leveraging the oneof validator tag in the controller for category validation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread internal/service/manage_plugin.go
return exception.BadRequestError(errors.New("invalid plugin category")).ToResponse()
}

skippedMatches := (page - 1) * page_size

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If page is less than 1, skippedMatches can result in a negative value. Although the controller validates page >= 1, it is safer to guard against this in the service layer to prevent potential negative index issues if called from other contexts.

Suggested change
skippedMatches := (page - 1) * page_size
skippedMatches := 0
if page > 1 {
skippedMatches = (page - 1) * page_size
}

Comment on lines +188 to +190
if err != nil {
return exception.InternalServerError(err).ToResponse()
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If helper.CombinedGetPluginDeclaration fails for any single plugin installation (e.g., due to corrupted files or missing cache), the entire ListPluginsByCategory API will fail with a 500 Internal Server Error. To make the API more robust, consider skipping the corrupted plugin so that the rest of the tenant's plugins can still be loaded successfully.

Suggested change
if err != nil {
return exception.InternalServerError(err).ToResponse()
}
if err != nil {
continue
}

Comment thread internal/server/controllers/plugins.go
@hjlarry
hjlarry marked this pull request as ready for review June 15, 2026 06:39
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request plugin-daemon labels Jun 15, 2026
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 15, 2026
@fatelei
fatelei merged commit 064f67d into langgenius:main Jun 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer plugin-daemon size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants