Skip to content

Improve skills-validator invoking and outputs presenting #1387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aaronpowell merged 1 commit into from
Apr 14, 2026
+78 −20
Merged

Improve skills-validator invoking and outputs presenting #1387

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 78 additions & 20 deletions .github/workflows/skill-check-comment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,41 +44,99 @@ jobs:
const totalChecked = skillCount + agentCount;

const marker = '<!-- skill-validator-results -->';
const output = fs.readFileSync('sv-output.txt', 'utf8').trim();

// Count errors, warnings, advisories from output
const errorCount = (output.match(/\bError\b/gi) || []).length;
const warningCount = (output.match(/\bWarning\b/gi) || []).length;
const advisoryCount = (output.match(/\bAdvisory\b/gi) || []).length;

let statusLine;
if (errorCount > 0) {
statusLine = `**${totalChecked} resource(s) checked** | ⛔ ${errorCount} error(s) | ⚠️ ${warningCount} warning(s) | ℹ️ ${advisoryCount} advisory(ies)`;
} else if (warningCount > 0) {
statusLine = `**${totalChecked} resource(s) checked** | ⚠️ ${warningCount} warning(s) | ℹ️ ${advisoryCount} advisory(ies)`;
} else {
statusLine = `**${totalChecked} resource(s) checked** | ✅ All checks passed`;
const rawOutput = fs.existsSync('sv-output.txt')
? fs.readFileSync('sv-output.txt', 'utf8')
: '';
const output = rawOutput.replace(/\x1b\[[0-9;]*m/g, '').trim();

const errorCount = (output.match(/❌/g) || []).length;
const warningCount = (output.match(/⚠/g) || []).length;
const advisoryCount = (output.match(/ℹ/g) || []).length;

let verdict = '✅ All checks passed';
if (exitCode !== '0' || errorCount > 0) {
verdict = '⛔ Findings need attention';
} else if (warningCount > 0 || advisoryCount > 0) {
verdict = '⚠️ Warnings or advisories found';
}

const highlightedLines = output
.split('\n')
.map(line => line.trim())
.filter(Boolean)
.filter(line => !line.startsWith('###'))
.filter(line => /^[❌⚠ℹ]/.test(line));

const summaryLines = highlightedLines.length > 0
? highlightedLines.slice(0, 10)
: output
.split('\n')
.map(line => line.trim())
.filter(Boolean)
.filter(line => !line.startsWith('###'))
.slice(0, 10);
Comment on lines +70 to +77
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

summaryLines falls back to the first 10 non-empty lines when no emoji-prefixed findings are detected. Since skill-validator output can include headers/info lines (e.g., "## ") even when there are no findings, this can populate the “Finding” table with non-findings and prevent the “No findings…” message from ever showing. Consider only building the findings table from highlightedLines (emoji-prefixed lines), and when that list is empty (and exitCode === '0'), render the no-findings message instead of a fallback slice of arbitrary output.

Suggested change
const summaryLines = highlightedLines.length > 0
? highlightedLines.slice(0, 10)
: output
.split('\n')
.map(line => line.trim())
.filter(Boolean)
.filter(line => !line.startsWith('###'))
.slice(0, 10);
const summaryLines = highlightedLines.slice(0, 10);

Copilot uses AI. Check for mistakes.

const scopeTable = [
'| Scope | Checked |',
'|---|---:|',
`| Skills | ${skillCount} |`,
`| Agents | ${agentCount} |`,
`| Total | ${totalChecked} |`,
];

const severityTable = [
'| Severity | Count |',
'|---|---:|',
`| ❌ Errors | ${errorCount} |`,
`| ⚠️ Warnings | ${warningCount} |`,
`| ℹ️ Advisories | ${advisoryCount} |`,
];

const findingsTable = summaryLines.length === 0
? ['_No findings were emitted by the validator._']
: [
'| Level | Finding |',
'|---|---|',
...summaryLines.map(line => {
const level = line.startsWith('❌')
? '❌'
: line.startsWith('⚠')
? '⚠️'
: line.startsWith('ℹ')
? 'ℹ️'
: (exitCode !== '0' ? '⛔' : 'ℹ️');
const text = line.replace(/^[❌⚠ℹ️\s]+/, '').replace(/\|/g, '\\|');
return `| ${level} | ${text} |`;
}),
];

const body = [
marker,
'## 🔍 Skill Validator Results',
'',
statusLine,
`**${verdict}**`,
'',
...scopeTable,
'',
...severityTable,
'',
'### Summary',
'',
...findingsTable,
'',
'<details>',
'<summary>Full output</summary>',
'<summary>Full validator output</summary>',
'',
'```',
output,
'```text',
output || 'No validator output captured.',
'```',
'',
'</details>',
'',
exitCode !== '0'
? '> **Note:** Errors were found. These are currently reported as warnings and do not block merge. Please review and address when possible.'
? '> **Note:** The validator returned a non-zero exit code. Please review the findings above before merge.'
: '',
].join('\n');
].filter(Boolean).join('\n');

// Find existing comment with our marker
const { data: comments } = await github.rest.issues.listComments({
Expand Down
Loading