Skip to content

style: update style for mcp server #8338

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.

Sign up for GitHub

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

Merged
f2c-ci-robot merged 1 commit into from
Apr 7, 2025
Merged

style: update style for mcp server #8338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/app/service/mcp_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func (m McpServerService) GetBindDomain() (response.McpBindDomainRes, error) {
}
website, err := websiteRepo.GetFirst(commonRepo.WithByID(websiteID))
if err != nil {
return res, err
return res, nil
}
res.WebsiteID = website.ID
res.Domain = website.PrimaryDomain
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 7, 2025

Choose a reason for hiding this comment

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

The changes made in this code snippet seem appropriate since returning nil instead of an error indicates that there was no issue fetching the website record with the provided ID. However, for better readability and maintainability, it's good practice to log errors if they occur. Here's an optimized version:

func (m McpServerService) GetBindDomain() (response.McpBindDomainRes, error) {
	var res response.McpBindDomainRes

	websiteRepo := &WebsiteRepository{}
	err := websiteRepo.GetFirst(commonRepo.WithByID(websiteID), &res)
	if err != nil {
		log.Errorf("Failed to fetch website with ID %d: %v", websiteID, err)
		return res, err // Keeping original return pattern for clarity
	}

	res.WebsiteID = website.ID
	res.Domain = website.PrimaryDomain

	return res, nil
}

This code includes logging when an error occurs using a package like log. Additionally, I've used type assertions in the repository call (&res) for explicitness, which might not be necessary in simpler contexts but can enhance clarity.

Expand Down
2 changes: 1 addition & 1 deletion backend/app/service/website.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ func (w WebsiteService) DeleteWebsite(req request.WebsiteDelete) error {
}
websiteID := GetWebsiteID()
if req.ID == websiteID {
_ = settingRepo.UpdateOrCreate("MCP_WEBSITE_ID", "0")
_ = settingRepo.Update("MCP_WEBSITE_ID", "0")
}
tx.Commit()

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 7, 2025

Choose a reason for hiding this comment

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

The code has an issue where the parameter req.ID is compared to variable websiteID, which seems incorrect. Typically, you should compare them if they refer to different objects or IDs. However, given the context of the function name and usage, it might be intended to delete the website identified by req.ID. If so, ensure that websiteID is correctly set to match the value of req.ID.

Additionally, there's unnecessary use of _ before settingRepo.UpdateOrCreate("MCP_WEBSITE_ID", "0"). Removing this underscore would make the code cleaner.

Here’s a revised version with these adjustments:

@@ -498,7 +498,11 @@ func (w WebsiteService) DeleteWebsite(req request.WebsiteDelete) error {
 	}
 	websiteID := GetWebsiteID()
 	if req.ID == websiteID {
-		_ = settingRepo.UpdateOrCreate("MCP_WEBSITE_ID", "0")
+		err := settingRepo.Update("MCP_WEBSITE_ID", "0")
+		if err != nil {
+			return err // Handle the error appropriately
+		}
 	}
 	tx.Commit()

Make sure to handle any potential errors from the Update method appropriately based on your application's requirements.

Expand Down
2 changes: 2 additions & 0 deletions frontend/src/api/interface/ai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ export namespace AI {
volumes: Volume[];
dir?: string;
hostIP: string;
protocol: string;
url: string;
}

export interface McpServerSearch extends ReqPage {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2609,7 +2609,7 @@ const message = {
edit: 'Edit MCP Server',
commandHelper: 'For example: npx -y {0}',
baseUrl: 'External Access Path',
baseUrlHelper: 'For example: https://127.0.0.1:8080',
baseUrlHelper: 'For example: http://192.168.1.2:8000',
ssePath: 'SSE Path',
ssePathHelper: 'For example: /sse, note not to duplicate with other servers',
environment: 'Environment Variables',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/ja.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2579,7 +2579,7 @@ const message = {
edit: 'サーバーを編集',
commandHelper: '例: npx -y {0}',
baseUrl: '外部アクセスパス',
baseUrlHelper: '例: https://127.0.0.1:8080',
baseUrlHelper: '例: http://192.168.1.2:8000',
ssePath: 'SSE パス',
ssePathHelper: '例: /sse, 他のサーバーと重複しないように注意してください',
environment: '環境変数',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/ko.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2538,7 +2538,7 @@ const message = {
edit: 'サーバーを編集',
commandHelper: '例: npx -y {0}',
baseUrl: '外部アクセスパス',
baseUrlHelper: '例: https://127.0.0.1:8080',
baseUrlHelper: '例: http://192.168.1.2:8000',
ssePath: 'SSE パス',
ssePathHelper: '例: /sse, 他のサーバーと重複しないように注意してください',
environment: '環境変数',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/ms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2637,7 +2637,7 @@ const message = {
edit: 'Edit Pelayan',
commandHelper: 'Contoh: npx -y {0}',
baseUrl: 'Laluan Akses Luar',
baseUrlHelper: 'Contoh: https://127.0.0.1:8080',
baseUrlHelper: 'Contoh: http://192.168.1.2:8000',
ssePath: 'Laluan SSE',
ssePathHelper: 'Contoh: /sse, berhati-hati jangan bertindan dengan pelayan lain',
environment: 'Pemboleh Ubah Persekitaran',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/pt-br.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2635,7 +2635,7 @@ const message = {
edit: 'Editar Servidor',
commandHelper: 'Por exemplo: npx -y {0}',
baseUrl: 'Caminho de Acesso Externo',
baseUrlHelper: 'Por exemplo: https://127.0.0.1:8080',
baseUrlHelper: 'Por exemplo: http://192.168.1.2:8000',
ssePath: 'Caminho SSE',
ssePathHelper: 'Por exemplo: /sse, tome cuidado para não duplicar com outros servidores',
environment: 'Variáveis de Ambiente',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/ru.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2631,7 +2631,7 @@ const message = {
edit: 'Редактировать сервер',
commandHelper: 'Например: npx -y {0}',
baseUrl: 'Внешний путь доступа',
baseUrlHelper: 'Например: https://127.0.0.1:8080',
baseUrlHelper: 'Например: http://192.168.1.2:8000',
ssePath: 'Путь SSE',
ssePathHelper: 'Например: /sse, будьте осторожны, чтобы не дублировать с другими серверами',
environment: 'Переменные среды',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/tw.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2444,7 +2444,7 @@ const message = {
edit: '編輯 MCP Server',
commandHelper: '例如：npx -y {0}',
baseUrl: '外部訪問路徑',
baseUrlHelper: '例如：https://127.0.0.1:8080',
baseUrlHelper: '例如：http://192.168.1.2:8000',
ssePath: 'SSE 路徑',
ssePathHelper: '例如：/sse,注意不要與其他 Server 重複',
environment: '環境變數',
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lang/modules/zh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,7 @@ const message = {
edit: '编辑 MCP Server',
commandHelper: '例如：npx -y {0}',
baseUrl: '外部访问路径',
baseUrlHelper: '例如：https://127.0.0.1:8080',
baseUrlHelper: '例如：http://192.168.1.1:8000',
ssePath: 'SSE 路径',
ssePathHelper: '例如：/sse,注意不要与其他 Server 重复',
environment: '环境变量',
Expand Down
26 changes: 22 additions & 4 deletions frontend/src/views/ai/mcp/server/operate/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,15 @@
</el-form-item>
</el-col>
</el-row>
<el-form-item :label="$t('mcp.baseUrl')" prop="baseUrl">
<el-input v-model.trim="mcpServer.baseUrl"></el-input>
<el-form-item :label="$t('mcp.baseUrl')" prop="url">
<el-input v-model.trim="mcpServer.url">
<template #prepend>
<el-select v-model="mcpServer.protocol" class="pre-select">
<el-option label="http" value="http://" />
<el-option label="https" value="https://" />
</el-select>
</template>
</el-input>
<span class="input-help">
{{ $t('mcp.baseUrlHelper') }}
</span>
Expand Down Expand Up @@ -142,6 +149,8 @@ const newMcpServer = () => {
environments: [],
volumes: [],
hostIP: '127.0.0.1',
protocol: 'http://',
url: '',
};
};
const em = defineEmits(['close']);
Expand All @@ -151,7 +160,7 @@ const rules = ref({
command: [Rules.requiredInput],
port: [Rules.requiredInput, Rules.port],
containerName: [Rules.requiredInput],
baseUrl: [Rules.requiredInput],
url: [Rules.requiredInput],
ssePath: [Rules.requiredInput],
key: [Rules.requiredInput],
value: [Rules.requiredInput],
Expand All @@ -167,14 +176,22 @@ const acceptParams = async (params: AI.McpServer) => {
if (!mcpServer.value.volumes) {
mcpServer.value.volumes = [];
}
const parts = mcpServer.value.baseUrl.split(/(https?:\/\/)/).filter(Boolean);
mcpServer.value.protocol = parts[0];
mcpServer.value.url = parts[1];
} else {
mcpServer.value = newMcpServer();
if (params.port) {
mcpServer.value.port = params.port;
}
try {
const res = await getMcpDomain();
mcpServer.value.baseUrl = res.data.connUrl;
if (res.data.connUrl != '') {
const parts = res.data.connUrl.split(/(https?:\/\/)/).filter(Boolean);
mcpServer.value.protocol = parts[0];
mcpServer.value.url = parts[1];
mcpServer.value.baseUrl = res.data.connUrl;
}
} catch (error) {
MsgError(error);
}
Expand Down Expand Up @@ -229,6 +246,7 @@ const submit = async (formEl: FormInstance | undefined) => {
}
try {
loading.value = true;
mcpServer.value.baseUrl = mcpServer.value.protocol + mcpServer.value.url;
if (mode.value == 'create') {
await createMcpServer(mcpServer.value);
MsgSuccess(i18n.global.t('commons.msg.createSuccess'));
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 7, 2025

Choose a reason for hiding this comment

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

There are two main changes:

  1. Protocol Prepend: In the previous version, there was no protocol being prepended to baseUrl. Now, it is mandatory by adding protocol and url fields in state object.

  2. Connection URL Handling: After fetching connection URL from API (getMcpDomain()), the old approach of setting baseUrl directly with res.data.connUrl has changed. New URLs will now also have their protocols extracted (either "http//" or "https//") and assigned separately to protocol and url, before assigning them back to baseUrl.

Additionally, this might be affecting form validation rules as well, so you'd need review those too to ensure they account for these new properties.

Expand Down
Loading