fix(skills): address Copilot review nits (safety + UI guards)

This commit is contained in:
Abhimanyu Saharan
2026-02-13 22:50:29 +00:00
parent 294457e76b
commit 84cf22e42b
4 changed files with 30 additions and 2 deletions

View File

@@ -176,6 +176,9 @@ async def delete_gateway(
await service.clear_agent_foreign_keys(agent_id=agent.id)
await session.delete(agent)
# NOTE: The migration declares `ondelete="CASCADE"` for gateway_installed_skills.gateway_id,
# but some backends/test environments (e.g. SQLite without FK pragma) may not
# enforce cascades. Delete rows explicitly to guarantee cleanup semantics.
installed_skills = await GatewayInstalledSkill.objects.filter_by(
gateway_id=gateway.id,
).all(session)

View File

@@ -243,6 +243,15 @@ def _collect_pack_skills_from_index(
if not index_file.is_file():
return None
try:
stat = index_file.stat()
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:
@@ -382,7 +391,10 @@ def _collect_pack_skills(source_url: str) -> list[PackSkillCandidate]:
raise RuntimeError("timed out cloning pack repository") from exc
except subprocess.CalledProcessError as exc:
stderr = (exc.stderr or "").strip()
detail = stderr or "unable to clone pack repository"
# 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
try:
@@ -397,6 +409,9 @@ def _collect_pack_skills(source_url: str) -> list[PackSkillCandidate]:
branch = "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"
return _collect_pack_skills_from_repo(
repo_dir=repo_dir,