Switch skill CI validation workflows to vally lint#2030
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the downloaded skill-validator binary with px @microsoft/vally-cli lint in the external plugin quality gates pipeline: - Remove downloadSkillValidator() and SKILL_VALIDATOR_ARCHIVE_URL constant - Replace �uildSkillValidatorArgs() + unSkillValidatorGate() with �uildVallyLintArgs() + unVallyLintGate() that run px vally-cli lint per resolved skill directory (falling back to the full plugin root when no specific skill paths can be resolved from plugin.json) - Rename result keys skill_validator_status / skill_validator_output to �ally_lint_status / �ally_lint_output throughout both �ng/external-plugin-quality-gates.mjs and �ng/external-plugin-intake.mjs - Update PR comment markdown to show 'vally lint' instead of 'skill-validator' - Update CONTRIBUTING.md prose references accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aaronpowell
changed the title
Replace the npx-spawned vally-cli process with a direct call to the
@microsoft/vally core library in the external plugin quality gates scripts:
- Add @microsoft/vally as a devDependency in package.json
- Import runLint and LintConsoleReporter from @microsoft/vally
- Replace runVallyLintGate() process spawn with async API call:
- runLint({ rootPath }) returns structured LintResults
- LintConsoleReporter with a Writable capture stream collects
text output without printing to stdout
- Make runExternalPluginQualityGates() async (propagated to
runExternalPluginPrQualityGates() and both main entry points)
- Use Promise.all in runExternalPluginPrQualityGates() for parallel
plugin checks
- Fix remaining skill_validator_status reference in pr-quality-gates
summary string (now vally-lint=...) and YAML workflow table header
- Add 'npm install @microsoft/vally' step to both calling workflows
This removes a layer of indirection (Node -> npx -> CLI -> library)
and replaces it with a direct in-process library call, which is faster,
more reliable, and gives structured access to lint results.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aaronpowell
removed
targets-main
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the repo’s skill/static-analysis validation from the externally-downloaded skill-validator binary to Vally-based linting (vally lint / @microsoft/vally) across skill CI workflows and external plugin quality gates, and updates related reporting/comment UX and documentation accordingly.
Changes:
- Updated skill PR gate + PR comment workflows to run Vally lint and publish renamed artifacts/outputs.
- Updated nightly skill quality report workflow to run Vally lint for skills and emit an informational “agents not linted” note.
- Refactored external plugin quality gates to use
@microsoft/vallyAPIs and renamed result keys fromskill_validator_*→vally_lint_*, plus updated intake comment formatting and docs.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds @microsoft/vally as a dev dependency for Node-based linting in eng/ scripts. |
| package-lock.json | Locks transitive dependencies introduced by @microsoft/vally. |
| eng/external-plugin-quality-gates.mjs | Replaces binary download + CLI invocation with @microsoft/vally lint execution and new result fields. |
| eng/external-plugin-pr-quality-gates.mjs | Updates PR gate runner for async vally-based quality gates and new summary fields. |
| eng/external-plugin-intake.mjs | Updates intake comment rendering and defaults to new vally_lint_* result keys. |
| CONTRIBUTING.md | Updates external-plugin workflow documentation references to use vally lint. |
| .github/workflows/skill-quality-report.yml | Switches nightly scan to Vally lint and adjusts report parsing/strings for the new output. |
| .github/workflows/skill-check.yml | Switches PR gate checks to npx @microsoft/vally-cli lint, updates artifact/output names and messaging. |
| .github/workflows/skill-check-comment.yml | Updates the commenting workflow to read the renamed artifact/output and adjust UI strings. |
| .github/workflows/external-plugin-quality-gates.yml | Installs @microsoft/vally and runs updated Node-based quality gates. |
| .github/workflows/external-plugin-pr-quality-gates.yml | Installs @microsoft/vally and updates PR quality reporting to use vally_lint_status. |
| .codespellrc | Adds Vally/vally to ignore list to avoid false positives. |
Review details
- Files reviewed: 11/12 changed files
- Comments generated: 7
- Review effort level: Low
Comment on lines
423
to
427
| function normalizeQualityGateResult(rawResult) { | ||
| const defaults = { | ||
| overall_status: "not_run", | ||
| skill_validator_status: "not_run", | ||
| vally_lint_status: "not_run", | ||
| smoke_status: "not_run", |
Comment on lines
+191
to
211
| let combinedOutput = ""; | ||
| let anyFailure = false; | ||
|
|
||
| for (const target of targets) { | ||
| const chunks = []; | ||
| const captureStream = new Writable({ | ||
| write(chunk, _encoding, callback) { | ||
| chunks.push(chunk.toString()); | ||
| callback(); | ||
| }, | ||
| }); | ||
|
|
||
| const result = await runLint({ rootPath: target }); | ||
| const reporter = new LintConsoleReporter({ verbose: true, stream: captureStream }); | ||
| await reporter.report(result); | ||
|
|
||
| combinedOutput += chunks.join("") + "\n"; | ||
| if (!result.passed) { | ||
| anyFailure = true; | ||
| } | ||
| } |
Comment on lines
+74
to
78
| const checkedPlugins = await Promise.all(plugins.map(async (plugin) => { | ||
| const quality = await runExternalPluginQualityGates(plugin); | ||
| return { | ||
| name: plugin?.name ?? "unknown", | ||
| source: plugin?.source ?? {}, |
Comment on lines
+39
to
+40
| - name: Install @microsoft/vally | ||
| run: npm install @microsoft/vally |
Comment on lines
+98
to
+99
| - name: Install @microsoft/vally | ||
| run: npm install @microsoft/vally |
| --skills ./skills \ | ||
| --verbose \ | ||
| 2>&1 | tee sv-skills-output.txt | ||
| npx --yes @microsoft/vally-cli lint ./skills --verbose 2>&1 | tee vally-skills-output.txt |
| for skill_dir in "${SKILL_DIRS[@]}"; do | ||
| echo "### Linting ${skill_dir}" | tee -a vally-output.txt | ||
| set +e | ||
| OUTPUT=$(npx --yes @microsoft/vally-cli lint "$skill_dir" --verbose 2>&1) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the external
skill-validatorbinary (downloaded nightly from GitHub releases) with@microsoft/vally-cli(vally lint) for all skill static-analysis CI checks.Why vally lint?
npx, no curl+tar+chmod per runvally lintenforces the full Agent Skills spec: valid frontmatter types, file-reference integrity (valid-refs), orphan-file detection, and a 500-lineSKILL.mdcapChanges
Skill PR gate & comment (
skill-check.yml,skill-check-comment.yml)actions/setup-node+npx @microsoft/vally-cli lintvally-lint-results/vally-output.txtNightly quality report (
skill-quality-report.yml)SKILL.md, not.agent.md)External plugin quality gates (
eng/external-plugin-quality-gates.mjs)downloadSkillValidator()andSKILL_VALIDATOR_ARCHIVE_URLbuildSkillValidatorArgs()+runSkillValidatorGate()withbuildVallyLintArgs()+runVallyLintGate(): runsnpx @microsoft/vally-cli lintper resolved skill directory, falling back to the full plugin root when no specific skill paths are foundskill_validator_status→vally_lint_status,skill_validator_output→vally_lint_outputExternal plugin intake comments (
eng/external-plugin-intake.mjs)normalizeQualityGateResultdefaults updated to new key namesDocumentation (
CONTRIBUTING.md)skill-validator check --pluginupdated tovally lintBehavioural notes
vally lintexit code but does not fail the workflow (same as before)vallytreats prose absolute paths like/spec/as broken file references (valid-refs: absolute-path). Skills with such documentation text may show new warningsvallyenforces a 500-line cap onSKILL.md. Many existing skills exceed this; the gate is warn-only so this doesn't block PRs but does surface in the quality reportvally lintonly processesSKILL.md;.agent.mdlinting remains out of scope for this change