[shaohuzhang1]

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

[shaohuzhang1]

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.