Skip to content

fix: [Portal] You can access agents that have not yet been published via the portal's MCP service. #7856

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

fix: [Portal] You can access agents that have not yet been published via the portal's MCP service. #7856

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
6 changes: 5 additions & 1 deletion apps/chat/mcp/application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,11 @@ def get_application_ids(self):
return application_ids

def get_applications(self):
return QuerySet(Application).filter(id__in=self.get_application_ids(), is_publish=True).order_by("name")
return (
QuerySet(Application)
.filter(id__in=self.get_application_ids(), is_portal=True, is_publish=True)
.order_by("name")
)

def is_accessible_application(self, application_id):
try:

@shaohuzhang1 shaohuzhang1 Nov 12, 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.

Here's my review of the diff:

Summary

The change adds an is_portal=True filter to get_applications(). It looks defensive and is likely correct, but a few things are worth verifying.

Potential Issues

  1. Consistency between methods: get_applications() now filters on is_portal=True, but is_accessible_application(application_id) (the method right below) may not apply the same filter. If is_accessible_application uses get_applications() internally, fine — but if it queries Application directly or checks only is_publish, you'll have an inconsistency where an app is "accessible" per one method but absent from the list returned by the other. Worth confirming.

  2. Index / performance: Adding is_portal to the WHERE clause means the query now filters on three columns (id__in, is_portal, is_publish). Make sure there is a suitable composite index, e.g. (is_portal, is_publish, name) or (is_publish, is_portal). Otherwise this can degrade on large tables — especially combined with .order_by("name"), which may not be able to use an index once the filter set changes.

  3. Semantic change: Any caller relying on get_applications() returning all published apps (not just portal ones) will silently lose results. Grep for callers and confirm they expect portal-only behavior.

  4. QuerySet construction: QuerySet(Application) (as opposed to Application.objects) is unusual. If this is a custom wrapper, fine — but verify it doesn't bypass default managers or soft-delete filters. Also double-check that QuerySet(Application).filter(...) doesn't re-evaluate per call; caching get_application_ids() result locally would avoid computing IDs twice if reused.

  5. N+1 risk: If downstream code accesses related attributes on each Application, consider .select_related(...) / .prefetch_related(...) here.

Minor Optimizations

  • Cache self.get_application_ids() in a local variable (if it hits the DB) to avoid duplicate work if called elsewhere in the same request.
  • Consider extracting the filter conditions into a shared helper (e.g. _portal_published_qs()) so get_applications() and is_accessible_application() stay in sync.
  • Formatting/line-break style is fine; the parenthesized chained-call style is readable.

Verdict

No syntax/logic bug in the diff itself. The main risks are (a) behavioral divergence with is_accessible_application, (b) missing composite index for the new filter combination, and (c) hidden callers expecting non-portal apps.

Expand Down
Loading