Skip to content

feat: add enable_agent_mcp_service field to portal model and update related components #7725

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
wxg0103 merged 1 commit into from
Nov 6, 2026
Merged

feat: add enable_agent_mcp_service field to portal model and update related components #7725

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
17 changes: 17 additions & 0 deletions apps/portal/migrations/0002_portal_enable_agent_mcp_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 6.1.2 on 2026-11-06 09:23

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("portal", "0001_initial"),
]

operations = [
migrations.AddField(
model_name="portal",
name="enable_agent_mcp_service",
field=models.BooleanField(default=True, verbose_name="是否开启智能体MCP服务"),
),
]

@shaohuzhang1 shaohuzhang1 Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No blocking syntax or Django-migration irregularities are visible. The migration is a standard AddField operation and should work as generated.

Potential issues / things to verify:

  1. default=True affects all existing rows

    • Every existing Portal row will get enable_agent_mcp_service=True.
    • If MCP service exposure has security/privacy implications, consider default=False and opt-in explicitly.
    • If True is only meant to populate existing rows but new rows should have no/default False, use preserve_default=False and then adjust the model field appropriately.

  2. Dependency ordering

    • ("portal", "0001_initial") is fine only if 0001_initial is the correct latest dependency.
    • If the portal app has later migrations, this migration should usually depend on the latest relevant migration to avoid ordering problems.

  3. Django version header

    • The file says Generated by Django 6.1.2. Confirm the project/CI actually uses Django 6.1.2.
    • The header itself is harmless, but a mismatch may indicate the migration was generated in the wrong environment.

  4. Large-table migration performance

    • Adding a non-null BooleanField with a default can lock or rewrite the table on some databases, especially older MySQL/SQLite setups.
    • On PostgreSQL 11+ with a constant default, this is usually fast.
    • For very large tables, consider nullable column + batched backfill + AlterField, with atomic = False if needed.

  5. Model/migration consistency

    • Ensure the model has exactly: 
      enable_agent_mcp_service = models.BooleanField(
    default=True,
    verbose_name="是否开启智能体MCP服务",
)
    • Run: 
      python manage.py makemigrations --check --dry-run
      to confirm no pending migration changes.

  6. No index needed

    • A plain BooleanField generally does not need an index due to low selectivity. Only add a partial index if you frequently query enabled portals and profiling justifies it.

  7. Generated migration hygiene

    • Do not hand-edit generated migrations unless intentional.
    • Commit the migration with the model change.
    • Add or update a test asserting the default value if this flag is business-critical.

Overall: No correctness bug in the migration itself. The main decision is whether default=True is intentional for all existing and new portals.

2 changes: 2 additions & 0 deletions apps/portal/models/portal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class Portal(AppModelMixin):
# API服务配置
enable_api = models.BooleanField(default=True, verbose_name="是否开启API服务")

enable_agent_mcp_service = models.BooleanField(default=True, verbose_name="是否开启智能体MCP服务")

enable_knowledge_base_api = models.BooleanField(default=True, verbose_name="是否开启知识库API")

# 身份认证配置
Expand Down
3 changes: 3 additions & 0 deletions apps/portal/serializers/portal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ class PortalSerializer(serializers.Serializer):
required=False, label=_("enable public access"), help_text=_("enable public access")
)
enable_api = serializers.BooleanField(required=False, label=_("enable api"), help_text=_("enable api"))
enable_agent_mcp_service = serializers.BooleanField(
required=False, label=_("enable agent mcp service"), help_text=_("enable agent mcp service")
)
enable_knowledge_base_api = serializers.BooleanField(
required=False, label=_("enable knowledge base api"), help_text=_("enable knowledge base api")
)

@shaohuzhang1 shaohuzhang1 Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The added field is syntactically valid and consistent with the surrounding BooleanField declarations. I don’t see an immediate bug in the snippet itself, but here are the points to verify and possible improvements:

  1. Persistence/model wiring

    • If PortalSerializer is a ModelSerializer, confirm that enable_agent_mcp_service exists on the model and that a migration has been created.
    • If it is a plain Serializer, adding the field alone does not persist it. Ensure create() / update() explicitly handle this value.

  2. Default semantics

    • required=False means the field may be omitted. If omitted, it will not appear in validated_data.
    • If the intended behavior is “missing means disabled/false,” consider default=False. However, be careful with full updates: a default can overwrite existing values when the client omits the field. For partial updates (PATCH), DRF skips missing fields, so behavior is safer.
    • If this is a feature flag that must be explicitly enabled, required=False without a default may be preferable.

  3. Security implications

    • “Agent MCP service” likely exposes an API/service surface. Ensure:
      • disabled by default,
      • authorization and tenant isolation are enforced,
      • rate limiting and audit logging are present,
      • the endpoint is not enabled accidentally through mass assignment.
    • Consider whether enabling it should require a separate permission or admin action.

  4. API contract and compatibility

    • Since the field is optional, existing clients should not break.
    • Regenerate OpenAPI/schema docs and update API documentation.
    • Confirm frontend/UI, config, and deployment code know how to read/write this field.

  5. Naming and i18n copy

    • The label/help text are lowercase and identical. Prefer clearer, consistent copy, e.g.: 
      label=_("Enable Agent MCP service"),
help_text=_("Allow access to the Agent MCP service."),
    • Check whether the project uses title case or sentence case for other labels.

  6. Consistency with other enable_* fields

    • If other boolean feature flags have defaults, validators, or permissions, mirror that behavior.
    • Ensure this field is included in any allowlists/denylists, serializers, exports, or config synchronization logic.

  7. Tests

    • Add serializer tests for:
      • field omitted,
      • field True / False,
      • partial update behavior,
      • persistence and permission enforcement,
      • schema output.

  8. Lint/style

    • Check line length and formatting against the project’s linter. The added line is long but may still be acceptable.

Overall: the field addition itself looks fine, but the main risks are persistence wiring, default behavior, and security/authorization around enabling the MCP service.

Expand Down
1 change: 1 addition & 0 deletions ui/src/api/types/portal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface PortalSetting {
logo: string | null
enable_public_access: boolean
enable_api: boolean
enable_agent_mcp_service: boolean
enable_knowledge_base_api: boolean
enable_auth: boolean
auth_config: PortalAuthConfig
Expand Down
7 changes: 4 additions & 3 deletions ui/src/views/system/chat-management/portal-setting/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import ButtonPortalAuthSetting from './components/ButtonPortalAuthSetting.vue'
import ButtonPortalCorsSetting from './components/ButtonPortalCorsSetting.vue'
import { useStore } from '@/stores'

type AccessField = 'enable_public_access' | 'enable_api' | 'enable_knowledge_base_api' | 'enable_auth' | 'enable_cors'
type AccessField = 'enable_public_access' | 'enable_api' | 'enable_agent_mcp_service' | 'enable_knowledge_base_api' | 'enable_auth' | 'enable_cors'

const portalSetting = reactive<PortalSetting>({
id: '',
Expand All @@ -21,6 +21,7 @@ const portalSetting = reactive<PortalSetting>({
logo: '',
enable_public_access: true,
enable_api: true,
enable_agent_mcp_service: true,
enable_knowledge_base_api: true,
enable_auth: false,
auth_config: {
Expand Down Expand Up @@ -173,9 +174,9 @@ const canEditPortal = computed(() => perm.system.portal.edit())
</span>
<el-switch
:disabled="!canEditPortal || saving"
:model-value="portalSetting.enable_knowledge_base_api"
:model-value="portalSetting.enable_agent_mcp_service"
size="small"
@change="handleAccessChange('enable_knowledge_base_api', $event)"
@change="handleAccessChange('enable_agent_mcp_service', $event)"
/>
</div>

@shaohuzhang1 shaohuzhang1 Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review of the diff

🔴 Critical / likely bugs

  1. This looks like a replacement rather than an addition.
    The switch that previously controlled enable_knowledge_base_api now controls enable_agent_mcp_service. If the intent was to add a new toggle for the Agent MCP service, the knowledge-base API toggle has now been removed from the UI, so users can no longer turn KB API access on/off. Verify whether:

    • a second <el-switch> for enable_agent_mcp_service should have been added instead, or
    • the old KB switch exists elsewhere in the template (otherwise it's a functional regression).

  2. The label/description next to the switch was probably not updated.
    The diff only touches :model-value and @change; the visible text (and any tooltip / help link) in that block still describes "Knowledge Base API". Users would be toggling MCP while reading a KB label. Make sure the surrounding markup, i18n keys, and any permission/guard text were updated too.

  3. PortalSetting type must be extended.
    reactive<PortalSetting>({ ... enable_agent_mcp_service: true ... }) will only type-check if the PortalSetting interface declares enable_agent_mcp_service. If it doesn't, either add it or you'll get an excess-property error (and it's easy to miss because reactive sometimes hides this).

  4. Default value true deserves a second thought.
    Enabling a new external-facing service (MCP endpoint) by default is a security/behavioral decision. Check:

    • what the backend default is (client/server defaults should match, otherwise the UI shows true while the server has false until the user touches the switch),
    • whether existing portals getting this value implicitly (e.g. from a missing field in the API payload) will silently expose a new endpoint.

  5. Any other place that enumerates AccessField must be updated.
    Search for exhaustive Record<AccessField, …>, switch statements, field lists for "enable all" / permission checks, analytics events, or a "public access" summary badge. Adding a union member breaks exhaustiveness checks or silently omits the new field.

🟡 Potential issues

  1. Controlled switch + async save has no rollback shown in the diff.
    :model-value + @change means the UI reflects portalSetting state. Ensure handleAccessChange reverts the reactive value when the API call fails; otherwise the switch stays visually "on" while the server rejected it.

  2. @change payload typing.
    Element Plus el-switch emits boolean | string | number. If handleAccessChange isn't already casted ($event as boolean), the field type may silently accept a string.

  3. Keep-alive / re-fetch consistency.
    If the parent re-fetches settings, confirm the new field is part of the response mapping (Object.assign(portalSetting, data) or explicit mapping); otherwise the switch resets to true on every reload regardless of the server value.

🟢 Optimization suggestions

  1. De-duplicate the switch markup.
    You now have several near-identical <el-switch> blocks. Consider driving them from a config array:

    const accessSwitches: { field: AccessField; labelKey: string; disabled?: () => boolean }[] = [
  { field: 'enable_public_access',  labelKey: 'portal.public_access' },
  { field: 'enable_api',            labelKey: 'portal.api' },
  { field: 'enable_agent_mcp_service', labelKey: 'portal.agent_mcp_service' },
  ...
]

    and v-for over it — fewer copy-paste mistakes like issue build(deps): bump the pip group across 1 directory with 2 updates #1.

  2. Name it consistently with the domain.
    enable_agent_mcp_service is long and mixes agent + mcp. If the API already names it something else (e.g. enable_mcp), align the client field name with the backend to avoid mapping layers.

  3. Consider grouping/ordering.
    Put the MCP toggle next to related "API access" toggles rather than in the middle of the KB block, and gate it behind the same feature flag/permission as the rest of the portal editing (canEditPortal already applies — good).

Bottom line

The type + default additions are fine, but the template change almost certainly drops an existing feature and the associated label/i18n text was not updated. Confirm whether the new switch was meant to be added alongside enable_knowledge_base_api rather than replacing it, and double-check PortalSetting and the backend default for the new field.

Expand Down
Loading