From 07023644f705ff150b0c61fb60e570403c476319 Mon Sep 17 00:00:00 2001 From: David Abutbul Date: Thu, 11 Jun 2026 10:10:17 +0300 Subject: [PATCH] fix(skill-release): report skillspector scans on PRs --- .github/workflows/skill-release.yml | 160 ++++++++++++++++++++++-- scripts/test-skill-release-workflow.mjs | 34 ++++- 2 files changed, 182 insertions(+), 12 deletions(-) diff --git a/.github/workflows/skill-release.yml b/.github/workflows/skill-release.yml index d9d1224..e679316 100644 --- a/.github/workflows/skill-release.yml +++ b/.github/workflows/skill-release.yml @@ -400,12 +400,15 @@ jobs: } touched_skills_file="$(mktemp)" - git diff --name-only "${BASE_SHA}...${HEAD_SHA}" -- 'skills/*/skill.json' 'skills/*/SKILL.md' \ + git diff --name-only "${BASE_SHA}...${HEAD_SHA}" -- \ + 'skills/*/**' \ + ':(exclude)skills/*/test/**' \ + ':(exclude)skills/*/tests/**' \ | awk -F/ 'NF >= 3 {print $1 "/" $2}' \ | sort -u > "${touched_skills_file}" if [ ! -s "${touched_skills_file}" ]; then - echo "No skill metadata files changed in this PR." + echo "No release-relevant skill package files changed in this PR." rm -f "${touched_skills_file}" exit 0 fi @@ -526,11 +529,6 @@ jobs: md_version_changed=true fi - if [ "${json_version_changed}" != "true" ] && [ "${md_version_changed}" != "true" ]; then - echo "No version bump detected for ${skill_dir}; skipping dry-run." - continue - fi - if [ -z "${head_json_version}" ] || [ -z "${head_md_version}" ] || [ "${head_json_version}" != "${head_md_version}" ]; then echo "::error file=${skill_dir}::Version metadata is invalid for dry-run. Ensure validate-pr-version-sync passes." failures=$((failures + 1)) @@ -777,12 +775,156 @@ jobs: fi if [ "${dry_run_count}" -eq 0 ]; then - echo "No version bumps detected in changed skill metadata files." + echo "No changed skill directories required dry-run assets." exit 0 fi echo "Release dry-run completed successfully for ${dry_run_count} changed skill(s)." + - name: Prepare SkillSpector PR report artifact + if: always() + run: | + set -euo pipefail + rm -rf dist/skillspector-pr-reports + mkdir -p dist/dry-run dist/skillspector-pr-reports + + found_reports=false + while IFS= read -r report_path; do + tag="${report_path#dist/dry-run/}" + tag="${tag%%/*}" + mkdir -p "dist/skillspector-pr-reports/${tag}" + cp "${report_path}" "dist/skillspector-pr-reports/${tag}/skillspector-report.md" + found_reports=true + done < <(find dist/dry-run -path '*/release-assets/skillspector-report.md' -type f | sort) + + if [ "${found_reports}" != "true" ]; then + printf 'No SkillSpector reports were generated for this pull request.\n' > dist/skillspector-pr-reports/NO_SKILLSPECTOR_REPORTS.txt + fi + + - name: Upload SkillSpector PR reports + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: skillspector-pr-reports + path: dist/skillspector-pr-reports + retention-days: 14 + + comment-skillspector-report: + if: github.event_name == 'pull_request' && needs.release.result == 'success' + needs: release + runs-on: ubuntu-latest + continue-on-error: true + permissions: + actions: read + contents: read + issues: write + pull-requests: read + steps: + - name: Download SkillSpector reports + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: skillspector-pr-reports + path: skillspector-pr-reports + + - name: Comment SkillSpector reports + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const fs = require("node:fs/promises"); + const path = require("node:path"); + + const root = "skillspector-pr-reports"; + const maxCommentLength = 65000; + + async function findReports(dir) { + let entries; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch (error) { + if (error.code === "ENOENT") { + return []; + } + throw error; + } + + const reports = []; + for (const entry of entries) { + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + reports.push(...await findReports(fullPath)); + } else if (entry.isFile() && entry.name === "skillspector-report.md") { + reports.push(fullPath); + } + } + return reports; + } + + function tagFromReportPath(reportPath) { + const parts = reportPath.split(path.sep); + const releaseAssetsIndex = parts.lastIndexOf("release-assets"); + if (releaseAssetsIndex > 0) { + return parts[releaseAssetsIndex - 1]; + } + return path.basename(path.dirname(reportPath)); + } + + function buildComment({ tag, report }) { + const marker = ``; + const footer = `_Generated by the Skill Release dry-run for \`${tag}\`._`; + let body = `${marker}\n${report.trimEnd()}\n\n${footer}`; + + if (body.length <= maxCommentLength) { + return body; + } + + const truncatedFooter = [ + "_Report truncated because it exceeds GitHub's comment size limit._", + "_Download the `skillspector-pr-reports` workflow artifact for the full report._", + footer, + ].join("\n"); + const budget = maxCommentLength - marker.length - truncatedFooter.length - 8; + return `${marker}\n${report.slice(0, Math.max(0, budget)).trimEnd()}\n\n${truncatedFooter}`; + } + + const reports = await findReports(root); + if (reports.length === 0) { + core.info("No SkillSpector reports found; nothing to comment."); + return; + } + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + per_page: 100, + }); + + for (const reportPath of reports.sort()) { + const tag = tagFromReportPath(reportPath); + const report = await fs.readFile(reportPath, "utf8"); + const marker = ``; + const body = buildComment({ tag, report }); + const existing = comments.find((comment) => comment.body?.includes(marker)); + + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + core.info(`Updated SkillSpector PR comment for ${tag}.`); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + core.info(`Created SkillSpector PR comment for ${tag}.`); + } + } + simulate-tag-release-build: if: github.event_name == 'pull_request' needs: validate-pr-version-sync @@ -1424,8 +1566,6 @@ jobs: "", process.env.QUICK_INSTALL || "", "", - "### SkillSpector Security Report", - "", report, "", `Download the generated release-payload scan: [skillspector-report.md](https://github.com/${process.env.REPO}/releases/download/${process.env.TAG}/skillspector-report.md)`, diff --git a/scripts/test-skill-release-workflow.mjs b/scripts/test-skill-release-workflow.mjs index 87dfd2b..3c34426 100644 --- a/scripts/test-skill-release-workflow.mjs +++ b/scripts/test-skill-release-workflow.mjs @@ -88,10 +88,16 @@ assert.match( 'Skill release workflow must generate a SkillSpector report for each released skill', ); +assert.doesNotMatch( + workflow, + /"### SkillSpector Security Report"/, + 'GitHub release notes must not add a duplicate SkillSpector heading before the generated report', +); + assert.match( workflow, - /### SkillSpector Security Report[\s\S]*\[skillspector-report\.md\]\(https:\/\/github\.com\/\$\{process\.env\.REPO\}\/releases\/download\/\$\{process\.env\.TAG\}\/skillspector-report\.md\)/, - 'GitHub release notes must include a direct SkillSpector report link', + /readFileSync\("release-assets\/skillspector-report\.md", "utf8"\)[\s\S]*report,[\s\S]*\[skillspector-report\.md\]\(https:\/\/github\.com\/\$\{process\.env\.REPO\}\/releases\/download\/\$\{process\.env\.TAG\}\/skillspector-report\.md\)/, + 'GitHub release notes must embed the generated SkillSpector report and include a direct report link', ); assert.match( @@ -118,6 +124,12 @@ assert.match( 'PR dry-run SkillSpector scan must target the staged release payload, not the source skill directory', ); +assert.match( + workflow, + /Run release dry-run for changed skills[\s\S]*git diff --name-only "\$\{BASE_SHA\}\.\.\.\$\{HEAD_SHA\}" --[\s\S]*'skills\/\*\/\*\*'[\s\S]*':\(exclude\)skills\/\*\/test\/\*\*'[\s\S]*':\(exclude\)skills\/\*\/tests\/\*\*'/, + 'PR dry-run SkillSpector scan must run when any release-relevant skill package file changes', +); + assert.doesNotMatch( workflow, /generate_skillspector_report "\$\{skill_dir\}" "\$\{out_assets\}\/skillspector-report\.md"/, @@ -187,6 +199,24 @@ assert.match( 'SkillSpector report must be included in the signed checksums manifest', ); +assert.match( + workflow, + /Upload SkillSpector PR reports[\s\S]*actions\/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7\.0\.1[\s\S]*name: skillspector-pr-reports/, + 'PR dry-run must upload generated SkillSpector reports as workflow artifacts', +); + +assert.match( + workflow, + /comment-skillspector-report:[\s\S]*needs: release[\s\S]*issues: write[\s\S]*actions\/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8\.0\.1/, + 'Skill release workflow must download generated SkillSpector reports in a separate PR comment job with comment permissions', +); + +assert.match( + workflow, + /clawsec-skillspector-report:\$\{tag\}[\s\S]*github\.rest\.issues\.updateComment[\s\S]*github\.rest\.issues\.createComment/, + 'SkillSpector PR comments must use stable per-skill markers and update existing comments before creating new ones', +); + assert.match( workflow, /Simulate tag release build/,