From 40dcf50f4b5a819dd24bd97ae9d5bf04ba4ef5f6 Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Sat, 14 Feb 2026 12:26:45 +0530 Subject: [PATCH] feat(skills): add metadata and branch fields to skill packs and marketplace skills --- backend/app/api/deps.py | 3 +- backend/app/api/skills_marketplace.py | 341 ++++++++++++++++-- backend/app/models/marketplace_skills.py | 5 + backend/app/models/skill_packs.py | 6 + backend/app/schemas/skills_marketplace.py | 8 +- backend/app/services/organizations.py | 90 ++++- ...7e9b6a4f2_add_skills_marketplace_tables.py | 111 ++++++ backend/tests/test_organizations_service.py | 155 +++++++- backend/tests/test_skills_marketplace_api.py | 194 ++++++++++ .../api/generated/model/skillPackCreate.ts | 2 + .../src/api/generated/model/skillPackRead.ts | 2 + .../generated/model/skillPackSyncResponse.ts | 1 + .../app/skills/packs/[packId]/edit/page.tsx | 7 + frontend/src/app/skills/packs/new/page.tsx | 6 + frontend/src/app/skills/packs/page.tsx | 109 +++++- .../skills/MarketplaceSkillForm.tsx | 55 +++ .../src/components/skills/SkillPacksTable.tsx | 5 + 17 files changed, 1049 insertions(+), 51 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index ea4958f..cb4a72d 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -20,6 +20,7 @@ from __future__ import annotations from dataclasses import dataclass from typing import TYPE_CHECKING, Literal +from uuid import UUID from fastapi import Depends, HTTPException, status @@ -196,7 +197,7 @@ BOARD_READ_DEP = Depends(get_board_for_actor_read) async def get_task_or_404( - task_id: str, + task_id: UUID, board: Board = BOARD_READ_DEP, session: AsyncSession = SESSION_DEP, ) -> Task: diff --git a/backend/app/api/skills_marketplace.py b/backend/app/api/skills_marketplace.py index ac42ce7..3c800f2 100644 --- a/backend/app/api/skills_marketplace.py +++ b/backend/app/api/skills_marketplace.py @@ -52,6 +52,20 @@ ALLOWED_PACK_SOURCE_SCHEMES = {"https"} GIT_CLONE_TIMEOUT_SECONDS = 30 GIT_REV_PARSE_TIMEOUT_SECONDS = 10 BRANCH_NAME_ALLOWED_RE = r"^[A-Za-z0-9._/\-]+$" +SKILLS_INDEX_READ_CHUNK_BYTES = 16 * 1024 + + +def _normalize_pack_branch(raw_branch: str | None) -> str: + if not raw_branch: + return "main" + normalized = raw_branch.strip() + if not normalized: + return "main" + if any(ch in normalized for ch in {"\n", "\r", "\t"}): + return "main" + if not re.match(BRANCH_NAME_ALLOWED_RE, normalized): + return "main" + return normalized @dataclass(frozen=True) @@ -64,6 +78,7 @@ class PackSkillCandidate: category: str | None = None risk: str | None = None source: str | None = None + metadata: dict[str, object] | None = None def _skills_install_dir(workspace_root: str) -> str: @@ -145,6 +160,11 @@ def _normalize_repo_source_url(source_url: str) -> str: return normalized +def _normalize_pack_source_url(source_url: str) -> str: + """Normalize pack repository source URLs for uniqueness checks.""" + return _normalize_repo_source_url(source_url) + + def _validate_pack_source_url(source_url: str) -> None: """Validate that a skill pack source URL is safe to clone. @@ -168,6 +188,17 @@ def _validate_pack_source_url(source_url: str) -> None: if host in {"localhost"}: raise ValueError("Pack source URL hostname is not allowed") + if host != "github.com": + raise ValueError( + "Pack source URL must be a GitHub repository URL (https://github.com//)" + ) + + path = parsed.path.strip("/") + if not path or path.count("/") < 1: + raise ValueError( + "Pack source URL must be a GitHub repository URL (https://github.com//)" + ) + try: ip = ipaddress.ip_address(host) except ValueError: @@ -236,31 +267,185 @@ def _coerce_index_entries(payload: object) -> list[dict[str, object]]: return [] +class _StreamingJSONReader: + """Incrementally decode JSON content from a file object.""" + + def __init__(self, file_obj): + self._file_obj = file_obj + self._buffer = "" + self._position = 0 + self._eof = False + self._decoder = json.JSONDecoder() + + def _fill_buffer(self) -> None: + if self._eof: + return + + chunk = self._file_obj.read(SKILLS_INDEX_READ_CHUNK_BYTES) + if not chunk: + self._eof = True + return + self._buffer += chunk + + def _peek(self) -> str | None: + self._skip_whitespace() + if self._position >= len(self._buffer): + return None + return self._buffer[self._position] + + def _skip_whitespace(self) -> None: + while True: + while self._position < len(self._buffer) and self._buffer[self._position].isspace(): + self._position += 1 + + if self._position < len(self._buffer): + return + + self._fill_buffer() + if self._position < len(self._buffer): + return + if self._eof: + return + + def _decode_value(self): + self._skip_whitespace() + + while True: + try: + value, end = self._decoder.raw_decode(self._buffer, self._position) + self._position = end + return value + except json.JSONDecodeError: + if self._eof: + raise RuntimeError("skills_index.json is not valid JSON") + self._fill_buffer() + self._skip_whitespace() + if self._position >= len(self._buffer): + if self._eof: + raise RuntimeError("skills_index.json is not valid JSON") + + def _consume_char(self, expected: str) -> None: + self._skip_whitespace() + if self._position >= len(self._buffer): + self._fill_buffer() + self._skip_whitespace() + if self._position >= len(self._buffer): + raise RuntimeError("skills_index.json is not valid JSON") + + actual = self._buffer[self._position] + if actual != expected: + raise RuntimeError("skills_index.json is not valid JSON") + self._position += 1 + + def read_top_level_entries(self) -> list[dict[str, object]]: + self._fill_buffer() + self._skip_whitespace() + first = self._peek() + if first is None: + raise RuntimeError("skills_index.json is not valid JSON") + + if first == "[": + self._position += 1 + return list(self._read_array_values()) + if first == "{": + self._position += 1 + return list(self._read_skills_from_object()) + raise RuntimeError("skills_index.json is not valid JSON") + + def _read_array_values(self): + while True: + self._skip_whitespace() + current = self._peek() + if current is None: + if self._eof: + raise RuntimeError("skills_index.json is not valid JSON") + continue + if current == "]": + self._position += 1 + return + + if current == ",": + self._position += 1 + continue + + entry = self._decode_value() + if isinstance(entry, dict): + yield entry + + def _read_skills_from_object(self): + while True: + self._skip_whitespace() + current = self._peek() + if current is None: + if self._eof: + raise RuntimeError("skills_index.json is not valid JSON") + continue + + if current == "}": + self._position += 1 + return + + key = self._decode_value() + if not isinstance(key, str): + raise RuntimeError("skills_index.json is not valid JSON") + + self._skip_whitespace() + if self._peek() == ":": + self._position += 1 + else: + self._consume_char(":") + + if key == "skills": + self._skip_whitespace() + current = self._peek() + if current is None: + if self._eof: + raise RuntimeError("skills_index.json is not valid JSON") + continue + + if current != "[": + value = self._decode_value() + if isinstance(value, list): + for entry in value: + if isinstance(entry, dict): + yield entry + continue + + self._position += 1 + yield from self._read_array_values() + else: + self._decode_value() + + self._skip_whitespace() + current = self._peek() + if current == ",": + self._position += 1 + continue + if current == "}": + self._position += 1 + return + + def _collect_pack_skills_from_index( *, repo_dir: Path, source_url: str, branch: str, + discovery_warnings: list[str] | None = None, ) -> list[PackSkillCandidate] | None: index_file = repo_dir / "skills_index.json" if not index_file.is_file(): return None try: - stat = index_file.stat() + with index_file.open(encoding="utf-8") as fp: + payload = _StreamingJSONReader(fp).read_top_level_entries() except OSError as exc: raise RuntimeError("unable to read skills_index.json") from exc - - # Hard cap to avoid loading an arbitrarily large file into memory. - if stat.st_size > 256 * 1024: - raise RuntimeError("skills_index.json is too large") - - try: - payload = json.loads(index_file.read_text(encoding="utf-8")) - except OSError as exc: - raise RuntimeError("unable to read skills_index.json") from exc - except json.JSONDecodeError as exc: - raise RuntimeError("skills_index.json is not valid JSON") from exc + except RuntimeError as exc: + if discovery_warnings is not None: + discovery_warnings.append(f"Failed to parse skills_index.json: {exc}") + return None found: dict[str, PackSkillCandidate] = {} for entry in _coerce_index_entries(payload): @@ -273,15 +458,22 @@ def _collect_pack_skills_from_index( indexed_source = entry.get("source_url") candidate_source_url: str | None = None + resolved_metadata: dict[str, object] = { + "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://")): candidate_source_url = source_candidate else: indexed_rel = _normalize_repo_path(source_candidate) + 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 not candidate_source_url: @@ -326,6 +518,7 @@ def _collect_pack_skills_from_index( category=category, risk=risk, source=source_label, + metadata=resolved_metadata, ) return list(found.values()) @@ -336,11 +529,13 @@ def _collect_pack_skills_from_repo( repo_dir: Path, source_url: str, branch: str, + discovery_warnings: list[str] | None = None, ) -> list[PackSkillCandidate]: indexed = _collect_pack_skills_from_index( repo_dir=repo_dir, source_url=source_url, branch=branch, + discovery_warnings=discovery_warnings, ) if indexed is not None: return indexed @@ -368,6 +563,11 @@ def _collect_pack_skills_from_repo( name=name, description=description, source_url=tree_url, + metadata={ + "discovery_mode": "skills_md", + "pack_branch": branch, + "skill_dir": rel_dir, + }, ) if found: @@ -376,16 +576,42 @@ def _collect_pack_skills_from_repo( return [] -def _collect_pack_skills(source_url: str) -> list[PackSkillCandidate]: +def _collect_pack_skills(*, source_url: str, branch: str) -> 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, + branch=branch, + )[0] + + +def _collect_pack_skills_with_warnings( + *, + source_url: str, + branch: str, +) -> tuple[list[PackSkillCandidate], list[str]]: + """Clone a pack repository and return discovered skills plus sync warnings.""" # Defense-in-depth: validate again at point of use before invoking git. _validate_pack_source_url(source_url) + requested_branch = _normalize_pack_branch(branch) + discovery_warnings: list[str] = [] + with TemporaryDirectory(prefix="skill-pack-sync-") as tmp_dir: repo_dir = Path(tmp_dir) + used_branch = requested_branch try: subprocess.run( - ["git", "clone", "--depth", "1", source_url, str(repo_dir)], + [ + "git", + "clone", + "--depth", + "1", + "--single-branch", + "--branch", + requested_branch, + source_url, + str(repo_dir), + ], check=True, capture_output=True, text=True, @@ -396,15 +622,35 @@ def _collect_pack_skills(source_url: str) -> list[PackSkillCandidate]: except subprocess.TimeoutExpired as exc: raise RuntimeError("timed out cloning pack repository") from exc except subprocess.CalledProcessError as exc: - stderr = (exc.stderr or "").strip() - # Avoid reflecting arbitrary stderr to callers; keep details minimal. - detail = "unable to clone pack repository" - if stderr: - detail = f"{detail}: {stderr.splitlines()[0][:200]}" - raise RuntimeError(detail) from exc + if requested_branch != "main": + try: + subprocess.run( + ["git", "clone", "--depth", "1", source_url, str(repo_dir)], + check=True, + capture_output=True, + text=True, + timeout=GIT_CLONE_TIMEOUT_SECONDS, + ) + used_branch = "main" + except ( + FileNotFoundError, + subprocess.TimeoutExpired, + subprocess.CalledProcessError, + ): + stderr = (exc.stderr or "").strip() + detail = "unable to clone pack repository" + if stderr: + detail = f"{detail}: {stderr.splitlines()[0][:200]}" + raise RuntimeError(detail) from exc + else: + stderr = (exc.stderr or "").strip() + detail = "unable to clone pack repository" + if stderr: + detail = f"{detail}: {stderr.splitlines()[0][:200]}" + raise RuntimeError(detail) from exc try: - branch = subprocess.run( + discovered_branch = subprocess.run( ["git", "-C", str(repo_dir), "rev-parse", "--abbrev-ref", "HEAD"], check=True, capture_output=True, @@ -412,20 +658,16 @@ def _collect_pack_skills(source_url: str) -> list[PackSkillCandidate]: timeout=GIT_REV_PARSE_TIMEOUT_SECONDS, ).stdout.strip() except (FileNotFoundError, subprocess.TimeoutExpired, subprocess.CalledProcessError): - branch = "main" + discovered_branch = used_branch or "main" - branch = branch or "main" - # Fail closed on weird output to avoid constructing malformed URLs. - if any(ch in branch for ch in {"\n", "\r", "\t"}): - branch = "main" - - if not re.match(BRANCH_NAME_ALLOWED_RE, branch): - branch = "main" - - return _collect_pack_skills_from_repo( - repo_dir=repo_dir, - source_url=source_url, - branch=branch, + return ( + _collect_pack_skills_from_repo( + repo_dir=repo_dir, + source_url=source_url, + branch=_normalize_pack_branch(discovered_branch), + discovery_warnings=discovery_warnings, + ), + discovery_warnings, ) @@ -472,6 +714,7 @@ def _as_card( risk=skill.risk, source=skill.source, source_url=skill.source_url, + metadata=skill.metadata_ or {}, created_at=skill.created_at, updated_at=skill.updated_at, installed=installation is not None, @@ -486,6 +729,8 @@ def _as_skill_pack_read(pack: SkillPack) -> SkillPackRead: name=pack.name, description=pack.description, source_url=pack.source_url, + branch=pack.branch or "main", + metadata=pack.metadata_ or {}, skill_count=0, created_at=pack.created_at, updated_at=pack.updated_at, @@ -671,6 +916,9 @@ def _apply_pack_candidate_updates( if existing.source != candidate.source: existing.source = candidate.source changed = True + if existing.metadata_ != (candidate.metadata or {}): + existing.metadata_ = candidate.metadata or {} + changed = True return changed @@ -720,6 +968,7 @@ async def create_marketplace_skill( session.add(existing) await session.commit() await session.refresh(existing) + existing.metadata_ = existing.metadata_ or {} return existing skill = MarketplaceSkill( @@ -727,10 +976,12 @@ async def create_marketplace_skill( source_url=source_url, name=payload.name or _infer_skill_name(source_url), description=payload.description, + metadata={}, ) session.add(skill) await session.commit() await session.refresh(skill) + skill.metadata_ = skill.metadata_ or {} return skill @@ -833,7 +1084,7 @@ async def create_skill_pack( ctx: OrganizationContext = ORG_ADMIN_DEP, ) -> SkillPackRead: """Register a new skill pack source URL.""" - source_url = str(payload.source_url).strip() + source_url = _normalize_pack_source_url(str(payload.source_url)) try: _validate_pack_source_url(source_url) except ValueError as exc: @@ -851,6 +1102,13 @@ async def create_skill_pack( if payload.description is not None and existing.description != payload.description: existing.description = payload.description changed = True + normalized_branch = _normalize_pack_branch(payload.branch) + if existing.branch != normalized_branch: + existing.branch = normalized_branch + changed = True + if existing.metadata_ != payload.metadata: + existing.metadata_ = payload.metadata + changed = True if changed: existing.updated_at = utcnow() session.add(existing) @@ -867,6 +1125,8 @@ async def create_skill_pack( source_url=source_url, name=payload.name or _infer_skill_name(source_url), description=payload.description, + branch=_normalize_pack_branch(payload.branch), + metadata_=payload.metadata, ) session.add(pack) await session.commit() @@ -887,7 +1147,7 @@ async def update_skill_pack( ) -> SkillPackRead: """Update a skill pack URL and metadata.""" pack = await _require_skill_pack_for_org(pack_id=pack_id, session=session, ctx=ctx) - source_url = str(payload.source_url).strip() + source_url = _normalize_pack_source_url(str(payload.source_url)) try: _validate_pack_source_url(source_url) except ValueError as exc: @@ -906,6 +1166,8 @@ async def update_skill_pack( pack.source_url = source_url 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.updated_at = utcnow() session.add(pack) await session.commit() @@ -945,7 +1207,10 @@ async def sync_skill_pack( raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc try: - discovered = _collect_pack_skills(pack.source_url) + discovered, warnings = _collect_pack_skills_with_warnings( + source_url=pack.source_url, + branch=_normalize_pack_branch(pack.branch), + ) except RuntimeError as exc: raise HTTPException( status_code=status.HTTP_502_BAD_GATEWAY, @@ -971,6 +1236,7 @@ async def sync_skill_pack( category=candidate.category, risk=candidate.risk, source=candidate.source, + metadata_=candidate.metadata or {}, ), ) created += 1 @@ -989,4 +1255,5 @@ async def sync_skill_pack( synced=len(discovered), created=created, updated=updated, + warnings=warnings, ) diff --git a/backend/app/models/marketplace_skills.py b/backend/app/models/marketplace_skills.py index 432aef9..b890823 100644 --- a/backend/app/models/marketplace_skills.py +++ b/backend/app/models/marketplace_skills.py @@ -5,6 +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 sqlmodel import Field @@ -34,5 +35,9 @@ class MarketplaceSkill(TenantScoped, table=True): risk: str | None = Field(default=None) source: str | None = Field(default=None) source_url: str + metadata_: dict[str, object] = Field( + default_factory=dict, + sa_column=Column("metadata", JSON, nullable=False), + ) created_at: datetime = Field(default_factory=utcnow) updated_at: datetime = Field(default_factory=utcnow) diff --git a/backend/app/models/skill_packs.py b/backend/app/models/skill_packs.py index a0bb4f5..9e6e6fa 100644 --- a/backend/app/models/skill_packs.py +++ b/backend/app/models/skill_packs.py @@ -5,6 +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 sqlmodel import Field @@ -31,5 +32,10 @@ class SkillPack(TenantScoped, table=True): name: str description: str | None = Field(default=None) source_url: str + branch: str = Field(default="main") + metadata_: dict[str, object] = Field( + default_factory=dict, + sa_column=Column("metadata", JSON, nullable=False), + ) created_at: datetime = Field(default_factory=utcnow) updated_at: datetime = Field(default_factory=utcnow) diff --git a/backend/app/schemas/skills_marketplace.py b/backend/app/schemas/skills_marketplace.py index 0a608de..8b89bf0 100644 --- a/backend/app/schemas/skills_marketplace.py +++ b/backend/app/schemas/skills_marketplace.py @@ -6,7 +6,7 @@ from datetime import datetime from uuid import UUID from pydantic import AnyHttpUrl -from sqlmodel import SQLModel +from sqlmodel import Field, SQLModel from app.schemas.common import NonEmptyStr @@ -27,6 +27,8 @@ class SkillPackCreate(SQLModel): source_url: AnyHttpUrl name: NonEmptyStr | None = None description: str | None = None + branch: str = "main" + metadata: dict[str, object] = Field(default_factory=dict) class MarketplaceSkillRead(SQLModel): @@ -40,6 +42,7 @@ class MarketplaceSkillRead(SQLModel): risk: str | None = None source: str | None = None source_url: str + metadata: dict[str, object] created_at: datetime updated_at: datetime @@ -52,6 +55,8 @@ class SkillPackRead(SQLModel): name: str description: str | None = None source_url: str + branch: str + metadata: dict[str, object] skill_count: int = 0 created_at: datetime updated_at: datetime @@ -81,3 +86,4 @@ class SkillPackSyncResponse(SQLModel): synced: int created: int updated: int + warnings: list[str] = Field(default_factory=list) diff --git a/backend/app/services/organizations.py b/backend/app/services/organizations.py index b39dcfb..33b1d83 100644 --- a/backend/app/services/organizations.py +++ b/backend/app/services/organizations.py @@ -3,9 +3,10 @@ from __future__ import annotations from dataclasses import dataclass -from typing import TYPE_CHECKING, Iterable +from typing import TYPE_CHECKING from fastapi import HTTPException, status +from sqlalchemy.exc import IntegrityError from sqlalchemy import or_ from sqlmodel import col, select @@ -16,6 +17,7 @@ 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.users import User @@ -31,6 +33,30 @@ if TYPE_CHECKING: ) DEFAULT_ORG_NAME = "Personal" + + +def _normalize_skill_pack_source_url(source_url: str) -> str: + """Normalize pack source URL so duplicates with trivial formatting differences match.""" + normalized = str(source_url).strip().rstrip("/") + if normalized.endswith(".git"): + return normalized[: -len(".git")] + return normalized + + +DEFAULT_INSTALLER_SKILL_PACKS = ( + ( + "sickn33/antigravity-awesome-skills", + "antigravity-awesome-skills", + "The Ultimate Collection of 800+ Agentic Skills for Claude Code/Antigravity/Cursor. " + "Battle-tested, high-performance skills for AI agents including official skills from " + "Anthropic and Vercel.", + ), + ( + "BrianRWagner/ai-marketing-skills", + "ai-marketing-skills", + "Marketing frameworks that AI actually executes. Use for Claude Code, OpenClaw, etc.", + ), +) ADMIN_ROLES = {"owner", "admin"} ROLE_RANK = {"member": 0, "admin": 1, "owner": 2} @@ -209,6 +235,40 @@ async def accept_invite( return member +def _get_default_skill_pack_records(org_id: UUID, now: "datetime") -> list[SkillPack]: + """Build default installer skill pack rows for a new organization.""" + source_base = "https://github.com" + seen_urls: set[str] = set() + records: list[SkillPack] = [] + for repo, name, description in DEFAULT_INSTALLER_SKILL_PACKS: + source_url = _normalize_skill_pack_source_url(f"{source_base}/{repo}") + if source_url in seen_urls: + continue + seen_urls.add(source_url) + records.append( + SkillPack( + organization_id=org_id, + name=name, + description=description, + source_url=source_url, + created_at=now, + updated_at=now, + ), + ) + return records + + +async def _fetch_existing_default_pack_sources( + session: AsyncSession, + org_id: UUID, +) -> set[str]: + """Return existing default skill pack URLs for the organization.""" + return { + _normalize_skill_pack_source_url(row.source_url) + for row in await SkillPack.objects.filter_by(organization_id=org_id).all(session) + } + + async def ensure_member_for_user( session: AsyncSession, user: User, @@ -250,10 +310,36 @@ async def ensure_member_for_user( created_at=now, updated_at=now, ) + 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) user.active_organization_id = org_id session.add(user) session.add(member) - await session.commit() + try: + await session.commit() + except IntegrityError as err: + await session.rollback() + existing_member = await get_first_membership(session, user.id) + if existing_member is None: + raise + if user.active_organization_id != existing_member.organization_id: + user.active_organization_id = existing_member.organization_id + session.add(user) + await session.commit() + await session.refresh(existing_member) + return existing_member + + for pack in default_skill_packs: + if pack.source_url in existing_pack_urls: + continue + session.add(pack) + try: + await session.commit() + except IntegrityError: + await session.rollback() + existing_pack_urls.add(pack.source_url) + continue + await session.refresh(member) return member diff --git a/backend/migrations/versions/c9d7e9b6a4f2_add_skills_marketplace_tables.py b/backend/migrations/versions/c9d7e9b6a4f2_add_skills_marketplace_tables.py index 4d8593a..f21e174 100644 --- a/backend/migrations/versions/c9d7e9b6a4f2_add_skills_marketplace_tables.py +++ b/backend/migrations/versions/c9d7e9b6a4f2_add_skills_marketplace_tables.py @@ -23,6 +23,13 @@ def _has_table(table_name: str) -> bool: return sa.inspect(op.get_bind()).has_table(table_name) +def _has_column(table_name: str, column_name: str) -> bool: + if not _has_table(table_name): + return False + columns = sa.inspect(op.get_bind()).get_columns(table_name) + return any(column["name"] == column_name for column in columns) + + def _has_index(table_name: str, index_name: str) -> bool: if not _has_table(table_name): return False @@ -30,6 +37,13 @@ def _has_index(table_name: str, index_name: str) -> bool: return any(index["name"] == index_name for index in indexes) +def _has_constraint(table_name: str, constraint_name: str) -> bool: + if not _has_table(table_name): + return False + constraints = sa.inspect(op.get_bind()).get_check_constraints(table_name) + return any(constraint["name"] == constraint_name for constraint in constraints) + + def upgrade() -> None: if not _has_table("marketplace_skills"): op.create_table( @@ -42,6 +56,12 @@ def upgrade() -> None: sa.Column("risk", sqlmodel.sql.sqltypes.AutoString(), nullable=True), sa.Column("source", sqlmodel.sql.sqltypes.AutoString(), nullable=True), sa.Column("source_url", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column( + "metadata", + sa.JSON(), + nullable=False, + server_default=sa.text("'{}'"), + ), sa.Column("created_at", sa.DateTime(), nullable=False), sa.Column("updated_at", sa.DateTime(), nullable=False), sa.ForeignKeyConstraint( @@ -55,6 +75,32 @@ def upgrade() -> None: name="uq_marketplace_skills_org_source_url", ), ) + if not _has_column("marketplace_skills", "metadata"): + op.add_column( + "marketplace_skills", + sa.Column( + "metadata", + sa.JSON(), + nullable=False, + server_default=sa.text("'{}'"), + ), + ) + if _has_column("marketplace_skills", "resolution_metadata") and not _has_column( + "marketplace_skills", "metadata", + ): + op.execute( + sa.text( + "UPDATE marketplace_skills SET metadata = resolution_metadata WHERE resolution_metadata IS NOT NULL" + ) + ) + elif _has_column("marketplace_skills", "path_metadata") and not _has_column( + "marketplace_skills", "metadata" + ): + op.execute( + sa.text( + "UPDATE marketplace_skills SET metadata = path_metadata WHERE path_metadata IS NOT NULL" + ) + ) marketplace_org_idx = op.f("ix_marketplace_skills_organization_id") if not _has_index("marketplace_skills", marketplace_org_idx): @@ -116,6 +162,18 @@ def upgrade() -> None: sa.Column("name", sqlmodel.sql.sqltypes.AutoString(), nullable=False), sa.Column("description", sqlmodel.sql.sqltypes.AutoString(), nullable=True), sa.Column("source_url", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column( + "branch", + sqlmodel.sql.sqltypes.AutoString(), + nullable=False, + server_default=sa.text("'main'"), + ), + sa.Column( + "metadata", + sa.JSON(), + nullable=False, + server_default=sa.text("'{}'"), + ), sa.Column("created_at", sa.DateTime(), nullable=False), sa.Column("updated_at", sa.DateTime(), nullable=False), sa.ForeignKeyConstraint( @@ -129,6 +187,51 @@ def upgrade() -> None: name="uq_skill_packs_org_source_url", ), ) + if not _has_constraint( + "skill_packs", + "ck_skill_packs_source_url_github", + ): + op.create_check_constraint( + "ck_skill_packs_source_url_github", + "skill_packs", + "source_url LIKE 'https://github.com/%/%'", + ) + if not _has_column("skill_packs", "branch"): + op.add_column( + "skill_packs", + sa.Column( + "branch", + sqlmodel.sql.sqltypes.AutoString(), + nullable=False, + server_default=sa.text("'main'"), + ), + ) + if not _has_column("skill_packs", "metadata"): + op.add_column( + "skill_packs", + sa.Column( + "metadata", + sa.JSON(), + nullable=False, + server_default=sa.text("'{}'"), + ), + ) + if _has_column("skill_packs", "resolution_metadata") and not _has_column( + "skill_packs", "metadata" + ): + op.execute( + sa.text( + "UPDATE skill_packs SET metadata = resolution_metadata WHERE resolution_metadata IS NOT NULL" + ) + ) + elif _has_column("skill_packs", "path_metadata") and not _has_column( + "skill_packs", "metadata" + ): + op.execute( + sa.text( + "UPDATE skill_packs SET metadata = path_metadata WHERE path_metadata IS NOT NULL" + ) + ) skill_packs_org_idx = op.f("ix_skill_packs_organization_id") if not _has_index("skill_packs", skill_packs_org_idx): @@ -141,6 +244,14 @@ def upgrade() -> None: def downgrade() -> None: + skill_pack_github_constraint = "ck_skill_packs_source_url_github" + if _has_constraint("skill_packs", skill_pack_github_constraint): + op.drop_constraint( + skill_pack_github_constraint, + "skill_packs", + type_="check", + ) + skill_packs_org_idx = op.f("ix_skill_packs_organization_id") if _has_index("skill_packs", skill_packs_org_idx): op.drop_index( diff --git a/backend/tests/test_organizations_service.py b/backend/tests/test_organizations_service.py index 8618496..ea3c940 100644 --- a/backend/tests/test_organizations_service.py +++ b/backend/tests/test_organizations_service.py @@ -3,11 +3,13 @@ from __future__ import annotations from dataclasses import dataclass, field +from datetime import datetime from typing import Any from uuid import uuid4 import pytest from fastapi import HTTPException +from sqlalchemy.exc import IntegrityError from app.models.boards import Board from app.models.organization_board_access import OrganizationBoardAccess @@ -15,6 +17,7 @@ from app.models.organization_invite_board_access import OrganizationInviteBoardA 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.users import User from app.schemas.organizations import OrganizationBoardAccessSpec, OrganizationMemberAccessUpdate from app.services import organizations @@ -107,6 +110,44 @@ def test_normalize_role(value: str, expected: str) -> None: assert organizations.normalize_role(value) == expected +def test_normalize_skill_pack_source_url_normalizes_trivial_variants() -> None: + assert ( + organizations._normalize_skill_pack_source_url("https://github.com/org/repo") + == "https://github.com/org/repo" + ) + assert ( + organizations._normalize_skill_pack_source_url("https://github.com/org/repo/") + == "https://github.com/org/repo" + ) + assert ( + organizations._normalize_skill_pack_source_url(" https://github.com/org/repo.git ") + == "https://github.com/org/repo" + ) + + +def test_get_default_skill_pack_records_deduplicates_normalized_urls( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + organizations, + "DEFAULT_INSTALLER_SKILL_PACKS", + ( + ("owner/repo", "pack one", "first"), + ("owner/repo/", "pack duplicate", "duplicate"), + ("owner/repo.git", "pack duplicate again", "duplicate again"), + ("owner/other", "other", "other"), + ), + ) + now = datetime(2025, 1, 1) + records = organizations._get_default_skill_pack_records(org_id=uuid4(), now=now) + + assert len(records) == 2 + assert {pack.source_url for pack in records} == { + "https://github.com/owner/repo", + "https://github.com/owner/other", + } + + def test_role_rank_unknown_role_falls_back_to_member_rank() -> None: assert organizations._role_rank("madeup") == 0 assert organizations._role_rank(None) == 0 @@ -218,7 +259,119 @@ async def test_ensure_member_for_user_creates_personal_org_and_owner( assert any( isinstance(item, Organization) and item.id == out.organization_id for item in session.added ) - assert session.committed == 1 + skill_packs = [ + item + for item in [*session.added, *[record for batch in session.added_all for record in batch]] + if isinstance(item, SkillPack) + ] + assert len(skill_packs) == 2 + pack_sources = {pack.source_url: pack.description for pack in skill_packs} + assert ( + pack_sources["https://github.com/sickn33/antigravity-awesome-skills"] + == "The Ultimate Collection of 800+ Agentic Skills for Claude Code/Antigravity/Cursor. " + "Battle-tested, high-performance skills for AI agents including official skills from " + "Anthropic and Vercel." + ) + assert ( + pack_sources["https://github.com/BrianRWagner/ai-marketing-skills"] + == "Marketing frameworks that AI actually executes. Use for Claude Code, OpenClaw, etc." + ) + assert session.committed == 3 + assert len(session.added_all) == 0 + assert {pack.source_url for pack in skill_packs} == { + "https://github.com/sickn33/antigravity-awesome-skills", + "https://github.com/BrianRWagner/ai-marketing-skills", + } + + +@pytest.mark.asyncio +async def test_ensure_member_for_user_skips_already_existing_default_pack_by_source_url( + monkeypatch: pytest.MonkeyPatch, +) -> None: + user = User(clerk_user_id="u1", email=None) + existing_pack_source = "https://github.com/sickn33/antigravity-awesome-skills/" + + async def _fake_get_active(_session: Any, _user: User) -> None: + return None + + async def _fake_get_first(_session: Any, _user_id: Any) -> None: + return None + + async def _fake_fetch_existing_pack_sources( + _session: Any, + _org_id: Any, + ) -> set[str]: + return {existing_pack_source} + + monkeypatch.setattr(organizations, "get_active_membership", _fake_get_active) + monkeypatch.setattr(organizations, "get_first_membership", _fake_get_first) + monkeypatch.setattr( + organizations, + "_fetch_existing_default_pack_sources", + _fake_fetch_existing_pack_sources, + ) + + session = _FakeSession(exec_results=[_FakeExecResult()]) + + out = await organizations.ensure_member_for_user(session, user) + assert out.user_id == user.id + assert out.role == "owner" + assert out.organization_id == user.active_organization_id + skill_packs = [item for item in session.added if isinstance(item, SkillPack)] + assert len(skill_packs) == 1 + assert skill_packs[0].source_url == "https://github.com/BrianRWagner/ai-marketing-skills" + assert session.committed == 2 + assert len(session.added_all) == 0 + + +@pytest.mark.asyncio +async def test_ensure_member_for_user_recovers_on_default_install_integrity_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + org_id = uuid4() + user = User(clerk_user_id="u1", email=None, active_organization_id=org_id) + existing_member = OrganizationMember( + organization_id=org_id, + user_id=user.id, + role="owner", + ) + + call_count = 0 + + async def _fake_get_active(_session: Any, _user: User) -> None: + return None + + async def _fake_get_first(_session: Any, _user_id: Any) -> OrganizationMember | None: + nonlocal call_count + call_count += 1 + if call_count == 1: + return None + return existing_member + + async def _fake_fetch_existing_pack_sources( + _session: Any, + _org_id: Any, + ) -> set[str]: + return set() + + monkeypatch.setattr(organizations, "get_active_membership", _fake_get_active) + monkeypatch.setattr(organizations, "get_first_membership", _fake_get_first) + monkeypatch.setattr( + organizations, + "_fetch_existing_default_pack_sources", + _fake_fetch_existing_pack_sources, + ) + + session = _FakeSession( + exec_results=[_FakeExecResult(), _FakeExecResult()], + commit_side_effects=[IntegrityError("statement", [], None)], + ) + + out = await organizations.ensure_member_for_user(session, user) + assert out is existing_member + assert out.organization_id == org_id + assert call_count == 2 + assert user.active_organization_id == org_id @pytest.mark.asyncio diff --git a/backend/tests/test_skills_marketplace_api.py b/backend/tests/test_skills_marketplace_api.py index d30dca3..bc087c1 100644 --- a/backend/tests/test_skills_marketplace_api.py +++ b/backend/tests/test_skills_marketplace_api.py @@ -485,6 +485,61 @@ async def test_create_skill_pack_rejects_localhost_source_url() -> None: await engine.dispose() +@pytest.mark.asyncio +async def test_create_skill_pack_is_unique_by_normalized_source_url() -> None: + engine = await _make_engine() + session_maker = async_sessionmaker( + engine, + class_=AsyncSession, + expire_on_commit=False, + ) + try: + async with session_maker() as session: + organization, _gateway = await _seed_base(session) + await session.commit() + + app = _build_test_app(session_maker, organization=organization) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + first = await client.post( + "/api/v1/skills/packs", + json={"source_url": "https://github.com/org/repo"}, + ) + spaced = await client.post( + "/api/v1/skills/packs", + json={"source_url": " https://github.com/org/repo.git "}, + ) + second = await client.post( + "/api/v1/skills/packs", + json={"source_url": "https://github.com/org/repo/"}, + ) + third = await client.post( + "/api/v1/skills/packs", + json={"source_url": "https://github.com/org/repo.git"}, + ) + packs = await client.get("/api/v1/skills/packs") + + assert first.status_code == 200 + assert spaced.status_code == 200 + assert second.status_code == 200 + assert third.status_code == 200 + assert spaced.json()["id"] == first.json()["id"] + assert spaced.json()["source_url"] == first.json()["source_url"] + assert second.json()["id"] == first.json()["id"] + assert second.json()["source_url"] == first.json()["source_url"] + assert third.json()["id"] == first.json()["id"] + assert third.json()["source_url"] == first.json()["source_url"] + assert packs.status_code == 200 + pack_items = packs.json() + assert len(pack_items) == 1 + assert pack_items[0]["source_url"] == "https://github.com/org/repo" + finally: + await engine.dispose() + + @pytest.mark.asyncio async def test_list_skill_packs_includes_skill_count() -> None: engine = await _make_engine() @@ -548,6 +603,109 @@ async def test_list_skill_packs_includes_skill_count() -> None: await engine.dispose() +@pytest.mark.asyncio +async def test_update_skill_pack_rejects_duplicate_normalized_source_url() -> None: + engine = await _make_engine() + session_maker = async_sessionmaker( + engine, + class_=AsyncSession, + expire_on_commit=False, + ) + try: + async with session_maker() as session: + organization, _gateway = await _seed_base(session) + pack_a = SkillPack( + organization_id=organization.id, + source_url="https://github.com/org/repo", + name="Pack A", + ) + pack_b = SkillPack( + organization_id=organization.id, + source_url="https://github.com/org/other-repo", + name="Pack B", + ) + session.add(pack_a) + session.add(pack_b) + await session.commit() + await session.refresh(pack_a) + await session.refresh(pack_b) + + app = _build_test_app(session_maker, organization=organization) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.patch( + f"/api/v1/skills/packs/{pack_b.id}", + json={"source_url": "https://github.com/org/repo/"}, + ) + + assert response.status_code == 409 + assert "already exists" in response.json()["detail"].lower() + + async with session_maker() as session: + pack_rows = ( + await session.exec( + select(SkillPack) + .where(col(SkillPack.organization_id) == organization.id) + .order_by(col(SkillPack.created_at).asc()) + ) + ).all() + assert len(pack_rows) == 2 + assert {str(pack.source_url) for pack in pack_rows} == { + "https://github.com/org/repo", + "https://github.com/org/other-repo", + } + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_update_skill_pack_normalizes_source_url_on_update() -> None: + engine = await _make_engine() + session_maker = async_sessionmaker( + engine, + class_=AsyncSession, + expire_on_commit=False, + ) + try: + async with session_maker() as session: + organization, _gateway = await _seed_base(session) + pack = SkillPack( + organization_id=organization.id, + source_url="https://github.com/org/old", + name="Initial", + ) + session.add(pack) + await session.commit() + await session.refresh(pack) + + app = _build_test_app(session_maker, organization=organization) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.patch( + f"/api/v1/skills/packs/{pack.id}", + json={"source_url": " https://github.com/org/new.git/ "}, + ) + + assert response.status_code == 200 + assert response.json()["source_url"] == "https://github.com/org/new" + + async with session_maker() as session: + updated = ( + await session.exec( + select(SkillPack).where(col(SkillPack.id) == pack.id), + ) + ).one() + assert str(updated.source_url) == "https://github.com/org/new" + finally: + await engine.dispose() + + def test_collect_pack_skills_from_repo_uses_root_index_when_present(tmp_path: Path) -> None: repo_dir = tmp_path / "repo" repo_dir.mkdir() @@ -672,3 +830,39 @@ def test_collect_pack_skills_from_repo_supports_top_level_skill_folders( "https://github.com/BrianRWagner/ai-marketing-skills/tree/main/homepage-audit" in by_source ) + + +def test_collect_pack_skills_from_repo_streams_large_index(tmp_path: Path) -> None: + repo_dir = tmp_path / "repo" + repo_dir.mkdir() + (repo_dir / "SKILL.md").write_text("# Fallback Skill\n", encoding="utf-8") + + huge_description = "x" * (300 * 1024) + (repo_dir / "skills_index.json").write_text( + json.dumps( + { + "skills": [ + { + "id": "oversized", + "name": "Huge Index Skill", + "description": huge_description, + "path": "skills/ignored", + }, + ], + } + ), + encoding="utf-8", + ) + + skills = _collect_pack_skills_from_repo( + repo_dir=repo_dir, + source_url="https://github.com/example/oversized-pack", + branch="main", + ) + + assert len(skills) == 1 + assert ( + skills[0].source_url + == "https://github.com/example/oversized-pack/tree/main/skills/ignored" + ) + assert skills[0].name == "Huge Index Skill" diff --git a/frontend/src/api/generated/model/skillPackCreate.ts b/frontend/src/api/generated/model/skillPackCreate.ts index 40d9ecf..847cf61 100644 --- a/frontend/src/api/generated/model/skillPackCreate.ts +++ b/frontend/src/api/generated/model/skillPackCreate.ts @@ -9,8 +9,10 @@ * Payload used to register a pack URL in the organization. */ export interface SkillPackCreate { + branch?: string; description?: string | null; name?: string | null; + metadata?: Record; /** @minLength 1 */ source_url: string; } diff --git a/frontend/src/api/generated/model/skillPackRead.ts b/frontend/src/api/generated/model/skillPackRead.ts index 9214f84..f615260 100644 --- a/frontend/src/api/generated/model/skillPackRead.ts +++ b/frontend/src/api/generated/model/skillPackRead.ts @@ -12,8 +12,10 @@ export interface SkillPackRead { created_at: string; description?: string | null; id: string; + branch: string; name: string; organization_id: string; + metadata: Record; skill_count?: number; source_url: string; updated_at: string; diff --git a/frontend/src/api/generated/model/skillPackSyncResponse.ts b/frontend/src/api/generated/model/skillPackSyncResponse.ts index 7b83aa8..8f0f099 100644 --- a/frontend/src/api/generated/model/skillPackSyncResponse.ts +++ b/frontend/src/api/generated/model/skillPackSyncResponse.ts @@ -11,6 +11,7 @@ export interface SkillPackSyncResponse { created: number; ok?: boolean; + warnings: string[]; pack_id: string; synced: number; updated: number; diff --git a/frontend/src/app/skills/packs/[packId]/edit/page.tsx b/frontend/src/app/skills/packs/[packId]/edit/page.tsx index 870f403..b95c660 100644 --- a/frontend/src/app/skills/packs/[packId]/edit/page.tsx +++ b/frontend/src/app/skills/packs/[packId]/edit/page.tsx @@ -73,12 +73,17 @@ export default function EditSkillPackPage() { sourceUrl: pack.source_url, name: pack.name, description: pack.description ?? "", + branch: pack.branch || "main", }} sourceLabel="Pack URL" nameLabel="Pack name (optional)" descriptionLabel="Pack description (optional)" + branchLabel="Pack branch (optional)" + branchPlaceholder="main" + showBranch descriptionPlaceholder="Short summary shown in the packs list." requiredUrlMessage="Pack URL is required." + invalidUrlMessage="Pack URL must be a GitHub repository URL (https://github.com//)." submitLabel="Save changes" submittingLabel="Saving..." isSubmitting={saveMutation.isPending} @@ -90,6 +95,8 @@ export default function EditSkillPackPage() { source_url: values.sourceUrl, name: values.name || undefined, description: values.description || undefined, + branch: values.branch || "main", + metadata: pack.metadata || {}, }, }); if (result.status !== 200) { diff --git a/frontend/src/app/skills/packs/new/page.tsx b/frontend/src/app/skills/packs/new/page.tsx index 7c2751b..4490622 100644 --- a/frontend/src/app/skills/packs/new/page.tsx +++ b/frontend/src/app/skills/packs/new/page.tsx @@ -36,7 +36,11 @@ export default function NewSkillPackPage() { nameLabel="Pack name (optional)" descriptionLabel="Pack description (optional)" descriptionPlaceholder="Short summary shown in the packs list." + branchLabel="Pack branch (optional)" + branchPlaceholder="main" + showBranch requiredUrlMessage="Pack URL is required." + invalidUrlMessage="Pack URL must be a GitHub repository URL (https://github.com//)." submitLabel="Add pack" submittingLabel="Adding..." isSubmitting={createMutation.isPending} @@ -47,6 +51,8 @@ export default function NewSkillPackPage() { source_url: values.sourceUrl, name: values.name || undefined, description: values.description || undefined, + branch: values.branch || "main", + metadata: {}, }, }); if (result.status !== 200) { diff --git a/frontend/src/app/skills/packs/page.tsx b/frontend/src/app/skills/packs/page.tsx index b87e483..319cef7 100644 --- a/frontend/src/app/skills/packs/page.tsx +++ b/frontend/src/app/skills/packs/page.tsx @@ -24,7 +24,13 @@ import { ConfirmActionDialog } from "@/components/ui/confirm-action-dialog"; import { useOrganizationMembership } from "@/lib/use-organization-membership"; import { useUrlSorting } from "@/lib/use-url-sorting"; -const PACKS_SORTABLE_COLUMNS = ["name", "source_url", "skill_count", "updated_at"]; +const PACKS_SORTABLE_COLUMNS = [ + "name", + "source_url", + "branch", + "skill_count", + "updated_at", +]; export default function SkillsPacksPage() { const queryClient = useQueryClient(); @@ -32,6 +38,9 @@ export default function SkillsPacksPage() { const { isAdmin } = useOrganizationMembership(isSignedIn); const [deleteTarget, setDeleteTarget] = useState(null); const [syncingPackIds, setSyncingPackIds] = useState>(new Set()); + const [isSyncingAll, setIsSyncingAll] = useState(false); + const [syncAllError, setSyncAllError] = useState(null); + const [syncWarnings, setSyncWarnings] = useState([]); const { sorting, onSortingChange } = useUrlSorting({ allowedColumnIds: PACKS_SORTABLE_COLUMNS, @@ -91,7 +100,9 @@ export default function SkillsPacksPage() { }; const handleSyncPack = async (pack: SkillPackRead) => { - if (syncingPackIds.has(pack.id)) return; + if (isSyncingAll || syncingPackIds.has(pack.id)) return; + setSyncAllError(null); + setSyncWarnings([]); setSyncingPackIds((previous) => { const next = new Set(previous); @@ -99,9 +110,10 @@ export default function SkillsPacksPage() { return next; }); try { - await syncMutation.mutateAsync({ + const response = await syncMutation.mutateAsync({ packId: pack.id, }); + setSyncWarnings(response.data.warnings ?? []); } finally { setSyncingPackIds((previous) => { const next = new Set(previous); @@ -111,6 +123,54 @@ export default function SkillsPacksPage() { } }; + const handleSyncAllPacks = async () => { + if (!isAdmin || isSyncingAll || syncingPackIds.size > 0 || packs.length === 0) { + return; + } + + setSyncAllError(null); + setSyncWarnings([]); + setIsSyncingAll(true); + + try { + let hasFailure = false; + + for (const pack of packs) { + if (!pack.id) continue; + setSyncingPackIds((previous) => { + const next = new Set(previous); + next.add(pack.id); + return next; + }); + + try { + const response = await syncMutation.mutateAsync({ packId: pack.id }); + setSyncWarnings((previous) => [ + ...previous, + ...(response.data.warnings ?? []), + ]); + } catch { + hasFailure = true; + } finally { + setSyncingPackIds((previous) => { + const next = new Set(previous); + next.delete(pack.id); + return next; + }); + } + } + + if (hasFailure) { + setSyncAllError("Some skill packs failed to sync. Please try again."); + } + } finally { + setIsSyncingAll(false); + await queryClient.invalidateQueries({ + queryKey: packsQueryKey, + }); + } + }; + return ( <> - Add pack - +
+ + + Add pack + +
) : null } isAdmin={isAdmin} @@ -167,6 +246,18 @@ export default function SkillsPacksPage() { {syncMutation.error ? (

{syncMutation.error.message}

) : null} + {syncAllError ? ( +

{syncAllError}

+ ) : null} + {syncWarnings.length > 0 ? ( +
+ {syncWarnings.map((warning) => ( +

+ {warning} +

+ ))} +
+ ) : null} diff --git a/frontend/src/components/skills/MarketplaceSkillForm.tsx b/frontend/src/components/skills/MarketplaceSkillForm.tsx index 4cbade1..541e97e 100644 --- a/frontend/src/components/skills/MarketplaceSkillForm.tsx +++ b/frontend/src/components/skills/MarketplaceSkillForm.tsx @@ -9,6 +9,7 @@ type MarketplaceSkillFormValues = { sourceUrl: string; name: string; description: string; + branch: string; }; type MarketplaceSkillFormProps = { @@ -21,9 +22,14 @@ type MarketplaceSkillFormProps = { namePlaceholder?: string; descriptionLabel?: string; descriptionPlaceholder?: string; + branchLabel?: string; + branchPlaceholder?: string; + defaultBranch?: string; requiredUrlMessage?: string; + invalidUrlMessage?: string; submitLabel: string; submittingLabel: string; + showBranch?: boolean; isSubmitting: boolean; onCancel: () => void; onSubmit: (values: MarketplaceSkillFormValues) => Promise; @@ -33,6 +39,7 @@ const DEFAULT_VALUES: MarketplaceSkillFormValues = { sourceUrl: "", name: "", description: "", + branch: "main", }; const extractErrorMessage = (error: unknown, fallback: string) => { @@ -51,7 +58,12 @@ export function MarketplaceSkillForm({ namePlaceholder = "Deploy Helper", descriptionLabel = "Description (optional)", descriptionPlaceholder = "Short summary shown in the marketplace.", + branchLabel = "Branch (optional)", + branchPlaceholder = "main", + defaultBranch = "main", + showBranch = false, requiredUrlMessage = "Skill URL is required.", + invalidUrlMessage = "Skill URL must be a GitHub repository URL (https://github.com//).", submitLabel, submittingLabel, isSubmitting, @@ -59,11 +71,30 @@ export function MarketplaceSkillForm({ onSubmit, }: MarketplaceSkillFormProps) { const resolvedInitial = initialValues ?? DEFAULT_VALUES; + const normalizedDefaultBranch = defaultBranch.trim() || "main"; const [sourceUrl, setSourceUrl] = useState(resolvedInitial.sourceUrl); const [name, setName] = useState(resolvedInitial.name); const [description, setDescription] = useState(resolvedInitial.description); + const [branch, setBranch] = useState( + resolvedInitial.branch?.trim() || normalizedDefaultBranch, + ); const [errorMessage, setErrorMessage] = useState(null); + const isValidSourceUrl = (value: string) => { + try { + const parsed = new URL(value); + if (parsed.protocol !== "https:") return false; + if (parsed.hostname !== "github.com") return false; + const parts = parsed.pathname + .split("/") + .map((segment) => segment.trim()) + .filter((segment) => segment.length > 0); + return parts.length >= 2; + } catch { + return false; + } + }; + const handleSubmit = async (event: React.FormEvent) => { event.preventDefault(); const normalizedUrl = sourceUrl.trim(); @@ -72,6 +103,11 @@ export function MarketplaceSkillForm({ return; } + if (!isValidSourceUrl(normalizedUrl)) { + setErrorMessage(invalidUrlMessage); + return; + } + setErrorMessage(null); try { @@ -79,6 +115,7 @@ export function MarketplaceSkillForm({ sourceUrl: normalizedUrl, name: name.trim(), description: description.trim(), + branch: branch.trim() || normalizedDefaultBranch, }); } catch (error) { setErrorMessage(extractErrorMessage(error, "Unable to save skill.")); @@ -112,6 +149,24 @@ export function MarketplaceSkillForm({ ) : null} + {showBranch ? ( +
+ + setBranch(event.target.value)} + placeholder={branchPlaceholder} + disabled={isSubmitting} + /> +
+ ) : null} +