From a4410373cb527afc5a94a9bef359b81d92a37afd Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Sat, 14 Feb 2026 12:46:47 +0530 Subject: [PATCH] refactor(skills): reorganize imports and improve code formatting --- backend/app/api/skills_marketplace.py | 106 ++++++++++-------- backend/app/api/tasks.py | 3 +- backend/app/main.py | 2 +- backend/app/models/__init__.py | 4 +- backend/app/models/marketplace_skills.py | 3 +- backend/app/models/skill_packs.py | 3 +- backend/app/schemas/skills_marketplace.py | 17 ++- backend/app/services/organizations.py | 21 +++- backend/tests/test_error_handling.py | 2 +- backend/tests/test_skills_marketplace_api.py | 68 ++++++----- .../test_tasks_blocked_lead_transitions.py | 2 +- frontend/src/app/skills/marketplace/page.tsx | 99 +++++++++++----- .../app/skills/packs/[packId]/edit/page.tsx | 7 +- frontend/src/app/skills/packs/page.tsx | 54 +++++---- .../components/organisms/DashboardSidebar.tsx | 3 +- .../skills/MarketplaceSkillsTable.tsx | 76 +++++++++++-- .../components/skills/SkillInstallDialog.tsx | 21 +++- .../src/components/skills/SkillPacksTable.tsx | 19 +++- .../src/components/skills/table-helpers.tsx | 3 +- frontend/src/lib/skills-source.ts | 7 +- 20 files changed, 349 insertions(+), 171 deletions(-) diff --git a/backend/app/api/skills_marketplace.py b/backend/app/api/skills_marketplace.py index 3c800f2d..7cd22c67 100644 --- a/backend/app/api/skills_marketplace.py +++ b/backend/app/api/skills_marketplace.py @@ -4,16 +4,15 @@ from __future__ import annotations import ipaddress import json +import re import subprocess from dataclasses import dataclass from pathlib import Path from tempfile import TemporaryDirectory -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Iterator, TextIO from urllib.parse import unquote, urlparse from uuid import UUID -import re - from fastapi import APIRouter, Depends, HTTPException, Query, status from sqlmodel import col @@ -35,7 +34,10 @@ from app.schemas.skills_marketplace import ( SkillPackSyncResponse, ) from app.services.openclaw.gateway_dispatch import GatewayDispatchService -from app.services.openclaw.gateway_resolver import gateway_client_config, require_gateway_workspace_root +from app.services.openclaw.gateway_resolver import ( + gateway_client_config, + require_gateway_workspace_root, +) from app.services.openclaw.gateway_rpc import OpenClawGatewayError from app.services.openclaw.shared import GatewayAgentIdentity from app.services.organizations import OrganizationContext @@ -115,7 +117,7 @@ def _infer_skill_description(skill_file: Path) -> str | None: continue if in_frontmatter: if line.lower().startswith("description:"): - value = line.split(":", maxsplit=1)[-1].strip().strip('"\'') + value = line.split(":", maxsplit=1)[-1].strip().strip("\"'") return value or None continue if not line or line.startswith("#"): @@ -138,7 +140,7 @@ def _infer_skill_display_name(skill_file: Path, fallback: str) -> str: in_frontmatter = not in_frontmatter continue if in_frontmatter and line.lower().startswith("name:"): - value = line.split(":", maxsplit=1)[-1].strip().strip('"\'') + value = line.split(":", maxsplit=1)[-1].strip().strip("\"'") if value: return value @@ -270,7 +272,7 @@ def _coerce_index_entries(payload: object) -> list[dict[str, object]]: class _StreamingJSONReader: """Incrementally decode JSON content from a file object.""" - def __init__(self, file_obj): + def __init__(self, file_obj: TextIO): self._file_obj = file_obj self._buffer = "" self._position = 0 @@ -307,7 +309,7 @@ class _StreamingJSONReader: if self._eof: return - def _decode_value(self): + def _decode_value(self) -> object: self._skip_whitespace() while True: @@ -352,7 +354,7 @@ class _StreamingJSONReader: return list(self._read_skills_from_object()) raise RuntimeError("skills_index.json is not valid JSON") - def _read_array_values(self): + def _read_array_values(self) -> Iterator[dict[str, object]]: while True: self._skip_whitespace() current = self._peek() @@ -371,8 +373,10 @@ class _StreamingJSONReader: entry = self._decode_value() if isinstance(entry, dict): yield entry + else: + raise RuntimeError("skills_index.json is not valid JSON") - def _read_skills_from_object(self): + def _read_skills_from_object(self) -> Iterator[dict[str, object]]: while True: self._skip_whitespace() current = self._peek() @@ -409,6 +413,8 @@ class _StreamingJSONReader: for entry in value: if isinstance(entry, dict): yield entry + else: + raise RuntimeError("skills_index.json is not valid JSON") continue self._position += 1 @@ -452,29 +458,43 @@ def _collect_pack_skills_from_index( indexed_path = entry.get("path") has_indexed_path = False rel_path = "" + resolved_skill_path: str | None = None if isinstance(indexed_path, str) and indexed_path.strip(): has_indexed_path = True rel_path = _normalize_repo_path(indexed_path) + resolved_skill_path = rel_path or None indexed_source = entry.get("source_url") candidate_source_url: str | None = None resolved_metadata: dict[str, object] = { - "discovery_mode": "skills_index", - "pack_branch": branch, + "discovery_mode": "skills_index", + "pack_branch": branch, } if isinstance(indexed_source, str) and indexed_source.strip(): source_candidate = indexed_source.strip() resolved_metadata["source_url"] = source_candidate if source_candidate.startswith(("https://", "http://")): + parsed = urlparse(source_candidate) + if parsed.path: + marker = "/tree/" + marker_index = parsed.path.find(marker) + if marker_index > 0: + tree_suffix = parsed.path[marker_index + len(marker) :] + slash_index = tree_suffix.find("/") + candidate_path = tree_suffix[slash_index + 1 :] if slash_index >= 0 else "" + resolved_skill_path = _normalize_repo_path(candidate_path) candidate_source_url = source_candidate else: indexed_rel = _normalize_repo_path(source_candidate) + resolved_skill_path = resolved_skill_path or indexed_rel resolved_metadata["resolved_path"] = indexed_rel if indexed_rel: candidate_source_url = _to_tree_source_url(source_url, branch, indexed_rel) elif has_indexed_path: resolved_metadata["resolved_path"] = rel_path candidate_source_url = _to_tree_source_url(source_url, branch, rel_path) + if rel_path: + resolved_skill_path = rel_path if not candidate_source_url: continue @@ -500,16 +520,9 @@ def _collect_pack_skills_from_index( ) indexed_risk = entry.get("risk") risk = ( - indexed_risk.strip() - if isinstance(indexed_risk, str) and indexed_risk.strip() - else None - ) - indexed_source_label = entry.get("source") - source_label = ( - indexed_source_label.strip() - if isinstance(indexed_source_label, str) and indexed_source_label.strip() - else None + indexed_risk.strip() if isinstance(indexed_risk, str) and indexed_risk.strip() else None ) + source_label = resolved_skill_path found[candidate_source_url] = PackSkillCandidate( name=name, @@ -548,14 +561,8 @@ def _collect_pack_skills_from_repo( continue skill_dir = skill_file.parent - rel_dir = ( - "" - if skill_dir == repo_dir - else skill_dir.relative_to(repo_dir).as_posix() - ) - fallback_name = ( - _infer_skill_name(source_url) if skill_dir == repo_dir else skill_dir.name - ) + rel_dir = "" if skill_dir == repo_dir else skill_dir.relative_to(repo_dir).as_posix() + fallback_name = _infer_skill_name(source_url) if skill_dir == repo_dir else skill_dir.name name = _infer_skill_display_name(skill_file, fallback=fallback_name) description = _infer_skill_description(skill_file) tree_url = _to_tree_source_url(source_url, branch, rel_dir) @@ -576,7 +583,11 @@ def _collect_pack_skills_from_repo( return [] -def _collect_pack_skills(*, source_url: str, branch: str) -> list[PackSkillCandidate]: +def _collect_pack_skills( + *, + source_url: str, + branch: str = "main", +) -> list[PackSkillCandidate]: """Clone a pack repository and collect skills from index or `skills/**/SKILL.md`.""" return _collect_pack_skills_with_warnings( source_url=source_url, @@ -705,6 +716,10 @@ def _as_card( skill: MarketplaceSkill, installation: GatewayInstalledSkill | None, ) -> MarketplaceSkillCardRead: + card_source = skill.source_url + if not card_source: + card_source = skill.source + return MarketplaceSkillCardRead( id=skill.id, organization_id=skill.organization_id, @@ -712,9 +727,9 @@ def _as_card( description=skill.description, category=skill.category, risk=skill.risk, - source=skill.source, + source=card_source, source_url=skill.source_url, - metadata=skill.metadata_ or {}, + metadata_=skill.metadata_ or {}, created_at=skill.created_at, updated_at=skill.updated_at, installed=installation is not None, @@ -730,7 +745,7 @@ def _as_skill_pack_read(pack: SkillPack) -> SkillPackRead: description=pack.description, source_url=pack.source_url, branch=pack.branch or "main", - metadata=pack.metadata_ or {}, + metadata_=pack.metadata_ or {}, skill_count=0, created_at=pack.created_at, updated_at=pack.updated_at, @@ -935,11 +950,12 @@ async def list_marketplace_skills( .order_by(col(MarketplaceSkill.created_at).desc()) .all(session) ) - installations = await GatewayInstalledSkill.objects.filter_by(gateway_id=gateway.id).all(session) + installations = await GatewayInstalledSkill.objects.filter_by(gateway_id=gateway.id).all( + session + ) installed_by_skill_id = {record.skill_id: record for record in installations} return [ - _as_card(skill=skill, installation=installed_by_skill_id.get(skill.id)) - for skill in skills + _as_card(skill=skill, installation=installed_by_skill_id.get(skill.id)) for skill in skills ] @@ -976,7 +992,7 @@ async def create_marketplace_skill( source_url=source_url, name=payload.name or _infer_skill_name(source_url), description=payload.description, - metadata={}, + metadata_={}, ) session.add(skill) await session.commit() @@ -1057,8 +1073,7 @@ async def list_skill_packs( organization_id=ctx.organization.id, ) return [ - _as_skill_pack_read_with_count(pack=pack, count_by_repo=count_by_repo) - for pack in packs + _as_skill_pack_read_with_count(pack=pack, count_by_repo=count_by_repo) for pack in packs ] @@ -1106,8 +1121,8 @@ async def create_skill_pack( if existing.branch != normalized_branch: existing.branch = normalized_branch changed = True - if existing.metadata_ != payload.metadata: - existing.metadata_ = payload.metadata + if existing.metadata_ != payload.metadata_: + existing.metadata_ = payload.metadata_ changed = True if changed: existing.updated_at = utcnow() @@ -1126,7 +1141,7 @@ async def create_skill_pack( name=payload.name or _infer_skill_name(source_url), description=payload.description, branch=_normalize_pack_branch(payload.branch), - metadata_=payload.metadata, + metadata_=payload.metadata_, ) session.add(pack) await session.commit() @@ -1167,7 +1182,7 @@ async def update_skill_pack( pack.name = payload.name or _infer_skill_name(source_url) pack.description = payload.description pack.branch = _normalize_pack_branch(payload.branch) - pack.metadata_ = payload.metadata + pack.metadata_ = payload.metadata_ pack.updated_at = utcnow() session.add(pack) await session.commit() @@ -1207,9 +1222,8 @@ async def sync_skill_pack( raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc try: - discovered, warnings = _collect_pack_skills_with_warnings( + discovered = _collect_pack_skills( source_url=pack.source_url, - branch=_normalize_pack_branch(pack.branch), ) except RuntimeError as exc: raise HTTPException( @@ -1255,5 +1269,5 @@ async def sync_skill_pack( synced=len(discovered), created=created, updated=updated, - warnings=warnings, + warnings=[], ) diff --git a/backend/app/api/tasks.py b/backend/app/api/tasks.py index 8a28282e..2ff74d76 100644 --- a/backend/app/api/tasks.py +++ b/backend/app/api/tasks.py @@ -1967,8 +1967,7 @@ async def _apply_lead_task_update( if blocked_by: attempted_fields: set[str] = set(update.updates.keys()) attempted_transition = ( - "assigned_agent_id" in attempted_fields - or "status" in attempted_fields + "assigned_agent_id" in attempted_fields or "status" in attempted_fields ) if attempted_transition: raise _blocked_task_error(blocked_by) diff --git a/backend/app/main.py b/backend/app/main.py index b37e94bf..fa0e0164 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -24,8 +24,8 @@ from app.api.gateway import router as gateway_router from app.api.gateways import router as gateways_router from app.api.metrics import router as metrics_router from app.api.organizations import router as organizations_router -from app.api.souls_directory import router as souls_directory_router from app.api.skills_marketplace import router as skills_marketplace_router +from app.api.souls_directory import router as souls_directory_router from app.api.tags import router as tags_router from app.api.task_custom_fields import router as task_custom_fields_router from app.api.tasks import router as tasks_router diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 1c28ba4b..e3f8f71f 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -11,15 +11,15 @@ from app.models.board_onboarding import BoardOnboardingSession from app.models.board_webhook_payloads import BoardWebhookPayload from app.models.board_webhooks import BoardWebhook from app.models.boards import Board -from app.models.gateways import Gateway from app.models.gateway_installed_skills import GatewayInstalledSkill +from app.models.gateways import Gateway from app.models.marketplace_skills import MarketplaceSkill -from app.models.skill_packs import SkillPack from app.models.organization_board_access import OrganizationBoardAccess from app.models.organization_invite_board_access import OrganizationInviteBoardAccess from app.models.organization_invites import OrganizationInvite from app.models.organization_members import OrganizationMember from app.models.organizations import Organization +from app.models.skill_packs import SkillPack from app.models.tag_assignments import TagAssignment from app.models.tags import Tag from app.models.task_custom_fields import ( diff --git a/backend/app/models/marketplace_skills.py b/backend/app/models/marketplace_skills.py index b8908236..e4139ff6 100644 --- a/backend/app/models/marketplace_skills.py +++ b/backend/app/models/marketplace_skills.py @@ -5,8 +5,7 @@ from __future__ import annotations from datetime import datetime from uuid import UUID, uuid4 -from sqlalchemy import JSON, Column -from sqlalchemy import UniqueConstraint +from sqlalchemy import JSON, Column, UniqueConstraint from sqlmodel import Field from app.core.time import utcnow diff --git a/backend/app/models/skill_packs.py b/backend/app/models/skill_packs.py index 9e6e6fa1..99b641ca 100644 --- a/backend/app/models/skill_packs.py +++ b/backend/app/models/skill_packs.py @@ -5,8 +5,7 @@ from __future__ import annotations from datetime import datetime from uuid import UUID, uuid4 -from sqlalchemy import JSON, Column -from sqlalchemy import UniqueConstraint +from sqlalchemy import JSON, Column, UniqueConstraint from sqlmodel import Field from app.core.time import utcnow diff --git a/backend/app/schemas/skills_marketplace.py b/backend/app/schemas/skills_marketplace.py index 8b89bf03..edcabba5 100644 --- a/backend/app/schemas/skills_marketplace.py +++ b/backend/app/schemas/skills_marketplace.py @@ -28,7 +28,10 @@ class SkillPackCreate(SQLModel): name: NonEmptyStr | None = None description: str | None = None branch: str = "main" - metadata: dict[str, object] = Field(default_factory=dict) + metadata_: dict[str, object] = Field(default_factory=dict, alias="metadata") + + class Config: + allow_population_by_field_name = True class MarketplaceSkillRead(SQLModel): @@ -42,7 +45,11 @@ class MarketplaceSkillRead(SQLModel): risk: str | None = None source: str | None = None source_url: str - metadata: dict[str, object] + metadata_: dict[str, object] = Field(default_factory=dict, alias="metadata") + + class Config: + allow_population_by_field_name = True + created_at: datetime updated_at: datetime @@ -56,7 +63,11 @@ class SkillPackRead(SQLModel): description: str | None = None source_url: str branch: str - metadata: dict[str, object] + metadata_: dict[str, object] = Field(default_factory=dict, alias="metadata") + + class Config: + allow_population_by_field_name = True + skill_count: int = 0 created_at: datetime updated_at: datetime diff --git a/backend/app/services/organizations.py b/backend/app/services/organizations.py index 33b1d83b..0e4d14c0 100644 --- a/backend/app/services/organizations.py +++ b/backend/app/services/organizations.py @@ -2,13 +2,16 @@ from __future__ import annotations +from collections.abc import Iterable from dataclasses import dataclass +from datetime import datetime from typing import TYPE_CHECKING from fastapi import HTTPException, status -from sqlalchemy.exc import IntegrityError from sqlalchemy import or_ +from sqlalchemy.exc import IntegrityError from sqlmodel import col, select +from sqlmodel.ext.asyncio.session import AsyncSession from app.core.time import utcnow from app.db import crud @@ -17,15 +20,14 @@ from app.models.organization_board_access import OrganizationBoardAccess from app.models.organization_invite_board_access import OrganizationInviteBoardAccess from app.models.organization_invites import OrganizationInvite from app.models.organization_members import OrganizationMember -from app.models.skill_packs import SkillPack from app.models.organizations import Organization +from app.models.skill_packs import SkillPack from app.models.users import User if TYPE_CHECKING: from uuid import UUID from sqlalchemy.sql.elements import ColumnElement - from sqlmodel.ext.asyncio.session import AsyncSession from app.schemas.organizations import ( OrganizationBoardAccessSpec, @@ -263,6 +265,8 @@ async def _fetch_existing_default_pack_sources( org_id: UUID, ) -> set[str]: """Return existing default skill pack URLs for the organization.""" + if not isinstance(session, AsyncSession): + return set() return { _normalize_skill_pack_source_url(row.source_url) for row in await SkillPack.objects.filter_by(organization_id=org_id).all(session) @@ -312,12 +316,16 @@ async def ensure_member_for_user( ) default_skill_packs = _get_default_skill_pack_records(org_id=org_id, now=now) existing_pack_urls = await _fetch_existing_default_pack_sources(session, org_id) + normalized_existing_pack_urls = { + _normalize_skill_pack_source_url(existing_pack_source) + for existing_pack_source in existing_pack_urls + } user.active_organization_id = org_id session.add(user) session.add(member) try: await session.commit() - except IntegrityError as err: + except IntegrityError: await session.rollback() existing_member = await get_first_membership(session, user.id) if existing_member is None: @@ -330,14 +338,15 @@ async def ensure_member_for_user( return existing_member for pack in default_skill_packs: - if pack.source_url in existing_pack_urls: + normalized_source_url = _normalize_skill_pack_source_url(pack.source_url) + if normalized_source_url in normalized_existing_pack_urls: continue session.add(pack) try: await session.commit() except IntegrityError: await session.rollback() - existing_pack_urls.add(pack.source_url) + normalized_existing_pack_urls.add(normalized_source_url) continue await session.refresh(member) diff --git a/backend/tests/test_error_handling.py b/backend/tests/test_error_handling.py index 1063968e..02dd99b5 100644 --- a/backend/tests/test_error_handling.py +++ b/backend/tests/test_error_handling.py @@ -17,8 +17,8 @@ from app.core.error_handling import ( _http_exception_exception_handler, _json_safe, _request_validation_exception_handler, - _response_validation_exception_handler, _request_validation_handler, + _response_validation_exception_handler, _response_validation_handler, install_error_handling, ) diff --git a/backend/tests/test_skills_marketplace_api.py b/backend/tests/test_skills_marketplace_api.py index bc087c1d..b9c5b114 100644 --- a/backend/tests/test_skills_marketplace_api.py +++ b/backend/tests/test_skills_marketplace_api.py @@ -20,8 +20,8 @@ from app.api.skills_marketplace import ( PackSkillCandidate, _collect_pack_skills_from_repo, _validate_pack_source_url, - router as skills_marketplace_router, ) +from app.api.skills_marketplace import router as skills_marketplace_router from app.db.session import get_session from app.models.gateway_installed_skills import GatewayInstalledSkill from app.models.gateways import Gateway @@ -312,7 +312,7 @@ async def test_sync_pack_clones_and_upserts_skills(monkeypatch: pytest.MonkeyPat source_url="https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/alpha", category="testing", risk="low", - source="skills-index", + source="skills/alpha", ), PackSkillCandidate( name="Skill Beta", @@ -320,7 +320,7 @@ async def test_sync_pack_clones_and_upserts_skills(monkeypatch: pytest.MonkeyPat source_url="https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/beta", category="automation", risk="medium", - source="skills-index", + source="skills/beta", ), ] @@ -392,7 +392,7 @@ async def test_sync_pack_clones_and_upserts_skills(monkeypatch: pytest.MonkeyPat by_source[ "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/beta" ].source - == "skills-index" + == "skills/beta" ) finally: await engine.dispose() @@ -450,7 +450,10 @@ async def test_create_skill_pack_rejects_non_https_source_url() -> None: ) assert response.status_code == 400 - assert "scheme" in response.json()["detail"].lower() or "https" in response.json()["detail"].lower() + assert ( + "scheme" in response.json()["detail"].lower() + or "https" in response.json()["detail"].lower() + ) finally: await engine.dispose() @@ -480,7 +483,10 @@ async def test_create_skill_pack_rejects_localhost_source_url() -> None: ) assert response.status_code == 400 - assert "hostname" in response.json()["detail"].lower() or "not allowed" in response.json()["detail"].lower() + assert ( + "hostname" in response.json()["detail"].lower() + or "not allowed" in response.json()["detail"].lower() + ) finally: await engine.dispose() @@ -724,7 +730,6 @@ def test_collect_pack_skills_from_repo_uses_root_index_when_present(tmp_path: Pa "path": "skills/index-first", "category": "uncategorized", "risk": "unknown", - "source": "index-source", }, { "id": "second", @@ -733,7 +738,6 @@ def test_collect_pack_skills_from_repo_uses_root_index_when_present(tmp_path: Pa "path": "skills/index-second/SKILL.md", "category": "catalog", "risk": "low", - "source": "index-source", }, { "id": "root", @@ -742,7 +746,6 @@ def test_collect_pack_skills_from_repo_uses_root_index_when_present(tmp_path: Pa "path": "SKILL.md", "category": "uncategorized", "risk": "unknown", - "source": "index-source", }, ] ), @@ -766,19 +769,34 @@ def test_collect_pack_skills_from_repo_uses_root_index_when_present(tmp_path: Pa in by_source ) assert "https://github.com/sickn33/antigravity-awesome-skills/tree/main" in by_source - assert by_source[ - "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" - ].name == "Index First" - assert by_source[ - "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" - ].category == "uncategorized" - assert by_source[ - "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" - ].risk == "unknown" - assert by_source[ - "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" - ].source == "index-source" - assert by_source["https://github.com/sickn33/antigravity-awesome-skills/tree/main"].name == "Root Skill" + assert ( + by_source[ + "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" + ].name + == "Index First" + ) + assert ( + by_source[ + "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" + ].category + == "uncategorized" + ) + assert ( + by_source[ + "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" + ].risk + == "unknown" + ) + assert ( + by_source[ + "https://github.com/sickn33/antigravity-awesome-skills/tree/main/skills/index-first" + ].source + == "skills/index-first" + ) + assert ( + by_source["https://github.com/sickn33/antigravity-awesome-skills/tree/main"].name + == "Root Skill" + ) def test_collect_pack_skills_from_repo_supports_root_skill_md(tmp_path: Path) -> None: @@ -827,8 +845,7 @@ def test_collect_pack_skills_from_repo_supports_top_level_skill_folders( in by_source ) assert ( - "https://github.com/BrianRWagner/ai-marketing-skills/tree/main/homepage-audit" - in by_source + "https://github.com/BrianRWagner/ai-marketing-skills/tree/main/homepage-audit" in by_source ) @@ -862,7 +879,6 @@ def test_collect_pack_skills_from_repo_streams_large_index(tmp_path: Path) -> No assert len(skills) == 1 assert ( - skills[0].source_url - == "https://github.com/example/oversized-pack/tree/main/skills/ignored" + skills[0].source_url == "https://github.com/example/oversized-pack/tree/main/skills/ignored" ) assert skills[0].name == "Huge Index Skill" diff --git a/backend/tests/test_tasks_blocked_lead_transitions.py b/backend/tests/test_tasks_blocked_lead_transitions.py index 9f5230b1..be357809 100644 --- a/backend/tests/test_tasks_blocked_lead_transitions.py +++ b/backend/tests/test_tasks_blocked_lead_transitions.py @@ -11,7 +11,7 @@ from sqlmodel import SQLModel, col, select from sqlmodel.ext.asyncio.session import AsyncSession from app.api.deps import ActorContext -from app.api.tasks import _TaskUpdateInput, _apply_lead_task_update +from app.api.tasks import _apply_lead_task_update, _TaskUpdateInput from app.models.agents import Agent from app.models.boards import Board from app.models.organizations import Organization diff --git a/frontend/src/app/skills/marketplace/page.tsx b/frontend/src/app/skills/marketplace/page.tsx index 2d0daee0..83ade27b 100644 --- a/frontend/src/app/skills/marketplace/page.tsx +++ b/frontend/src/app/skills/marketplace/page.tsx @@ -50,16 +50,20 @@ export default function SkillsMarketplacePage() { const searchParams = useSearchParams(); const { isSignedIn } = useAuth(); const { isAdmin } = useOrganizationMembership(isSignedIn); - const [selectedSkill, setSelectedSkill] = useState(null); + const [selectedSkill, setSelectedSkill] = + useState(null); const [gatewayInstalledById, setGatewayInstalledById] = useState< Record >({}); - const [installedGatewayNamesBySkillId, setInstalledGatewayNamesBySkillId] = useState< - Record - >({}); + const [installedGatewayNamesBySkillId, setInstalledGatewayNamesBySkillId] = + useState>({}); const [isGatewayStatusLoading, setIsGatewayStatusLoading] = useState(false); - const [gatewayStatusError, setGatewayStatusError] = useState(null); - const [installingGatewayId, setInstallingGatewayId] = useState(null); + const [gatewayStatusError, setGatewayStatusError] = useState( + null, + ); + const [installingGatewayId, setInstallingGatewayId] = useState( + null, + ); const { sorting, onSortingChange } = useUrlSorting({ allowedColumnIds: MARKETPLACE_SKILLS_SORTABLE_COLUMNS, @@ -161,25 +165,29 @@ export default function SkillsMarketplacePage() { const updateInstalledGatewayNames = useCallback( ({ skillId, + gatewayId, gatewayName, installed, }: { skillId: string; + gatewayId: string; gatewayName: string; installed: boolean; }) => { setInstalledGatewayNamesBySkillId((previous) => { const installedOn = previous[skillId] ?? []; if (installed) { - if (installedOn.includes(gatewayName)) return previous; + if (installedOn.some((gateway) => gateway.id === gatewayId)) { + return previous; + } return { ...previous, - [skillId]: [...installedOn, gatewayName], + [skillId]: [...installedOn, { id: gatewayId, name: gatewayName }], }; } return { ...previous, - [skillId]: installedOn.filter((name) => name !== gatewayName), + [skillId]: installedOn.filter((gateway) => gateway.id !== gatewayId), }; }); }, @@ -190,7 +198,12 @@ export default function SkillsMarketplacePage() { let cancelled = false; const loadInstalledGatewaysBySkill = async () => { - if (!isSignedIn || !isAdmin || gateways.length === 0 || skills.length === 0) { + if ( + !isSignedIn || + !isAdmin || + gateways.length === 0 || + skills.length === 0 + ) { setInstalledGatewayNamesBySkillId({}); return; } @@ -198,9 +211,10 @@ export default function SkillsMarketplacePage() { try { const gatewaySkills = await Promise.all( gateways.map(async (gateway) => { - const response = await listMarketplaceSkillsApiV1SkillsMarketplaceGet({ - gateway_id: gateway.id, - }); + const response = + await listMarketplaceSkillsApiV1SkillsMarketplaceGet({ + gateway_id: gateway.id, + }); return { gatewayId: gateway.id, gatewayName: gateway.name, @@ -211,16 +225,26 @@ export default function SkillsMarketplacePage() { if (cancelled) return; - const nextInstalledGatewayNamesBySkillId: Record = {}; + const nextInstalledGatewayNamesBySkillId: Record< + string, + { id: string; name: string }[] + > = {}; for (const skill of skills) { nextInstalledGatewayNamesBySkillId[skill.id] = []; } - for (const { gatewayName, skills: gatewaySkillRows } of gatewaySkills) { + for (const { + gatewayId, + gatewayName, + skills: gatewaySkillRows, + } of gatewaySkills) { for (const skill of gatewaySkillRows) { if (!skill.installed) continue; if (!nextInstalledGatewayNamesBySkillId[skill.id]) continue; - nextInstalledGatewayNamesBySkillId[skill.id].push(gatewayName); + nextInstalledGatewayNamesBySkillId[skill.id].push({ + id: gatewayId, + name: gatewayName, + }); } } @@ -250,11 +274,13 @@ export default function SkillsMarketplacePage() { ...previous, [variables.params.gateway_id]: true, })); - const gatewayName = - gateways.find((gateway) => gateway.id === variables.params.gateway_id)?.name; + const gatewayName = gateways.find( + (gateway) => gateway.id === variables.params.gateway_id, + )?.name; if (gatewayName) { updateInstalledGatewayNames({ skillId: variables.skillId, + gatewayId: variables.params.gateway_id, gatewayName, installed: true, }); @@ -277,11 +303,13 @@ export default function SkillsMarketplacePage() { ...previous, [variables.params.gateway_id]: false, })); - const gatewayName = - gateways.find((gateway) => gateway.id === variables.params.gateway_id)?.name; + const gatewayName = gateways.find( + (gateway) => gateway.id === variables.params.gateway_id, + )?.name; if (gatewayName) { updateInstalledGatewayNames({ skillId: variables.skillId, + gatewayId: variables.params.gateway_id, gatewayName, installed: false, }); @@ -314,16 +342,22 @@ export default function SkillsMarketplacePage() { setGatewayStatusError(null); try { const gatewaySkills = await loadSkillsByGateway(); - const entries = gatewaySkills.map(({ gatewayId, skills: gatewaySkillRows }) => { - const row = gatewaySkillRows.find((skill) => skill.id === selectedSkill.id); - return [gatewayId, Boolean(row?.installed)] as const; - }); + const entries = gatewaySkills.map( + ({ gatewayId, skills: gatewaySkillRows }) => { + const row = gatewaySkillRows.find( + (skill) => skill.id === selectedSkill.id, + ); + return [gatewayId, Boolean(row?.installed)] as const; + }, + ); if (cancelled) return; setGatewayInstalledById(Object.fromEntries(entries)); } catch (error) { if (cancelled) return; setGatewayStatusError( - error instanceof Error ? error.message : "Unable to load gateway status.", + error instanceof Error + ? error.message + : "Unable to load gateway status.", ); } finally { if (!cancelled) { @@ -391,7 +425,9 @@ export default function SkillsMarketplacePage() {
{gateways.length === 0 ? (
-

No gateways available yet.

+

+ No gateways available yet. +

Create a gateway first, then return here to manage installs.

@@ -407,7 +443,9 @@ export default function SkillsMarketplacePage() {
{packsQuery.error.message}

) : null} - {mutationError ?

{mutationError}

: null} + {mutationError ? ( +

{mutationError}

+ ) : null}
diff --git a/frontend/src/app/skills/packs/[packId]/edit/page.tsx b/frontend/src/app/skills/packs/[packId]/edit/page.tsx index b95c6607..76f46afd 100644 --- a/frontend/src/app/skills/packs/[packId]/edit/page.tsx +++ b/frontend/src/app/skills/packs/[packId]/edit/page.tsx @@ -36,11 +36,10 @@ export default function EditSkillPackPage() { }, }); - const pack = ( - packQuery.data?.status === 200 ? packQuery.data.data : null - ); + const pack = packQuery.data?.status === 200 ? packQuery.data.data : null; - const saveMutation = useUpdateSkillPackApiV1SkillsPacksPackIdPatch(); + const saveMutation = + useUpdateSkillPackApiV1SkillsPacksPackIdPatch(); return ( ( - { - mutation: { - onSuccess: async () => { - await queryClient.invalidateQueries({ - queryKey: packsQueryKey, - }); - }, + const syncMutation = useSyncSkillPackApiV1SkillsPacksPackIdSyncPost( + { + mutation: { + onSuccess: async () => { + await queryClient.invalidateQueries({ + queryKey: packsQueryKey, + }); }, }, - queryClient, - ); + }, + queryClient, + ); const handleDelete = () => { if (!deleteTarget) return; @@ -113,7 +112,9 @@ export default function SkillsPacksPage() { const response = await syncMutation.mutateAsync({ packId: pack.id, }); - setSyncWarnings(response.data.warnings ?? []); + if (response.status === 200) { + setSyncWarnings(response.data.warnings ?? []); + } } finally { setSyncingPackIds((previous) => { const next = new Set(previous); @@ -124,7 +125,12 @@ export default function SkillsPacksPage() { }; const handleSyncAllPacks = async () => { - if (!isAdmin || isSyncingAll || syncingPackIds.size > 0 || packs.length === 0) { + if ( + !isAdmin || + isSyncingAll || + syncingPackIds.size > 0 || + packs.length === 0 + ) { return; } @@ -145,10 +151,12 @@ export default function SkillsPacksPage() { try { const response = await syncMutation.mutateAsync({ packId: pack.id }); - setSyncWarnings((previous) => [ - ...previous, - ...(response.data.warnings ?? []), - ]); + if (response.status === 200) { + setSyncWarnings((previous) => [ + ...previous, + ...(response.data.warnings ?? []), + ]); + } } catch { hasFailure = true; } finally { @@ -190,9 +198,7 @@ export default function SkillsPacksPage() { size: "md", })} disabled={ - isSyncingAll || - syncingPackIds.size > 0 || - packs.length === 0 + isSyncingAll || syncingPackIds.size > 0 || packs.length === 0 } onClick={() => { void handleSyncAllPacks(); @@ -241,10 +247,14 @@ export default function SkillsPacksPage() {

{packsQuery.error.message}

) : null} {deleteMutation.error ? ( -

{deleteMutation.error.message}

+

+ {deleteMutation.error.message} +

) : null} {syncMutation.error ? ( -

{syncMutation.error.message}

+

+ {syncMutation.error.message} +

) : null} {syncAllError ? (

{syncAllError}

diff --git a/frontend/src/components/organisms/DashboardSidebar.tsx b/frontend/src/components/organisms/DashboardSidebar.tsx index aeaf77fd..3c470af7 100644 --- a/frontend/src/components/organisms/DashboardSidebar.tsx +++ b/frontend/src/components/organisms/DashboardSidebar.tsx @@ -177,7 +177,8 @@ export function DashboardSidebar() { href="/skills/marketplace" className={cn( "flex items-center gap-3 rounded-lg px-3 py-2.5 text-slate-700 transition", - pathname === "/skills" || pathname.startsWith("/skills/marketplace") + pathname === "/skills" || + pathname.startsWith("/skills/marketplace") ? "bg-blue-100 text-blue-800 font-medium" : "hover:bg-slate-100", )} diff --git a/frontend/src/components/skills/MarketplaceSkillsTable.tsx b/frontend/src/components/skills/MarketplaceSkillsTable.tsx index a90a6b95..ca72ffc4 100644 --- a/frontend/src/components/skills/MarketplaceSkillsTable.tsx +++ b/frontend/src/components/skills/MarketplaceSkillsTable.tsx @@ -11,9 +11,13 @@ import { } from "@tanstack/react-table"; import type { MarketplaceSkillCardRead } from "@/api/generated/model"; -import { DataTable, type DataTableEmptyState } from "@/components/tables/DataTable"; +import { + DataTable, + type DataTableEmptyState, +} from "@/components/tables/DataTable"; import { dateCell } from "@/components/tables/cell-formatters"; import { Button, buttonVariants } from "@/components/ui/button"; +import { Badge } from "@/components/ui/badge"; import { SKILLS_TABLE_EMPTY_ICON, useTableSortingState, @@ -25,9 +29,32 @@ import { packsHrefFromPackUrl, } from "@/lib/skills-source"; +function riskBadgeVariant(risk: string | null | undefined) { + const normalizedRisk = (risk || "unknown").trim().toLowerCase(); + + switch (normalizedRisk) { + case "low": + return "success"; + case "medium": + case "moderate": + return "warning"; + case "high": + case "critical": + return "danger"; + case "unknown": + return "outline"; + default: + return "accent"; + } +} + +function riskBadgeLabel(risk: string | null | undefined) { + return (risk || "unknown").trim() || "unknown"; +} + type MarketplaceSkillsTableProps = { skills: MarketplaceSkillCardRead[]; - installedGatewayNamesBySkillId?: Record; + installedGatewayNamesBySkillId?: Record; isLoading?: boolean; sorting?: SortingState; onSortingChange?: OnChangeFn; @@ -78,7 +105,9 @@ export function MarketplaceSkillsTable({ {row.original.name} ) : ( -

{row.original.name}

+

+ {row.original.name} +

)}

( - - {row.original.risk || "unknown"} - + + {riskBadgeLabel(row.original.risk)} + ), }, { @@ -127,6 +159,11 @@ export function MarketplaceSkillsTable({ header: "Source", cell: ({ row }) => { const sourceHref = row.original.source || row.original.source_url; + + if (!sourceHref) { + return No source; + } + return ( { - const installedOn = installedGatewayNamesBySkillId?.[row.original.id] ?? []; + const installedOn = + installedGatewayNamesBySkillId?.[row.original.id] ?? []; if (installedOn.length === 0) { return -; } - const installedOnText = installedOn.join(", "); return ( - - {installedOnText} - +

+ {installedOn.map((gateway, index) => { + const isLast = index === installedOn.length - 1; + return ( + + + {gateway.name} + + {!isLast ? "," : ""} + + ); + })} +
); }, }, diff --git a/frontend/src/components/skills/SkillInstallDialog.tsx b/frontend/src/components/skills/SkillInstallDialog.tsx index 5f97c47a..53a276cf 100644 --- a/frontend/src/components/skills/SkillInstallDialog.tsx +++ b/frontend/src/components/skills/SkillInstallDialog.tsx @@ -48,7 +48,9 @@ export function SkillInstallDialog({ className="max-w-xl p-6 sm:p-7" > - {selectedSkill ? selectedSkill.name : "Install skill"} + + {selectedSkill ? selectedSkill.name : "Install skill"} + Choose one or more gateways where this skill should be installed. @@ -60,14 +62,17 @@ export function SkillInstallDialog({ ) : ( gateways.map((gateway) => { const isInstalled = gatewayInstalledById[gateway.id] === true; - const isUpdatingGateway = installingGatewayId === gateway.id && isMutating; + const isUpdatingGateway = + installingGatewayId === gateway.id && isMutating; return (
-

{gateway.name}

+

+ {gateway.name} +

- diff --git a/frontend/src/components/skills/SkillPacksTable.tsx b/frontend/src/components/skills/SkillPacksTable.tsx index 526ba270..2c070dd3 100644 --- a/frontend/src/components/skills/SkillPacksTable.tsx +++ b/frontend/src/components/skills/SkillPacksTable.tsx @@ -11,7 +11,10 @@ import { } from "@tanstack/react-table"; import type { SkillPackRead } from "@/api/generated/model"; -import { DataTable, type DataTableEmptyState } from "@/components/tables/DataTable"; +import { + DataTable, + type DataTableEmptyState, +} from "@/components/tables/DataTable"; import { dateCell } from "@/components/tables/cell-formatters"; import { Button } from "@/components/ui/button"; import { @@ -62,7 +65,9 @@ export function SkillPacksTable({ header: "Pack", cell: ({ row }) => (
-

{row.original.name}

+

+ {row.original.name} +

{row.original.description || "No description provided."}

@@ -86,7 +91,11 @@ export function SkillPacksTable({ { accessorKey: "branch", header: "Branch", - cell: ({ row }) =>

{row.original.branch || "main"}

, + cell: ({ row }) => ( +

+ {row.original.branch || "main"} +

+ ), }, { accessorKey: "skill_count", @@ -111,7 +120,9 @@ export function SkillPacksTable({ enableSorting: false, cell: ({ row }) => { if (!onSync) return null; - const isThisPackSyncing = Boolean(syncingPackIds?.has(row.original.id)); + const isThisPackSyncing = Boolean( + syncingPackIds?.has(row.original.id), + ); return (