Repository navigation
feat(admin): add public team and bot management - #161
Merged
Merged
Conversation
Add support for system-level public agents (teams) and bots that are visible to all users. Public resources use user_id=0 as identifier. Backend changes: - Add Bot to PUBLIC_FALLBACK_KINDS for public bot lookup support - Add public team fallback in _get_team() after shared teams check - Include public teams in get_user_teams() query results via UNION - Add Pydantic schemas for public team/bot CRUD operations - Add admin API endpoints for managing public teams and bots - Add validation to ensure public teams only reference public bots - Add validation to ensure public bots only reference public resources Frontend changes: - Add TypeScript types and API methods for public teams/bots - Add public-teams and public-bots tabs to admin navigation - Create PublicTeamList component for admin interface - Create PublicBotList component for admin interface - Show "Public" badge for public teams in user's team list - Hide edit/delete buttons for public teams from regular users - Add i18n translations for both English and Chinese
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRemoves the old monolithic admin endpoint file and replaces it with a modular admin router exposing new admin CRUD APIs for public teams, bots, ghosts, shells, models, retrievers, keys, kinds, stats, tasks and system-config; adds new admin schemas, frontend admin UI tabs/components, i18n entries, and service/reader updates to include system-owned (user_id=0) resources. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Admin UI
participant API as Admin Router
participant Service as Service/Readers
participant DB as Database
UI->>API: GET /admin/public-{teams|bots|ghosts|shells}?page&limit
API->>Service: query public Kind records (user_id=0, kind=...)
Service->>DB: SELECT ... WHERE user_id=0 AND kind=... LIMIT/OFFSET
DB-->>Service: rows
Service-->>API: mapped Public*ListResponse
API-->>UI: 200 OK with items,total
UI->>API: POST /admin/public-{type} (create payload)
API->>Service: validate uniqueness and referenced public resources
Service->>DB: INSERT Kind (user_id=0) / UPDATE related tables
DB-->>Service: created row
Service-->>API: Public*Response
API-->>UI: 201 Created
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Import GlobeAltIcon from @heroicons/react/24/outline to match the existing icon pattern used in other admin components.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@backend/app/api/endpoints/admin.py`:
- Around line 1743-1783: The _validate_team_bot_references function currently
dereferences team_json["spec"], members and botRef without validating types,
which can raise AttributeError; add defensive shape checks: verify team_json is
a dict, spec is a dict, and members is a list before iterating, and for each
member ensure botRef is a dict and botRef.get("name") is a str (or skip if
missing/invalid) so you return (False, "<clear client-facing message>") for
malformed input instead of raising; keep existing logic that queries Kind via
db.query(...) when types are valid and return (True, None) only on success.
- Around line 2030-2109: The function _validate_bot_resource_references
currently assumes spec and the nested ghostRef/shellRef/modelRef are dicts and
calls .get on them, which can raise if they are not objects; update the function
to first validate that spec is a dict (isinstance(spec, dict)) and that
ghost_ref, shell_ref, and model_ref (when present) are dicts before using .get,
and if any are the wrong type return (False, "Invalid bot JSON: '<field>' must
be an object") so the caller can produce a 400. Ensure you reference and check
the variables spec, ghost_ref, shell_ref, and model_ref inside
_validate_bot_resource_references and return clear error messages for the
offending field.
- Around line 1785-1822: list_public_teams currently filters Kind by namespace
== "default" and pulls all rows into memory with .all() then slices, causing
teams in other namespaces to be hidden and inefficient pagination; remove the
namespace filter from the query (the Kind filter should remain: Kind.user_id ==
0, Kind.kind == "Team") and implement SQL-level pagination by replacing the
.all() + Python slicing with
query.order_by(Kind.updated_at.desc()).offset((page-1)*limit).limit(limit).all(),
then build and return the PublicTeamListResponse as before using the paginated
results.
- Around line 2111-2152: list_public_bots currently restricts public bots to
namespace == "default" and does in-memory pagination; remove the namespace
filter so the query filters only on Kind.user_id == 0 and Kind.kind == "Bot",
compute total = query.count(), then apply SQL-level pagination using
query.order_by(Kind.updated_at.desc()).offset((page-1)*limit).limit(limit).all()
to fetch paginated_bots; update the code paths inside list_public_bots that use
Kind, PublicBotResponse, and PublicBotListResponse accordingly.
- Around line 1723-2299: Add OpenTelemetry tracing by importing trace_async and
trace_sync from shared.telemetry.decorators and applying them: add `@trace_sync`
above the sync helpers _get_team_description, _get_team_display_name,
_validate_team_bot_references, _get_bot_ref_info, _get_bot_display_name, and
_validate_bot_resource_references; add `@trace_async` above the async endpoint
functions list_public_teams, create_public_team, update_public_team,
delete_public_team, list_public_bots, create_public_bot, and update_public_bot.
Place the import near other imports and ensure each decorator sits immediately
above the corresponding def signature so the observability pattern matches
existing guidelines.
- Around line 2302-2357: In delete_public_bot, the teams_using_bot query
incorrectly filters out inactive teams (Kind.is_active == True) so bots
referenced by inactive teams can be deleted; remove the Kind.is_active == True
condition from the db.query(Kind).filter(...) used to build teams_using_bot (and
any related is_active checks in that block), and update the function docstring
to state that both active and inactive public teams referencing the bot will
block deletion; keep all other logic (parsing team.json/spec/members and raising
the HTTPException with referencing_teams) unchanged.
🧹 Nitpick comments (2)
frontend/src/features/admin/components/PublicTeamList.tsx (1)
45-47: Use a single namespace inuseTranslation.Prefer
useTranslation('admin')and keep keys within that namespace, usingt('other:key')only when crossing namespaces. As per coding guidelines, please set a single namespace here.frontend/src/features/admin/components/PublicBotList.tsx (1)
45-46: Use a single namespace inuseTranslation.Prefer
useTranslation('admin')and keep keys within that namespace, usingt('other:key')only when crossing namespaces. As per coding guidelines, please set a single namespace here.
Comment on lines
+1723
to
+2299
| def _get_team_description(team: Kind) -> Optional[str]: | ||
| """Extract description from team json""" | ||
| if team.json and isinstance(team.json, dict): | ||
| spec = team.json.get("spec", {}) | ||
| if isinstance(spec, dict): | ||
| return spec.get("description") | ||
| return None | ||
|
|
||
|
|
||
| def _get_team_display_name(team: Kind) -> Optional[str]: | ||
| """Extract displayName from team json metadata""" | ||
| if team.json and isinstance(team.json, dict): | ||
| metadata = team.json.get("metadata", {}) | ||
| if isinstance(metadata, dict): | ||
| display_name = metadata.get("displayName") | ||
| if display_name and display_name != team.name: | ||
| return display_name | ||
| return None | ||
|
|
||
|
|
||
| def _validate_team_bot_references( | ||
| db: Session, team_json: dict | ||
| ) -> tuple[bool, Optional[str]]: | ||
| """ | ||
| Validate that all bots referenced by the team are public bots. | ||
|
|
||
| Returns: | ||
| (is_valid, error_message) | ||
| """ | ||
| spec = team_json.get("spec", {}) | ||
| members = spec.get("members", []) | ||
|
|
||
| for member in members: | ||
| bot_ref = member.get("botRef", {}) | ||
| bot_name = bot_ref.get("name") | ||
| bot_namespace = bot_ref.get("namespace", "default") | ||
|
|
||
| if not bot_name: | ||
| continue | ||
|
|
||
| # Check if bot exists as a public resource (user_id=0) | ||
| bot = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Bot", | ||
| Kind.name == bot_name, | ||
| Kind.namespace == bot_namespace, | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
|
|
||
| if not bot: | ||
| return ( | ||
| False, | ||
| f"Bot '{bot_namespace}/{bot_name}' is not a public resource. Please create it as a public bot first.", | ||
| ) | ||
|
|
||
| return (True, None) | ||
|
|
||
|
|
||
| @router.get("/public-teams", response_model=PublicTeamListResponse) | ||
| async def list_public_teams( | ||
| page: int = Query(1, ge=1), | ||
| limit: int = Query(20, ge=1, le=100), | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Get list of all public teams with pagination | ||
| """ | ||
| query = db.query(Kind).filter( | ||
| Kind.user_id == 0, Kind.kind == "Team", Kind.namespace == "default" | ||
| ) | ||
| total = query.count() | ||
| teams = query.order_by(Kind.updated_at.desc()).all() | ||
|
|
||
| # Apply pagination | ||
| start_idx = (page - 1) * limit | ||
| end_idx = start_idx + limit | ||
| paginated_teams = teams[start_idx:end_idx] | ||
|
|
||
| return PublicTeamListResponse( | ||
| total=total, | ||
| items=[ | ||
| PublicTeamResponse( | ||
| id=team.id, | ||
| name=team.name, | ||
| namespace=team.namespace, | ||
| display_name=_get_team_display_name(team), | ||
| description=_get_team_description(team), | ||
| team_json=team.json, | ||
| is_active=team.is_active, | ||
| created_at=team.created_at, | ||
| updated_at=team.updated_at, | ||
| ) | ||
| for team in paginated_teams | ||
| ], | ||
| ) | ||
|
|
||
|
|
||
| @router.post( | ||
| "/public-teams", | ||
| response_model=PublicTeamResponse, | ||
| status_code=status.HTTP_201_CREATED, | ||
| ) | ||
| async def create_public_team( | ||
| team_data: PublicTeamCreate, | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Create a new public team (admin only). | ||
| All bots referenced by the team must be public bots (user_id=0). | ||
| """ | ||
| # Check if team with same name and namespace already exists | ||
| existing_team = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Team", | ||
| Kind.name == team_data.name, | ||
| Kind.namespace == team_data.namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_team: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public team '{team_data.name}' already exists in namespace '{team_data.namespace}'", | ||
| ) | ||
|
|
||
| # Validate bot references | ||
| is_valid, error_message = _validate_team_bot_references(db, team_data.team_json) | ||
| if not is_valid: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=error_message, | ||
| ) | ||
|
|
||
| new_team = Kind( | ||
| user_id=0, | ||
| kind="Team", | ||
| name=team_data.name, | ||
| namespace=team_data.namespace, | ||
| json=team_data.team_json, | ||
| is_active=True, | ||
| ) | ||
| db.add(new_team) | ||
| db.commit() | ||
| db.refresh(new_team) | ||
|
|
||
| return PublicTeamResponse( | ||
| id=new_team.id, | ||
| name=new_team.name, | ||
| namespace=new_team.namespace, | ||
| display_name=_get_team_display_name(new_team), | ||
| description=_get_team_description(new_team), | ||
| team_json=new_team.json, | ||
| is_active=new_team.is_active, | ||
| created_at=new_team.created_at, | ||
| updated_at=new_team.updated_at, | ||
| ) | ||
|
|
||
|
|
||
| @router.put("/public-teams/{team_id}", response_model=PublicTeamResponse) | ||
| async def update_public_team( | ||
| team_data: PublicTeamUpdate, | ||
| team_id: int = Path(..., description="Team ID"), | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Update a public team (admin only). | ||
| All bots referenced by the team must be public bots (user_id=0). | ||
| """ | ||
| team = ( | ||
| db.query(Kind) | ||
| .filter(Kind.id == team_id, Kind.user_id == 0, Kind.kind == "Team") | ||
| .first() | ||
| ) | ||
| if not team: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail=f"Public team with id {team_id} not found", | ||
| ) | ||
|
|
||
| # Check name uniqueness if being changed | ||
| if team_data.name and team_data.name != team.name: | ||
| namespace = team_data.namespace or team.namespace | ||
| existing_team = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Team", | ||
| Kind.name == team_data.name, | ||
| Kind.namespace == namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_team: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public team '{team_data.name}' already exists in namespace '{namespace}'", | ||
| ) | ||
|
|
||
| # Validate bot references if json is being updated | ||
| if team_data.team_json: | ||
| is_valid, error_message = _validate_team_bot_references(db, team_data.team_json) | ||
| if not is_valid: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=error_message, | ||
| ) | ||
|
|
||
| # Update fields | ||
| if team_data.name is not None: | ||
| team.name = team_data.name | ||
| if team_data.namespace is not None: | ||
| team.namespace = team_data.namespace | ||
| if team_data.team_json is not None: | ||
| team.json = team_data.team_json | ||
| if team_data.is_active is not None: | ||
| team.is_active = team_data.is_active | ||
|
|
||
| db.commit() | ||
| db.refresh(team) | ||
|
|
||
| return PublicTeamResponse( | ||
| id=team.id, | ||
| name=team.name, | ||
| namespace=team.namespace, | ||
| display_name=_get_team_display_name(team), | ||
| description=_get_team_description(team), | ||
| team_json=team.json, | ||
| is_active=team.is_active, | ||
| created_at=team.created_at, | ||
| updated_at=team.updated_at, | ||
| ) | ||
|
|
||
|
|
||
| @router.delete("/public-teams/{team_id}", status_code=status.HTTP_204_NO_CONTENT) | ||
| async def delete_public_team( | ||
| team_id: int = Path(..., description="Team ID"), | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Delete a public team (admin only) | ||
| """ | ||
| team = ( | ||
| db.query(Kind) | ||
| .filter(Kind.id == team_id, Kind.user_id == 0, Kind.kind == "Team") | ||
| .first() | ||
| ) | ||
| if not team: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail=f"Public team with id {team_id} not found", | ||
| ) | ||
|
|
||
| db.delete(team) | ||
| db.commit() | ||
|
|
||
| return None | ||
|
|
||
|
|
||
| # ==================== Public Bot Management Endpoints ==================== | ||
|
|
||
|
|
||
| def _get_bot_ref_info( | ||
| bot: Kind, | ||
| ) -> tuple[Optional[str], Optional[str], Optional[str]]: | ||
| """Extract ghost, shell, and model reference names from bot json""" | ||
| ghost_name = None | ||
| shell_name = None | ||
| model_name = None | ||
|
|
||
| if bot.json and isinstance(bot.json, dict): | ||
| spec = bot.json.get("spec", {}) | ||
| if isinstance(spec, dict): | ||
| ghost_ref = spec.get("ghostRef", {}) | ||
| shell_ref = spec.get("shellRef", {}) | ||
| model_ref = spec.get("modelRef", {}) | ||
|
|
||
| if isinstance(ghost_ref, dict): | ||
| ghost_name = ghost_ref.get("name") | ||
| if isinstance(shell_ref, dict): | ||
| shell_name = shell_ref.get("name") | ||
| if isinstance(model_ref, dict): | ||
| model_name = model_ref.get("name") | ||
|
|
||
| return (ghost_name, shell_name, model_name) | ||
|
|
||
|
|
||
| def _get_bot_display_name(bot: Kind) -> Optional[str]: | ||
| """Extract displayName from bot json metadata""" | ||
| if bot.json and isinstance(bot.json, dict): | ||
| metadata = bot.json.get("metadata", {}) | ||
| if isinstance(metadata, dict): | ||
| display_name = metadata.get("displayName") | ||
| if display_name and display_name != bot.name: | ||
| return display_name | ||
| return None | ||
|
|
||
|
|
||
| def _validate_bot_resource_references( | ||
| db: Session, bot_json: dict | ||
| ) -> tuple[bool, Optional[str]]: | ||
| """ | ||
| Validate that all resources referenced by the bot (Ghost, Shell, Model) are public. | ||
|
|
||
| Returns: | ||
| (is_valid, error_message) | ||
| """ | ||
| spec = bot_json.get("spec", {}) | ||
|
|
||
| # Validate ghostRef | ||
| ghost_ref = spec.get("ghostRef", {}) | ||
| ghost_name = ghost_ref.get("name") | ||
| ghost_namespace = ghost_ref.get("namespace", "default") | ||
| if ghost_name: | ||
| ghost = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Ghost", | ||
| Kind.name == ghost_name, | ||
| Kind.namespace == ghost_namespace, | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
| if not ghost: | ||
| return ( | ||
| False, | ||
| f"Ghost '{ghost_namespace}/{ghost_name}' is not a public resource. Please create it as a public ghost first.", | ||
| ) | ||
|
|
||
| # Validate shellRef | ||
| shell_ref = spec.get("shellRef", {}) | ||
| shell_name = shell_ref.get("name") | ||
| shell_namespace = shell_ref.get("namespace", "default") | ||
| if shell_name: | ||
| shell = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Shell", | ||
| Kind.name == shell_name, | ||
| Kind.namespace == shell_namespace, | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
| if not shell: | ||
| return ( | ||
| False, | ||
| f"Shell '{shell_namespace}/{shell_name}' is not a public resource. Please create it as a public shell first.", | ||
| ) | ||
|
|
||
| # Validate modelRef (optional) | ||
| model_ref = spec.get("modelRef") | ||
| if model_ref: | ||
| model_name = model_ref.get("name") | ||
| model_namespace = model_ref.get("namespace", "default") | ||
| if model_name: | ||
| model = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Model", | ||
| Kind.name == model_name, | ||
| Kind.namespace == model_namespace, | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
| if not model: | ||
| return ( | ||
| False, | ||
| f"Model '{model_namespace}/{model_name}' is not a public resource. Please create it as a public model first.", | ||
| ) | ||
|
|
||
| return (True, None) | ||
|
|
||
|
|
||
| @router.get("/public-bots", response_model=PublicBotListResponse) | ||
| async def list_public_bots( | ||
| page: int = Query(1, ge=1), | ||
| limit: int = Query(20, ge=1, le=100), | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Get list of all public bots with pagination | ||
| """ | ||
| query = db.query(Kind).filter( | ||
| Kind.user_id == 0, Kind.kind == "Bot", Kind.namespace == "default" | ||
| ) | ||
| total = query.count() | ||
| bots = query.order_by(Kind.updated_at.desc()).all() | ||
|
|
||
| # Apply pagination | ||
| start_idx = (page - 1) * limit | ||
| end_idx = start_idx + limit | ||
| paginated_bots = bots[start_idx:end_idx] | ||
|
|
||
| items = [] | ||
| for bot in paginated_bots: | ||
| ghost_name, shell_name, model_name = _get_bot_ref_info(bot) | ||
| items.append( | ||
| PublicBotResponse( | ||
| id=bot.id, | ||
| name=bot.name, | ||
| namespace=bot.namespace, | ||
| display_name=_get_bot_display_name(bot), | ||
| bot_json=bot.json, | ||
| is_active=bot.is_active, | ||
| created_at=bot.created_at, | ||
| updated_at=bot.updated_at, | ||
| ghost_name=ghost_name, | ||
| shell_name=shell_name, | ||
| model_name=model_name, | ||
| ) | ||
| ) | ||
|
|
||
| return PublicBotListResponse(total=total, items=items) | ||
|
|
||
|
|
||
| @router.post( | ||
| "/public-bots", | ||
| response_model=PublicBotResponse, | ||
| status_code=status.HTTP_201_CREATED, | ||
| ) | ||
| async def create_public_bot( | ||
| bot_data: PublicBotCreate, | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Create a new public bot (admin only). | ||
| All resources referenced by the bot (Ghost, Shell, Model) must be public (user_id=0). | ||
| """ | ||
| # Check if bot with same name and namespace already exists | ||
| existing_bot = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Bot", | ||
| Kind.name == bot_data.name, | ||
| Kind.namespace == bot_data.namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_bot: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public bot '{bot_data.name}' already exists in namespace '{bot_data.namespace}'", | ||
| ) | ||
|
|
||
| # Validate resource references | ||
| is_valid, error_message = _validate_bot_resource_references(db, bot_data.bot_json) | ||
| if not is_valid: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=error_message, | ||
| ) | ||
|
|
||
| new_bot = Kind( | ||
| user_id=0, | ||
| kind="Bot", | ||
| name=bot_data.name, | ||
| namespace=bot_data.namespace, | ||
| json=bot_data.bot_json, | ||
| is_active=True, | ||
| ) | ||
| db.add(new_bot) | ||
| db.commit() | ||
| db.refresh(new_bot) | ||
|
|
||
| ghost_name, shell_name, model_name = _get_bot_ref_info(new_bot) | ||
| return PublicBotResponse( | ||
| id=new_bot.id, | ||
| name=new_bot.name, | ||
| namespace=new_bot.namespace, | ||
| display_name=_get_bot_display_name(new_bot), | ||
| bot_json=new_bot.json, | ||
| is_active=new_bot.is_active, | ||
| created_at=new_bot.created_at, | ||
| updated_at=new_bot.updated_at, | ||
| ghost_name=ghost_name, | ||
| shell_name=shell_name, | ||
| model_name=model_name, | ||
| ) | ||
|
|
||
|
|
||
| @router.put("/public-bots/{bot_id}", response_model=PublicBotResponse) | ||
| async def update_public_bot( | ||
| bot_data: PublicBotUpdate, | ||
| bot_id: int = Path(..., description="Bot ID"), | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(get_admin_user), | ||
| ): | ||
| """ | ||
| Update a public bot (admin only). | ||
| All resources referenced by the bot (Ghost, Shell, Model) must be public (user_id=0). | ||
| """ | ||
| bot = ( | ||
| db.query(Kind) | ||
| .filter(Kind.id == bot_id, Kind.user_id == 0, Kind.kind == "Bot") | ||
| .first() | ||
| ) | ||
| if not bot: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail=f"Public bot with id {bot_id} not found", | ||
| ) | ||
|
|
||
| # Check name uniqueness if being changed | ||
| if bot_data.name and bot_data.name != bot.name: | ||
| namespace = bot_data.namespace or bot.namespace | ||
| existing_bot = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Bot", | ||
| Kind.name == bot_data.name, | ||
| Kind.namespace == namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_bot: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public bot '{bot_data.name}' already exists in namespace '{namespace}'", | ||
| ) | ||
|
|
||
| # Validate resource references if json is being updated | ||
| if bot_data.bot_json: | ||
| is_valid, error_message = _validate_bot_resource_references( | ||
| db, bot_data.bot_json | ||
| ) | ||
| if not is_valid: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=error_message, | ||
| ) | ||
|
|
||
| # Update fields | ||
| if bot_data.name is not None: | ||
| bot.name = bot_data.name | ||
| if bot_data.namespace is not None: | ||
| bot.namespace = bot_data.namespace | ||
| if bot_data.bot_json is not None: | ||
| bot.json = bot_data.bot_json | ||
| if bot_data.is_active is not None: | ||
| bot.is_active = bot_data.is_active | ||
|
|
||
| db.commit() | ||
| db.refresh(bot) | ||
|
|
||
| ghost_name, shell_name, model_name = _get_bot_ref_info(bot) | ||
| return PublicBotResponse( | ||
| id=bot.id, | ||
| name=bot.name, | ||
| namespace=bot.namespace, | ||
| display_name=_get_bot_display_name(bot), | ||
| bot_json=bot.json, | ||
| is_active=bot.is_active, | ||
| created_at=bot.created_at, | ||
| updated_at=bot.updated_at, | ||
| ghost_name=ghost_name, | ||
| shell_name=shell_name, | ||
| model_name=model_name, | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check one of those files to see the import pattern and usage
rg -B5 -A2 "@trace_async|@trace_sync" backend/app/api/endpoints/models.py | head -50Repository: wecode-ai/Wegent
Length of output: 135
🏁 Script executed:
# List all endpoint files in the backend
ls -la backend/app/api/endpoints/Repository: wecode-ai/Wegent
Length of output: 1502
🏁 Script executed:
# Check for tracing decorators in several endpoint files
rg "@trace_async|@trace_sync" backend/app/api/endpoints/knowledge.py backend/app/api/endpoints/rag.py backend/app/api/endpoints/wizard.py | head -20Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Check the imports at the top of admin.py
head -50 backend/app/api/endpoints/admin.py | grep -E "^from|^import"Repository: wecode-ai/Wegent
Length of output: 648
🏁 Script executed:
# Search for any tracing imports in admin.py
rg "trace" backend/app/api/endpoints/admin.py | head -20Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Search across the entire codebase for tracing decorator usage
rg "@trace_async|@trace_sync" backend/ --type pyRepository: wecode-ai/Wegent
Length of output: 285
🏁 Script executed:
# Check where trace decorators are imported from
rg "from.*trace_sync|from.*trace_async|import.*trace_sync|import.*trace_async" backend/ --type pyRepository: wecode-ai/Wegent
Length of output: 283
🏁 Script executed:
# Now let's check the actual functions at lines 1723-2299 in admin.py
sed -n '1723,2299p' backend/app/api/endpoints/admin.py | grep -E "^async def|^def|@trace" | head -20Repository: wecode-ai/Wegent
Length of output: 529
🏁 Script executed:
# Check if there are other async functions in admin.py that have tracing decorators
rg -B2 "^async def" backend/app/api/endpoints/admin.py | grep -E "@trace_async|async def" | head -20Repository: wecode-ai/Wegent
Length of output: 708
🏁 Script executed:
# Check the beginning and middle of admin.py to see if ANY async functions have tracing
sed -n '1,200p' backend/app/api/endpoints/admin.py | grep -B2 "@trace"Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Check the entire admin.py file to see if any function has tracing decorators
grep -c "@trace" backend/app/api/endpoints/admin.pyRepository: wecode-ai/Wegent
Length of output: 61
🏁 Script executed:
# Check if the shared.telemetry.decorators module exists and what it contains
cat backend/shared/telemetry/decorators.py 2>/dev/null | head -50Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Find the telemetry decorators file
find backend -name "decorators.py" -path "*/telemetry/*" 2>/dev/nullRepository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Search for telemetry module anywhere in the codebase
find backend -type d -name "telemetry" 2>/dev/nullRepository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Check if the files that use trace decorators actually exist
ls -la backend/app/services/correction_service.py backend/app/services/chat/trigger/core.py 2>&1Repository: wecode-ai/Wegent
Length of output: 244
🏁 Script executed:
# Check the actual import statement in correction_service.py
rg "from shared.telemetry" backend/app/services/correction_service.py -A 3Repository: wecode-ai/Wegent
Length of output: 250
Add OpenTelemetry tracing decorators to public team and bot management functions.
The async endpoints (list_public_teams, create_public_team, update_public_team, delete_public_team, list_public_bots, create_public_bot, update_public_bot) must use @trace_async(), and the sync helpers (_get_team_description, _get_team_display_name, _validate_team_bot_references, _get_bot_ref_info, _get_bot_display_name, _validate_bot_resource_references) must use @trace_sync(). Import the decorators from shared.telemetry.decorators and apply them to match the observability pattern required by the coding guidelines.
🧰 Tools
🪛 Ruff (0.14.13)
1771-1771: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
1789-1789: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1790-1790: Unused function argument: current_user
(ARG001)
1790-1790: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1832-1832: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1833-1833: Unused function argument: current_user
(ARG001)
1833-1833: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1893-1893: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1894-1894: Unused function argument: current_user
(ARG001)
1894-1894: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1968-1968: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1969-1969: Unused function argument: current_user
(ARG001)
1969-1969: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2053-2053: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
2075-2075: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
2098-2098: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
2115-2115: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2116-2116: Unused function argument: current_user
(ARG001)
2116-2116: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2161-2161: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2162-2162: Unused function argument: current_user
(ARG001)
2162-2162: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2225-2225: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2226-2226: Unused function argument: current_user
(ARG001)
2226-2226: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin.py` around lines 1723 - 2299, Add
OpenTelemetry tracing by importing trace_async and trace_sync from
shared.telemetry.decorators and applying them: add `@trace_sync` above the sync
helpers _get_team_description, _get_team_display_name,
_validate_team_bot_references, _get_bot_ref_info, _get_bot_display_name, and
_validate_bot_resource_references; add `@trace_async` above the async endpoint
functions list_public_teams, create_public_team, update_public_team,
delete_public_team, list_public_bots, create_public_bot, and update_public_bot.
Place the import near other imports and ensure each decorator sits immediately
above the corresponding def signature so the observability pattern matches
existing guidelines.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/src/features/admin/components/PublicBotList.tsx`:
- Around line 45-47: The useTranslation call in the PublicBotList React
component is missing a translation namespace; update the useTranslation
invocation inside the PublicBotList component to pass the correct single
namespace (for example "admin" or the module-specific namespace used in your
app) so keys resolve unambiguously—locate the useTranslation() call in
PublicBotList and change it to useTranslation('your-namespace') and update any
t(...) keys if necessary to match that namespace.
- Around line 154-178: handleUpdateBot currently allows blank/whitespace
name/namespace values (unlike create) which can cause backend 400s; before
building AdminPublicBotUpdate, trim formData.name and formData.namespace,
validate that trimmedName is non-empty (show the existing toast error for
invalid name and return), and only assign updateData.name and
updateData.namespace when the trimmed values differ from
selectedBot.name/namespace; keep using validateConfig for json as-is. Ensure you
reference the handleUpdateBot function, formData.name/formData.namespace, and
AdminPublicBotUpdate when applying these changes.
- Change list_public_teams and list_public_bots to use SQL-level pagination instead of in-memory pagination for better performance - Add defensive type checking in _validate_team_bot_references and _validate_bot_resource_references to handle malformed JSON inputs - Fix delete_public_bot to check both active and inactive teams when preventing bot deletion due to team references - Update frontend components to use single 'admin' namespace in useTranslation hook following project i18n conventions
Add trim and empty validation for name in handleUpdateBot and handleUpdateTeam to prevent whitespace-only names from being submitted to the backend.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/app/api/endpoints/admin.py`:
- Around line 2357-2384: The delete handlers (delete_public_bot,
delete_public_ghost, delete_public_shell) assume team.json/spec/members/member
are well-formed and will raise on malformed data; update the loop that builds
referencing_teams (starting from teams_using_bot) to defensively validate types:
ensure team.json is a dict, spec = team.json.get("spec", {}) is a dict, members
is a list, each member is a dict, and botRef (member.get("botRef")) is a dict
before accessing .get("name")/.get("namespace"); if any check fails, skip that
team/member and continue; apply the same guarded logic to the other two similar
blocks (the ones around the other indicated ranges) so malformed JSON entries
are ignored instead of causing a 500.
In `@frontend/src/features/admin/components/PublicBotList.tsx`:
- Around line 438-454: In PublicBotList.tsx replace the hardcoded labels
"Ghost:", "Shell:", and "Model:" with i18n keys (use the existing
useTranslation/t function in this component) so the UI strings are localized;
update the JSX where bot.ghost_name, bot.shell_name, and bot.model_name are
rendered to call t('crd.ghost') / t('crd.shell') / t('crd.model') (or the
project's agreed CRD key names) instead of raw text, and ensure the component
imports and uses the i18n hook (e.g., useTranslation) if it doesn't already.
In `@frontend/src/features/admin/components/PublicGhostList.tsx`:
- Around line 113-176: The config-empty case is being reported as "invalid JSON"
even though validateConfig marks it as "required"; update both handleCreateGhost
and handleUpdateGhost to first check if formData.config.trim() is empty and
toast the t('public_ghosts.errors.config_required') message, otherwise call
validateConfig and if it returns falsy toast
t('public_ghosts.errors.config_invalid_json'); keep references to
validateConfig, formData.config, handleCreateGhost and handleUpdateGhost so the
change is applied in both create and update flows.
In `@frontend/src/features/admin/components/PublicShellList.tsx`:
- Around line 306-310: Replace the hardcoded "Type:" label in the
PublicShellList component (the JSX that renders shell.shell_type) with an i18n
lookup instead of a literal string: use the component’s existing translation
mechanism (e.g., the t function from useTranslation) to render a translatable
key (suggestion: a CRD-style key like "crd.shell.type" or "shell.type"), update
the locales/translation files to include that key/value for supported languages,
and ensure the JSX now shows t('crd.shell.type') + shell.shell_type (or
equivalent formatting) where the previous "Type:" was rendered.
♻️ Duplicate comments (1)
backend/app/api/endpoints/admin.py (1)
1731-2143: Add OpenTelemetry tracing decorators to the new public resource endpoints/helpers.The new helpers and endpoints here are missing
@trace_sync()/@trace_async()as required for backend observability. Please add the decorators and import them fromshared.telemetry.decorators.As per coding guidelines.
Also applies to: 2145-2863
🧹 Nitpick comments (2)
frontend/src/features/admin/components/PublicShellList.tsx (1)
240-249: Clear stale config errors when opening dialogs.
configErrorcan persist across create/edit flows. Consider resetting it (and form state) when opening a dialog to avoid showing old validation errors.♻️ Proposed tweak
const openEditDialog = (shell: AdminPublicShell) => { + setConfigError('') setSelectedShell(shell) setFormData({ name: shell.name, namespace: shell.namespace, config: JSON.stringify(shell.json, null, 2), is_active: shell.is_active, }) setIsEditDialogOpen(true) } @@ -<UnifiedAddButton onClick={() => setIsCreateDialogOpen(true)}> +<UnifiedAddButton + onClick={() => { + resetForm() + setIsCreateDialogOpen(true) + }} +>Also applies to: 348-349
frontend/src/features/admin/components/PublicBotList.tsx (1)
338-347: Reset stale validation state when opening dialogs.
configErrorcan linger between create/edit sessions. Resetting it (and form data for create) prevents showing old errors to admins.♻️ Proposed tweak
const openEditDialog = (bot: AdminPublicBot) => { + setConfigError('') setSelectedBot(bot) setFormData({ name: bot.name, namespace: bot.namespace, config: JSON.stringify(bot.json, null, 2), is_active: bot.is_active, }) setIsEditDialogOpen(true) } @@ -<UnifiedAddButton onClick={() => setIsCreateDialogOpen(true)}> +<UnifiedAddButton + onClick={() => { + resetForm() + setIsCreateDialogOpen(true) + }} +>Also applies to: 492-495
Comment on lines
+2357
to
+2384
| # Check if any public teams (active or inactive) reference this bot | ||
| teams_using_bot = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Team", | ||
| ) | ||
| .all() | ||
| ) | ||
|
|
||
| referencing_teams = [] | ||
| for team in teams_using_bot: | ||
| spec = team.json.get("spec", {}) if team.json else {} | ||
| members = spec.get("members", []) | ||
| for member in members: | ||
| bot_ref = member.get("botRef", {}) | ||
| if ( | ||
| bot_ref.get("name") == bot.name | ||
| and bot_ref.get("namespace", "default") == bot.namespace | ||
| ): | ||
| referencing_teams.append(team.name) | ||
| break | ||
|
|
||
| if referencing_teams: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Cannot delete bot '{bot.name}' because it is referenced by public teams: {', '.join(referencing_teams)}", | ||
| ) |
Contributor
There was a problem hiding this comment.
Guard delete reference scans against malformed JSON.
delete_public_bot, delete_public_ghost, and delete_public_shell assume json/spec/members are always dict/list. If malformed data exists, .get will raise and the delete will 500. Add type guards and skip invalid entries.
🛠️ Suggested fix (apply similarly in ghost/shell delete loops)
- for team in teams_using_bot:
- spec = team.json.get("spec", {}) if team.json else {}
- members = spec.get("members", [])
- for member in members:
- bot_ref = member.get("botRef", {})
+ for team in teams_using_bot:
+ if not isinstance(team.json, dict):
+ continue
+ spec = team.json.get("spec", {})
+ if not isinstance(spec, dict):
+ continue
+ members = spec.get("members", [])
+ if not isinstance(members, list):
+ continue
+ for member in members:
+ if not isinstance(member, dict):
+ continue
+ bot_ref = member.get("botRef", {})
+ if not isinstance(bot_ref, dict):
+ continue
if (
bot_ref.get("name") == bot.name
and bot_ref.get("namespace", "default") == bot.namespace
):
referencing_teams.append(team.name)
breakAlso applies to: 2597-2621, 2834-2857
🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin.py` around lines 2357 - 2384, The delete
handlers (delete_public_bot, delete_public_ghost, delete_public_shell) assume
team.json/spec/members/member are well-formed and will raise on malformed data;
update the loop that builds referencing_teams (starting from teams_using_bot) to
defensively validate types: ensure team.json is a dict, spec =
team.json.get("spec", {}) is a dict, members is a list, each member is a dict,
and botRef (member.get("botRef")) is a dict before accessing
.get("name")/.get("namespace"); if any check fails, skip that team/member and
continue; apply the same guarded logic to the other two similar blocks (the ones
around the other indicated ranges) so malformed JSON entries are ignored instead
of causing a 500.
Comment on lines
+438
to
+454
| {bot.ghost_name && ( | ||
| <> | ||
| <span>*</span> | ||
| <span>Ghost: {bot.ghost_name}</span> | ||
| </> | ||
| )} | ||
| {bot.shell_name && ( | ||
| <> | ||
| <span>*</span> | ||
| <span>Shell: {bot.shell_name}</span> | ||
| </> | ||
| )} | ||
| {bot.model_name && ( | ||
| <> | ||
| <span>*</span> | ||
| <span>Model: {bot.model_name}</span> | ||
| </> |
Contributor
There was a problem hiding this comment.
Localize the inline resource labels.
“Ghost:”, “Shell:”, and “Model:” are hardcoded. Please route these through i18n keys for consistency.
🔧 Suggested change
-<span>Ghost: {bot.ghost_name}</span>
+<span>{t('public_bots.columns.ghost')}: {bot.ghost_name}</span>
@@
-<span>Shell: {bot.shell_name}</span>
+<span>{t('public_bots.columns.shell')}: {bot.shell_name}</span>
@@
-<span>Model: {bot.model_name}</span>
+<span>{t('public_bots.columns.model')}: {bot.model_name}</span>Based on learnings, keep user-facing labels behind i18n keys that use CRD terms.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {bot.ghost_name && ( | |
| <> | |
| <span>*</span> | |
| <span>Ghost: {bot.ghost_name}</span> | |
| </> | |
| )} | |
| {bot.shell_name && ( | |
| <> | |
| <span>*</span> | |
| <span>Shell: {bot.shell_name}</span> | |
| </> | |
| )} | |
| {bot.model_name && ( | |
| <> | |
| <span>*</span> | |
| <span>Model: {bot.model_name}</span> | |
| </> | |
| {bot.ghost_name && ( | |
| <> | |
| <span>*</span> | |
| <span>{t('public_bots.columns.ghost')}: {bot.ghost_name}</span> | |
| </> | |
| )} | |
| {bot.shell_name && ( | |
| <> | |
| <span>*</span> | |
| <span>{t('public_bots.columns.shell')}: {bot.shell_name}</span> | |
| </> | |
| )} | |
| {bot.model_name && ( | |
| <> | |
| <span>*</span> | |
| <span>{t('public_bots.columns.model')}: {bot.model_name}</span> | |
| </> |
🤖 Prompt for AI Agents
In `@frontend/src/features/admin/components/PublicBotList.tsx` around lines 438 -
454, In PublicBotList.tsx replace the hardcoded labels "Ghost:", "Shell:", and
"Model:" with i18n keys (use the existing useTranslation/t function in this
component) so the UI strings are localized; update the JSX where bot.ghost_name,
bot.shell_name, and bot.model_name are rendered to call t('crd.ghost') /
t('crd.shell') / t('crd.model') (or the project's agreed CRD key names) instead
of raw text, and ensure the component imports and uses the i18n hook (e.g.,
useTranslation) if it doesn't already.
Comment on lines
+113
to
+176
| const handleCreateGhost = async () => { | ||
| if (!formData.name.trim()) { | ||
| toast({ | ||
| variant: 'destructive', | ||
| title: t('public_ghosts.errors.name_required'), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| const config = validateConfig(formData.config) | ||
| if (!config) { | ||
| toast({ | ||
| variant: 'destructive', | ||
| title: t('public_ghosts.errors.config_invalid_json'), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| setSaving(true) | ||
| try { | ||
| const createData: AdminPublicGhostCreate = { | ||
| name: formData.name.trim(), | ||
| namespace: formData.namespace.trim() || 'default', | ||
| json: config, | ||
| } | ||
| await adminApis.createPublicGhost(createData) | ||
| toast({ title: t('public_ghosts.success.created') }) | ||
| setIsCreateDialogOpen(false) | ||
| resetForm() | ||
| fetchGhosts() | ||
| } catch (error) { | ||
| toast({ | ||
| variant: 'destructive', | ||
| title: t('public_ghosts.errors.create_failed'), | ||
| description: (error as Error).message, | ||
| }) | ||
| } finally { | ||
| setSaving(false) | ||
| } | ||
| } | ||
|
|
||
| const handleUpdateGhost = async () => { | ||
| if (!selectedGhost) return | ||
|
|
||
| // Trim and validate name | ||
| const trimmedName = formData.name.trim() | ||
| const trimmedNamespace = formData.namespace.trim() | ||
|
|
||
| if (!trimmedName) { | ||
| toast({ | ||
| variant: 'destructive', | ||
| title: t('public_ghosts.errors.name_required'), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| const config = validateConfig(formData.config) | ||
| if (!config) { | ||
| toast({ | ||
| variant: 'destructive', | ||
| title: t('public_ghosts.errors.config_invalid_json'), | ||
| }) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Align config-error toast with the actual validation result.
When config is empty, validateConfig sets the “required” error, but the submit path always toasts “invalid JSON,” which is misleading.
💡 Suggested fix (apply to create + update)
- const config = validateConfig(formData.config)
- if (!config) {
- toast({
- variant: 'destructive',
- title: t('public_ghosts.errors.config_invalid_json'),
- })
- return
- }
+ if (!formData.config.trim()) {
+ toast({
+ variant: 'destructive',
+ title: t('public_ghosts.errors.config_required'),
+ })
+ return
+ }
+ const config = validateConfig(formData.config)
+ if (!config) {
+ toast({
+ variant: 'destructive',
+ title: t('public_ghosts.errors.config_invalid_json'),
+ })
+ return
+ }Also applies to: 154-176
🤖 Prompt for AI Agents
In `@frontend/src/features/admin/components/PublicGhostList.tsx` around lines 113
- 176, The config-empty case is being reported as "invalid JSON" even though
validateConfig marks it as "required"; update both handleCreateGhost and
handleUpdateGhost to first check if formData.config.trim() is empty and toast
the t('public_ghosts.errors.config_required') message, otherwise call
validateConfig and if it returns falsy toast
t('public_ghosts.errors.config_invalid_json'); keep references to
validateConfig, formData.config, handleCreateGhost and handleUpdateGhost so the
change is applied in both create and update flows.
Comment on lines
+306
to
+310
| {shell.shell_type && ( | ||
| <> | ||
| <span>*</span> | ||
| <span>Type: {shell.shell_type}</span> | ||
| </> |
Contributor
There was a problem hiding this comment.
Localize the shell type label.
Line 309 hardcodes “Type:”. Please move this to i18n so it’s consistent with the rest of the UI and translatable.
🔧 Suggested change
- <span>Type: {shell.shell_type}</span>
+ <span>{t('public_shells.columns.type')}: {shell.shell_type}</span>Based on learnings, keep user-facing labels behind i18n keys that use CRD terms.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {shell.shell_type && ( | |
| <> | |
| <span>*</span> | |
| <span>Type: {shell.shell_type}</span> | |
| </> | |
| {shell.shell_type && ( | |
| <> | |
| <span>*</span> | |
| <span>{t('public_shells.columns.type')}: {shell.shell_type}</span> | |
| </> |
🤖 Prompt for AI Agents
In `@frontend/src/features/admin/components/PublicShellList.tsx` around lines 306
- 310, Replace the hardcoded "Type:" label in the PublicShellList component (the
JSX that renders shell.shell_type) with an i18n lookup instead of a literal
string: use the component’s existing translation mechanism (e.g., the t function
from useTranslation) to render a translatable key (suggestion: a CRD-style key
like "crd.shell.type" or "shell.type"), update the locales/translation files to
include that key/value for supported languages, and ensure the JSX now shows
t('crd.shell.type') + shell.shell_type (or equivalent formatting) where the
previous "Type:" was rendered.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/features/tasks/components/chat/useChatAreaState.ts`:
- Around line 359-380: The matching logic in useChatAreaState can miss teams
when defaultConfig.namespace is undefined; normalize the namespace before
filtering by computing a normalizedNamespace = defaultConfig.namespace ||
'default' (or set it on defaultConfig) and use that in the teams.filter
predicate and subsequent console.log, so matchedTeams = teams.filter(team =>
team.name === defaultConfig.name && (team.namespace || 'default') ===
normalizedNamespace) and the selection logic (publicTeam/selectTeam) remains
unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@backend/app/api/endpoints/admin/kind_management.py`:
- Around line 238-269: The admin_delete_user_resource handler currently returns
a JSON dict while decorated with status_code=status.HTTP_204_NO_CONTENT; change
it to return no body by removing the dict return and returning None (or an
explicit fastapi.Response with status_code=status.HTTP_204_NO_CONTENT) after
calling kind_service.delete_resource; locate the admin_delete_user_resource
function and replace the final return block so the endpoint yields no content
for 204 responses.
In `@backend/app/api/endpoints/admin/public_bots.py`:
- Around line 278-312: The uniqueness check currently only runs when
bot_data.name changes; update it to run whenever either bot_data.name or
bot_data.namespace is being changed to avoid creating duplicates when only
namespace is updated. Specifically, in the public bot update handler in
public_bots.py, replace the "if bot_data.name and bot_data.name != bot.name:"
guard with a check that detects changes to name or namespace (e.g., if
(bot_data.name is not None and bot_data.name != bot.name) or (bot_data.namespace
is not None and bot_data.namespace != bot.namespace)), compute the target name
as bot_data.name or bot.name and target namespace as bot_data.namespace or
bot.namespace, then run the existing Kind query (Kind.user_id == 0, Kind.kind ==
"Bot", Kind.name == target_name, Kind.namespace == target_namespace) and raise
the same HTTPException if existing_bot is found; keep the subsequent resource
validation (_validate_bot_resource_references) and field assignments unchanged.
In `@backend/app/api/endpoints/admin/public_ghosts.py`:
- Around line 154-171: The uniqueness check currently runs only when
ghost_data.name changes (in the block referencing ghost_data, ghost, and Kind),
so a user can change only the namespace and bypass the check; update the
conditional to run the uniqueness lookup when either the name or the namespace
is being changed (i.e., if ghost_data.name and ghost_data.name != ghost.name OR
ghost_data.namespace and ghost_data.namespace != ghost.namespace), then compute
the effective name and namespace (name = ghost_data.name or ghost.name,
namespace = ghost_data.namespace or ghost.namespace) and perform the existing
db.query(Kind)...first() check and raise the same HTTPException if an
existing_ghost is found.
In `@backend/app/api/endpoints/admin/public_models.py`:
- Around line 183-206: In delete_public_model, add a referential-integrity check
before deleting the Kind record: query the Bot model for any public bots
(Bot.user_id == 0) whose spec references this model (check
Bot.spec.get("modelRef") or the equivalent accessor) and if any exist, raise an
HTTPException (e.g., 400/409) with a message that the model is in use; only
proceed to db.delete(model) and db.commit() when that query returns none.
Reference the delete_public_model function, Kind, Bot, and the spec.modelRef
field when implementing this check to mirror the behavior in
delete_public_ghost.
- Around line 62-74: The current code pulls all rows with query.all() and does
Python-level sorting via _get_display_name then slices paginated_models, which
defeats DB pagination; change the logic to perform sorting in SQL and apply
offset/limit on the query itself: modify the query for Kind (the variable query)
to include an order_by that extracts the JSON displayName (using SQL COALESCE to
fall back to another field like name or Kind.kind if displayName is null) so the
DB returns rows already ordered, then call query.count() if needed and use
query.offset((page-1)*limit).limit(limit).all() instead of
sorted_models/paginated_models; if your ORM cannot express JSON
extraction/COALESCE for displayName, add a clear comment/documentation near
_get_display_name and the pagination branch describing this known limitation and
warn about performance for large datasets.
- Around line 62-64: The list_public_models endpoint currently filters public
models to namespace "default"; update the query in list_public_models to remove
the namespace filter so it returns all public Model kinds across namespaces
(i.e., change the db.query(Kind).filter(...) used in list_public_models to only
filter Kind.user_id == 0 and Kind.kind == "Model"), and ensure no other logic in
list_public_models relies on namespace; alternatively if you prefer to restrict
creation, enforce namespace=="default" validation in the model creation
handler—pick one approach and make tests reflect the chosen behavior.
In `@backend/app/api/endpoints/admin/public_shells.py`:
- Around line 154-179: The uniqueness check currently only runs when
shell_data.name changes; update it to run whenever the resulting (name,
namespace) pair will change: compute new_name = shell_data.name or shell.name
and new_namespace = shell_data.namespace or shell.namespace, then query Kind for
user_id==0, kind=="Shell", name==new_name and namespace==new_namespace to detect
collisions (reuse the existing existing_shell logic) and raise the same
HTTPException if found; then apply updates to shell.name, shell.namespace, and
shell.json from shell_data as before.
In `@backend/app/api/endpoints/admin/public_teams.py`:
- Around line 218-235: The uniqueness check currently only runs when
team_data.name != team.name, so changing only team_data.namespace can bypass
validation; update the condition in the update handler to run the uniqueness
query when either team_data.name differs from team.name OR team_data.namespace
differs from team.namespace (use team_data.namespace or team.namespace to
compute namespace variable as before), then query Kind for Kind.user_id==0,
Kind.kind=="Team", Kind.name==resolved_name and
Kind.namespace==resolved_namespace and raise the same HTTPException if
existing_team is found; reference the existing symbols team_data, team, Kind,
existing_team, and HTTPException when making this change.
- Around line 115-139: The async endpoints lack the required OpenTelemetry
decorator; add the `@trace_async`() decorator immediately above each async
function definition for list_public_teams, create_public_team,
update_public_team, and delete_public_team, and ensure trace_async is imported
into the module so the decorator is available; place the decorator directly
above each async def line to enable tracing for those endpoints.
In `@backend/app/api/endpoints/admin/tasks.py`:
- Around line 71-84: The generate_admin_token endpoint creates a 500-year token
and accepts an unused db: Session parameter; update it to remove the unused db
parameter from the function signature (generate_admin_token) and its
Depends(get_db) injection, and replace the pseudo-permanent token strategy by
either (a) implementing a token revocation mechanism: persist issued tokens with
identifiers and a revoked flag (e.g., token blacklist table) and record the
created token in an audit log when create_access_token is called, or (b) change
to a shorter expires_delta and adopt a refresh-token flow; ensure created tokens
include an identifier you store so you can revoke via the new blacklist and
update any token validation logic to check that blacklist/audit before accepting
a token.
🧹 Nitpick comments (15)
backend/app/api/endpoints/admin/stats.py (1)
19-47: Missing OpenTelemetry tracing decorator.Per coding guidelines, async functions should use
@trace_async()decorator for observability. This endpoint performs multiple database queries and would benefit from tracing.♻️ Proposed fix
+from app.core.telemetry import trace_async + router = APIRouter() +@trace_async() `@router.get`("/stats", response_model=SystemStats) async def get_system_stats(backend/app/api/endpoints/admin/tasks.py (1)
23-44: Missing OpenTelemetry tracing decorator.Per coding guidelines, async functions should use
@trace_async()decorator. Task creation is a significant operation that should be traced for observability.♻️ Proposed fix
+from app.core.telemetry import trace_async + router = APIRouter() +@trace_async() `@router.post`( "/users/{user_id}/tasks",Apply similarly to other endpoints in this file.
backend/app/api/endpoints/admin/api_keys.py (2)
92-100: Minor: Comment inaccuracy about key length.
secrets.token_urlsafe(32)generates ~43 characters (base64url encoding of 32 bytes), not 32 characters. The key prefix shows first 8 characters which is correct.📝 Suggested comment fix
- # Generate key: wg-{32 random chars} + # Generate key: wg-{~43 random chars from 32 bytes} random_part = secrets.token_urlsafe(32)
33-70: Missing OpenTelemetry tracing decorators.Per coding guidelines, async functions should use
@trace_async()decorator for observability. API key management operations are security-sensitive and should be traced.♻️ Proposed fix pattern
+from app.core.telemetry import trace_async + router = APIRouter() # ==================== Service Key Management Endpoints ==================== +@trace_async() `@router.get`("/service-keys", response_model=ServiceKeyListResponse) async def list_service_keys(Apply
@trace_async()to all async endpoint functions in this file.backend/app/api/endpoints/admin/system_config.py (1)
75-96: Missing OpenTelemetry tracing decorators.Per coding guidelines, async functions should use
@trace_async()decorator. System configuration endpoints should be traced for observability and debugging.♻️ Proposed fix pattern
+from app.core.telemetry import trace_async + router = APIRouter() # Config keys QUICK_ACCESS_CONFIG_KEY = "quick_access_recommended" CHAT_SLOGAN_TIPS_CONFIG_KEY = "chat_slogan_tips" # ... default config ... +@trace_async() `@router.get`("/system-config/quick-access", response_model=SystemConfigResponse) async def get_quick_access_config(Apply
@trace_async()to all async endpoint functions in this file.backend/app/api/endpoints/admin/kind_management.py (3)
165-198: Use explicitOptionaltype annotation.The
resource: Dict[str, Any] = Noneparameter uses implicit Optional which is deprecated per PEP 484. This applies to multiple endpoints in this file.♻️ Proposed fix
+from typing import Any, Dict, List, Optional + `@router.post`("/users/{user_id}/kinds/{kinds}", status_code=status.HTTP_201_CREATED) async def admin_create_resource_for_user( user_id: int = Path(..., description="User ID"), kinds: str = Path( ..., description="Resource type. Valid options: ghosts, models, shells, bots, teams, workspaces, tasks", ), namespace: str = Query("default", description="Resource namespace"), - resource: Dict[str, Any] = None, + resource: Optional[Dict[str, Any]] = None,Apply the same fix to lines 209, 278, and 324.
301-318: Preserve exception context withraise ... from e.The broad exception catch loses the original stack trace. Use
raise ... from eto preserve the exception chain for better debugging.♻️ Proposed fix
try: # Execute batch operation results = batch_service.apply_resources(user_id, resources) success_count = sum(1 for r in results if r["success"]) total_count = len(results) return BatchResponse( success=success_count == total_count, message=f"Applied {success_count}/{total_count} resources for user {user_id}", results=results, ) except Exception as e: raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Error batch applying resources for user {user_id}: {str(e)}", - ) + detail=f"Error batch applying resources for user {user_id}: {e!s}", + ) from e
35-62: Missing OpenTelemetry tracing decorators.Per coding guidelines, async functions should use
@trace_async()decorator. Admin resource management operations should be traced for observability.♻️ Proposed fix pattern
+from app.core.telemetry import trace_async + router = APIRouter() # ==================== Admin Kind Management Endpoints ==================== +@trace_async() `@router.get`("/users/{user_id}/kinds/{kinds}") async def admin_list_user_resources(Apply
@trace_async()to all async endpoint functions in this file.backend/app/api/endpoints/admin/users.py (2)
136-143: Consider using FastAPI's BackgroundTasks instead of raw threading.Using
threading.Threadwithasyncio.run()inside is an anti-pattern in async FastAPI applications. The daemon thread may be terminated before completing if the process shuts down.FastAPI provides
BackgroundTasksfor this use case, which integrates better with the ASGI lifecycle.♻️ Proposed fix using BackgroundTasks
+from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Path, Query, status -import asyncio -import threading `@router.post`( "/users", response_model=AdminUserResponse, status_code=status.HTTP_201_CREATED ) async def create_user( user_data: AdminUserCreate, + background_tasks: BackgroundTasks, db: Session = Depends(get_db), current_user: User = Depends(get_admin_user), ): # ... user creation logic ... - # Apply default resources for the new user in a background thread - def run_async_task(): - asyncio.run(apply_default_resources_async(new_user.id)) - - thread = threading.Thread(target=run_async_task, daemon=True) - thread.start() + # Apply default resources for the new user in background + background_tasks.add_task(apply_default_resources_async, new_user.id) return _user_to_response(new_user)
45-74: Missing OpenTelemetry tracing decorators.Per coding guidelines, async functions should use
@trace_async()decorator. User management operations are sensitive and should be traced for observability and audit purposes.♻️ Proposed fix pattern
+from app.core.telemetry import trace_async + router = APIRouter() +@trace_async() `@router.get`("/users", response_model=AdminUserListResponse) async def list_all_users(Apply
@trace_async()to all async endpoint functions in this file.backend/app/api/endpoints/admin/public_retrievers.py (1)
39-115: Good separation of concerns using service layer.This file correctly delegates business logic to
public_retriever_service, which is a cleaner pattern compared to inline ORM queries. Consider applying this pattern to the other public entity endpoints for consistency.The same recommendation for adding
@trace_async()decorators applies here per coding guidelines.backend/app/api/endpoints/admin/public_ghosts.py (1)
210-218: Inefficient query: fetches all public bots to check references.Loading all public bots into memory to check ghost references is inefficient for large datasets. Consider using a more targeted query.
♻️ Proposed optimization using JSON containment query
- # Check if any public bots (active or inactive) reference this ghost - bots_using_ghost = ( - db.query(Kind) - .filter( - Kind.user_id == 0, - Kind.kind == "Bot", - ) - .all() - ) - - referencing_bots = [] - for bot in bots_using_ghost: - # Defensive validation: ensure bot.json is a dict - if not isinstance(bot.json, dict): - continue - spec = bot.json.get("spec", {}) - # Defensive validation: ensure spec is a dict - if not isinstance(spec, dict): - continue - ghost_ref = spec.get("ghostRef") - # Defensive validation: ensure ghostRef is a dict - if not isinstance(ghost_ref, dict): - continue - if ( - ghost_ref.get("name") == ghost.name - and ghost_ref.get("namespace", "default") == ghost.namespace - ): - referencing_bots.append(bot.name) + # Check if any public bots reference this ghost using JSON query + # Note: Implementation depends on your database's JSON query capabilities + from sqlalchemy import cast, String + from sqlalchemy.dialects.postgresql import JSONB + + referencing_bot = ( + db.query(Kind.name) + .filter( + Kind.user_id == 0, + Kind.kind == "Bot", + Kind.json["spec"]["ghostRef"]["name"].astext == ghost.name, + Kind.json["spec"]["ghostRef"]["namespace"].astext == ghost.namespace, + ) + .first() + ) + + if referencing_bot: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Cannot delete ghost '{ghost.name}' because it is referenced by public bot: {referencing_bot.name}", + )If PostgreSQL JSON operators are not available, consider at minimum using
.limit(1)to stop at first match rather than loading all bots.backend/app/api/endpoints/admin/public_bots.py (2)
175-392: Add OpenTelemetry tracing decorators to admin endpoints.Line 175 and the other async endpoints should be decorated with
@trace_async()per the backend tracing guideline. Please apply the same pattern to list/create/update/delete handlers in this module. As per coding guidelines, ...
87-145: Use SQLAlchemy boolean semantics instead of== True.Lines 94, 119, and 144 compare
Kind.is_active == True. PreferKind.is_active.is_(True)(or column truthiness) to satisfy E712 and avoid edge cases.♻️ Suggested change (apply to all three filters)
- Kind.is_active == True, + Kind.is_active.is_(True),backend/app/api/endpoints/admin/public_shells.py (1)
61-194: Add OpenTelemetry tracing decorators to admin endpoints.Line 61 and the other async endpoints should use
@trace_async()per the backend tracing guideline. Please apply the same pattern to list/create/update/delete handlers in this module. As per coding guidelines, ...
Comment on lines
+278
to
+312
| # Check name uniqueness if being changed | ||
| if bot_data.name and bot_data.name != bot.name: | ||
| namespace = bot_data.namespace or bot.namespace | ||
| existing_bot = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Bot", | ||
| Kind.name == bot_data.name, | ||
| Kind.namespace == namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_bot: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public bot '{bot_data.name}' already exists in namespace '{namespace}'", | ||
| ) | ||
|
|
||
| # Validate resource references if json is being updated | ||
| if bot_data.bot_json: | ||
| is_valid, error_message = _validate_bot_resource_references( | ||
| db, bot_data.bot_json | ||
| ) | ||
| if not is_valid: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=error_message, | ||
| ) | ||
|
|
||
| # Update fields | ||
| if bot_data.name is not None: | ||
| bot.name = bot_data.name | ||
| if bot_data.namespace is not None: | ||
| bot.namespace = bot_data.namespace |
Contributor
There was a problem hiding this comment.
Handle namespace-only updates in uniqueness check.
Line 278 only checks when the name changes. If only the namespace changes, you can accidentally create a duplicate (same name in the target namespace) or hit a DB uniqueness constraint. Consider validating uniqueness whenever either name or namespace changes.
🛠️ Proposed fix
- # Check name uniqueness if being changed
- if bot_data.name and bot_data.name != bot.name:
- namespace = bot_data.namespace or bot.namespace
+ # Check name/namespace uniqueness if either changes
+ if (
+ (bot_data.name is not None and bot_data.name != bot.name)
+ or (bot_data.namespace is not None and bot_data.namespace != bot.namespace)
+ ):
+ namespace = bot_data.namespace or bot.namespace
+ name = bot_data.name or bot.name
existing_bot = (
db.query(Kind)
.filter(
Kind.user_id == 0,
Kind.kind == "Bot",
- Kind.name == bot_data.name,
+ Kind.name == name,
Kind.namespace == namespace,
)
.first()
)🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/public_bots.py` around lines 278 - 312, The
uniqueness check currently only runs when bot_data.name changes; update it to
run whenever either bot_data.name or bot_data.namespace is being changed to
avoid creating duplicates when only namespace is updated. Specifically, in the
public bot update handler in public_bots.py, replace the "if bot_data.name and
bot_data.name != bot.name:" guard with a check that detects changes to name or
namespace (e.g., if (bot_data.name is not None and bot_data.name != bot.name) or
(bot_data.namespace is not None and bot_data.namespace != bot.namespace)),
compute the target name as bot_data.name or bot.name and target namespace as
bot_data.namespace or bot.namespace, then run the existing Kind query
(Kind.user_id == 0, Kind.kind == "Bot", Kind.name == target_name, Kind.namespace
== target_namespace) and raise the same HTTPException if existing_bot is found;
keep the subsequent resource validation (_validate_bot_resource_references) and
field assignments unchanged.
Comment on lines
+154
to
+171
| # Check name uniqueness if being changed | ||
| if ghost_data.name and ghost_data.name != ghost.name: | ||
| namespace = ghost_data.namespace or ghost.namespace | ||
| existing_ghost = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Ghost", | ||
| Kind.name == ghost_data.name, | ||
| Kind.namespace == namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_ghost: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public ghost '{ghost_data.name}' already exists in namespace '{namespace}'", | ||
| ) |
Contributor
There was a problem hiding this comment.
Same namespace-only change issue as in public_teams.py.
The uniqueness check only triggers when the name changes, not when only the namespace changes. Apply the same fix as suggested for public_teams.py.
🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/public_ghosts.py` around lines 154 - 171, The
uniqueness check currently runs only when ghost_data.name changes (in the block
referencing ghost_data, ghost, and Kind), so a user can change only the
namespace and bypass the check; update the conditional to run the uniqueness
lookup when either the name or the namespace is being changed (i.e., if
ghost_data.name and ghost_data.name != ghost.name OR ghost_data.namespace and
ghost_data.namespace != ghost.namespace), then compute the effective name and
namespace (name = ghost_data.name or ghost.name, namespace =
ghost_data.namespace or ghost.namespace) and perform the existing
db.query(Kind)...first() check and raise the same HTTPException if an
existing_ghost is found.
Comment on lines
+62
to
+74
| query = db.query(Kind).filter( | ||
| Kind.user_id == 0, Kind.kind == "Model", Kind.namespace == "default" | ||
| ) | ||
| total = query.count() | ||
| models = query.all() | ||
|
|
||
| # Sort models by displayName (case-insensitive) | ||
| sorted_models = sorted(models, key=lambda m: _get_display_name(m).lower()) | ||
|
|
||
| # Apply pagination after sorting | ||
| start_idx = (page - 1) * limit | ||
| end_idx = start_idx + limit | ||
| paginated_models = sorted_models[start_idx:end_idx] |
Contributor
There was a problem hiding this comment.
Performance issue: in-memory sorting defeats pagination.
Loading all models into memory (query.all()) to sort by displayName before paginating negates the performance benefits of pagination. For large datasets, this will cause memory issues and slow responses.
🐛 Proposed fix using SQL-level sorting with COALESCE
- query = db.query(Kind).filter(
- Kind.user_id == 0, Kind.kind == "Model", Kind.namespace == "default"
- )
- total = query.count()
- models = query.all()
-
- # Sort models by displayName (case-insensitive)
- sorted_models = sorted(models, key=lambda m: _get_display_name(m).lower())
-
- # Apply pagination after sorting
- start_idx = (page - 1) * limit
- end_idx = start_idx + limit
- paginated_models = sorted_models[start_idx:end_idx]
+ from sqlalchemy import func
+
+ query = db.query(Kind).filter(
+ Kind.user_id == 0, Kind.kind == "Model", Kind.namespace == "default"
+ )
+ total = query.count()
+
+ # Sort by displayName at SQL level using JSON extraction with fallback to name
+ # Note: Syntax may vary by database (PostgreSQL shown)
+ paginated_models = (
+ query.order_by(
+ func.lower(
+ func.coalesce(
+ Kind.json["metadata"]["displayName"].astext,
+ Kind.name
+ )
+ )
+ )
+ .offset((page - 1) * limit)
+ .limit(limit)
+ .all()
+ )If SQL-level JSON sorting is not feasible, document this as a known limitation and consider adding a warning for large model counts.
🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/public_models.py` around lines 62 - 74, The
current code pulls all rows with query.all() and does Python-level sorting via
_get_display_name then slices paginated_models, which defeats DB pagination;
change the logic to perform sorting in SQL and apply offset/limit on the query
itself: modify the query for Kind (the variable query) to include an order_by
that extracts the JSON displayName (using SQL COALESCE to fall back to another
field like name or Kind.kind if displayName is null) so the DB returns rows
already ordered, then call query.count() if needed and use
query.offset((page-1)*limit).limit(limit).all() instead of
sorted_models/paginated_models; if your ORM cannot express JSON
extraction/COALESCE for displayName, add a clear comment/documentation near
_get_display_name and the pagination branch describing this known limitation and
warn about performance for large datasets.
Comment on lines
+62
to
+64
| query = db.query(Kind).filter( | ||
| Kind.user_id == 0, Kind.kind == "Model", Kind.namespace == "default" | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the actual code around lines 62-64 in public_models.py
cd backend && sed -n '55,75p' app/api/endpoints/admin/public_models.pyRepository: wecode-ai/Wegent
Length of output: 741
🏁 Script executed:
# Check all public_*.py files for namespace filtering patterns
rg -n "namespace" app/api/endpoints/admin/public_*.py -B2 -A2 | head -100Repository: wecode-ai/Wegent
Length of output: 135
🏁 Script executed:
# Check if there are multiple namespaces being used in the database/model design
rg -n "namespace.*=" backend/app/models/ --type py -B1 -A1 | head -80Repository: wecode-ai/Wegent
Length of output: 801
🏁 Script executed:
# Find all public endpoint files
find backend/app/api/endpoints -name "*public*" -type fRepository: wecode-ai/Wegent
Length of output: 354
🏁 Script executed:
# Search for namespace filtering in the entire endpoints/admin directory
rg -n "namespace" backend/app/api/endpoints/admin/ --type py -B2 -A2Repository: wecode-ai/Wegent
Length of output: 38562
🏁 Script executed:
# Check if there are any other references to "default" namespace in model-related code
rg -n 'namespace.*==.*"default"' backend/app/api/endpoints/ --type py -B2 -A2Repository: wecode-ai/Wegent
Length of output: 7323
🏁 Script executed:
# Check list_public_models function signature
sed -n '50,75p' backend/app/api/endpoints/admin/public_models.pyRepository: wecode-ai/Wegent
Length of output: 875
🏁 Script executed:
# Check if other public endpoints have namespace parameter in their list functions
grep -A 10 "def list_public" backend/app/api/endpoints/admin/public_*.py | head -80Repository: wecode-ai/Wegent
Length of output: 5505
🏁 Script executed:
# Check the full list_public_models function
sed -n '50,85p' backend/app/api/endpoints/admin/public_models.pyRepository: wecode-ai/Wegent
Length of output: 1130
🏁 Script executed:
# Check the create_public_model function more completely
sed -n '88,125p' backend/app/api/endpoints/admin/public_models.pyRepository: wecode-ai/Wegent
Length of output: 1092
🏁 Script executed:
# Check the PublicModelCreateRequest schema to see if namespace has constraints
rg -A 10 "class PublicModelCreateRequest" backend/app/schemas/ --type pyRepository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Find the schema file that defines PublicModelCreate
fd "\.py$" backend/app/schemas | xargs grep -l "PublicModelCreate"Repository: wecode-ai/Wegent
Length of output: 88
🏁 Script executed:
# Also check if there's any validation or default for namespace in the model creation
rg "PublicModelCreate|class.*Model.*Create" backend/app/schemas -A 15 --type pyRepository: wecode-ai/Wegent
Length of output: 2709
list_public_models endpoint hardcodes namespace="default" filter, inconsistent with other public entity list endpoints.
The list endpoint only returns public models in the "default" namespace, but the create endpoint accepts models in any namespace. Other public entity endpoints (list_public_bots, list_public_shells, list_public_ghosts, list_public_teams) do not filter by namespace and return all public resources regardless of namespace. This inconsistency means public models created in non-default namespaces are not visible through the list endpoint. Either remove the namespace filter to match other endpoints or explicitly restrict model creation to the "default" namespace.
🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/public_models.py` around lines 62 - 64, The
list_public_models endpoint currently filters public models to namespace
"default"; update the query in list_public_models to remove the namespace filter
so it returns all public Model kinds across namespaces (i.e., change the
db.query(Kind).filter(...) used in list_public_models to only filter
Kind.user_id == 0 and Kind.kind == "Model"), and ensure no other logic in
list_public_models relies on namespace; alternatively if you prefer to restrict
creation, enforce namespace=="default" validation in the model creation
handler—pick one approach and make tests reflect the chosen behavior.
Comment on lines
+154
to
+179
| # Check name uniqueness if being changed | ||
| if shell_data.name and shell_data.name != shell.name: | ||
| namespace = shell_data.namespace or shell.namespace | ||
| existing_shell = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Shell", | ||
| Kind.name == shell_data.name, | ||
| Kind.namespace == namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_shell: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public shell '{shell_data.name}' already exists in namespace '{namespace}'", | ||
| ) | ||
|
|
||
| # Update fields | ||
| if shell_data.name is not None: | ||
| shell.name = shell_data.name | ||
| if shell_data.namespace is not None: | ||
| shell.namespace = shell_data.namespace | ||
| if shell_data.shell_json is not None: | ||
| shell.json = shell_data.shell_json |
Contributor
There was a problem hiding this comment.
Check uniqueness when namespace changes.
Line 155 only checks when the name changes. If the namespace changes alone, the update can collide with an existing shell in the target namespace. Please validate uniqueness when either name or namespace changes.
🛠️ Proposed fix
- # Check name uniqueness if being changed
- if shell_data.name and shell_data.name != shell.name:
- namespace = shell_data.namespace or shell.namespace
+ # Check name/namespace uniqueness if either changes
+ if (
+ (shell_data.name is not None and shell_data.name != shell.name)
+ or (shell_data.namespace is not None and shell_data.namespace != shell.namespace)
+ ):
+ namespace = shell_data.namespace or shell.namespace
+ name = shell_data.name or shell.name
existing_shell = (
db.query(Kind)
.filter(
Kind.user_id == 0,
Kind.kind == "Shell",
- Kind.name == shell_data.name,
+ Kind.name == name,
Kind.namespace == namespace,
)
.first()
)🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/public_shells.py` around lines 154 - 179, The
uniqueness check currently only runs when shell_data.name changes; update it to
run whenever the resulting (name, namespace) pair will change: compute new_name
= shell_data.name or shell.name and new_namespace = shell_data.namespace or
shell.namespace, then query Kind for user_id==0, kind=="Shell", name==new_name
and namespace==new_namespace to detect collisions (reuse the existing
existing_shell logic) and raise the same HTTPException if found; then apply
updates to shell.name, shell.namespace, and shell.json from shell_data as
before.
Comment on lines
+218
to
+235
| # Check name uniqueness if being changed | ||
| if team_data.name and team_data.name != team.name: | ||
| namespace = team_data.namespace or team.namespace | ||
| existing_team = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == 0, | ||
| Kind.kind == "Team", | ||
| Kind.name == team_data.name, | ||
| Kind.namespace == namespace, | ||
| ) | ||
| .first() | ||
| ) | ||
| if existing_team: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=f"Public team '{team_data.name}' already exists in namespace '{namespace}'", | ||
| ) |
Contributor
There was a problem hiding this comment.
Namespace-only change may bypass uniqueness validation.
The uniqueness check only triggers when team_data.name differs from team.name. If only the namespace is changed, no uniqueness validation occurs, potentially allowing a duplicate (name, namespace) combination.
🐛 Proposed fix
- # Check name uniqueness if being changed
- if team_data.name and team_data.name != team.name:
- namespace = team_data.namespace or team.namespace
+ # Check name/namespace uniqueness if either is being changed
+ new_name = team_data.name if team_data.name is not None else team.name
+ new_namespace = team_data.namespace if team_data.namespace is not None else team.namespace
+ if new_name != team.name or new_namespace != team.namespace:
existing_team = (
db.query(Kind)
.filter(
Kind.user_id == 0,
Kind.kind == "Team",
- Kind.name == team_data.name,
- Kind.namespace == namespace,
+ Kind.name == new_name,
+ Kind.namespace == new_namespace,
+ Kind.id != team_id, # Exclude current team
)
.first()
)
if existing_team:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
- detail=f"Public team '{team_data.name}' already exists in namespace '{namespace}'",
+ detail=f"Public team '{new_name}' already exists in namespace '{new_namespace}'",
)🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/public_teams.py` around lines 218 - 235, The
uniqueness check currently only runs when team_data.name != team.name, so
changing only team_data.namespace can bypass validation; update the condition in
the update handler to run the uniqueness query when either team_data.name
differs from team.name OR team_data.namespace differs from team.namespace (use
team_data.namespace or team.namespace to compute namespace variable as before),
then query Kind for Kind.user_id==0, Kind.kind=="Team", Kind.name==resolved_name
and Kind.namespace==resolved_namespace and raise the same HTTPException if
existing_team is found; reference the existing symbols team_data, team, Kind,
existing_team, and HTTPException when making this change.
Comment on lines
+71
to
+84
| @router.post("/generate-admin-token", response_model=Token) | ||
| async def generate_admin_token( | ||
| db: Session = Depends(get_db), current_user: User = Depends(get_admin_user) | ||
| ): | ||
| """ | ||
| Generate a permanent admin token (pseudo-permanent for 500 years) | ||
| """ | ||
| # Create a permanent token (set very long expiration time) | ||
| access_token = create_access_token( | ||
| data={"sub": current_user.user_name, "user_id": current_user.id}, | ||
| expires_delta=262800000, # 500 years | ||
| ) | ||
|
|
||
| return Token(access_token=access_token, token_type="bearer") |
Contributor
There was a problem hiding this comment.
Security concern: Pseudo-permanent tokens lack revocation mechanism.
Generating 500-year tokens is effectively creating non-expiring credentials. If such a token is compromised, there's no way to revoke it without changing the application's secret key (which would invalidate all tokens).
Consider:
- Adding a token revocation mechanism (e.g., token blacklist table)
- Recording token creation in an audit log for traceability
- Using shorter expiration with refresh token pattern
Also, the db parameter is unused and can be removed.
🔒 Minimal fix to remove unused parameter
`@router.post`("/generate-admin-token", response_model=Token)
async def generate_admin_token(
- db: Session = Depends(get_db), current_user: User = Depends(get_admin_user)
+ current_user: User = Depends(get_admin_user),
):🧰 Tools
🪛 Ruff (0.14.13)
73-73: Unused function argument: db
(ARG001)
73-73: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
73-73: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
84-84: Possible hardcoded password assigned to argument: "token_type"
(S106)
🤖 Prompt for AI Agents
In `@backend/app/api/endpoints/admin/tasks.py` around lines 71 - 84, The
generate_admin_token endpoint creates a 500-year token and accepts an unused db:
Session parameter; update it to remove the unused db parameter from the function
signature (generate_admin_token) and its Depends(get_db) injection, and replace
the pseudo-permanent token strategy by either (a) implementing a token
revocation mechanism: persist issued tokens with identifiers and a revoked flag
(e.g., token blacklist table) and record the created token in an audit log when
create_access_token is called, or (b) change to a shorter expires_delta and
adopt a refresh-token flow; ensure created tokens include an identifier you
store so you can revoke via the new blacklist and update any token validation
logic to check that blacklist/audit before accepting a token.
This was referenced Jan 19, 2026
maquan0927
pushed a commit
to maquan0927/Wegent
that referenced
this pull request
Jan 22, 2026
* feat(admin): add public team and bot management Add support for system-level public agents (teams) and bots that are visible to all users. Public resources use user_id=0 as identifier. Backend changes: - Add Bot to PUBLIC_FALLBACK_KINDS for public bot lookup support - Add public team fallback in _get_team() after shared teams check - Include public teams in get_user_teams() query results via UNION - Add Pydantic schemas for public team/bot CRUD operations - Add admin API endpoints for managing public teams and bots - Add validation to ensure public teams only reference public bots - Add validation to ensure public bots only reference public resources Frontend changes: - Add TypeScript types and API methods for public teams/bots - Add public-teams and public-bots tabs to admin navigation - Create PublicTeamList component for admin interface - Create PublicBotList component for admin interface - Show "Public" badge for public teams in user's team list - Hide edit/delete buttons for public teams from regular users - Add i18n translations for both English and Chinese * fix(admin): use GlobeAltIcon from heroicons instead of lucide-react Import GlobeAltIcon from @heroicons/react/24/outline to match the existing icon pattern used in other admin components. * fix(admin): address CodeRabbit review suggestions - Change list_public_teams and list_public_bots to use SQL-level pagination instead of in-memory pagination for better performance - Add defensive type checking in _validate_team_bot_references and _validate_bot_resource_references to handle malformed JSON inputs - Fix delete_public_bot to check both active and inactive teams when preventing bot deletion due to team references - Update frontend components to use single 'admin' namespace in useTranslation hook following project i18n conventions * fix(frontend): add input validation in update handlers Add trim and empty validation for name in handleUpdateBot and handleUpdateTeam to prevent whitespace-only names from being submitted to the backend. * feat: support public ghost page * feat: support order for public teams * refactor: split admin.py * fix: fix review issues --------- Co-authored-by: qdaxb <4157870+qdaxb@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
user_id=0as identifier (following existing pattern)Changes
Backend
BottoPUBLIC_FALLBACK_KINDSand public team fallback in_get_team()get_user_teams()query via UNIONGET/POST /admin/public-teams- List and create public teamsPUT/DELETE /admin/public-teams/{team_id}- Update and delete public teamsGET/POST /admin/public-bots- List and create public botsPUT/DELETE /admin/public-bots/{bot_id}- Update and delete public botsFrontend
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
i18n
✏️ Tip: You can customize this high-level summary in your review settings.