Skip to content

fix(scripts): run ruff format check and lock-bound ruff in lint:py #2709

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

Open
Bill Berry (WilliamBerryiii) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(scripts): run ruff format check and lock-bound ruff in lint:py #2709

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion .devcontainer/scripts/on-create.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ main() {
rm /tmp/uv.tar.gz

echo "Syncing Python environments for skills..."
find .github/skills -name pyproject.toml -type f -execdir uv sync \;
find .github/skills -name pyproject.toml -type f -execdir uv sync --locked \;

@jkim323 Jamie Kim (jkim323) Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this propagate a failed per-project uv sync --locked? With find -execdir ... \;, a nonzero command exit only makes that find expression evaluate false; it does not cause find itself to fail, so set -euo pipefail will not stop container setup. A stale skill lock can therefore leave its .venv uncreated while on-create.sh still reports success, and only fail later in lint:py as a missing/mismatched Ruff environment. Could we replace this with an explicit loop or wrapper that returns nonzero when any project sync fails, matching the direct moderation sync below?

@katriendg Katrien De Graeve (katriendg) Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The --locked change is right, but this loop is now load-bearing for the new lint contract and there's a project it doesn't reach.

Get-PythonSkill scans the whole repo. Outside .github/skills there are two Python projects:

./scripts/evals/moderation        [uv.lock / no .venv]
./.github/hooks/shared/telemetry  [uv.lock, ruff 0.15.16]

.github/hooks/shared/telemetry is provisioned by neither this loop (.github/skills only) nor copilot-setup-steps.yml. Under the new verify-only contract, a fresh devcontainer or coding-agent runner will fail npm run lint:py — and therefore npm run validate:local — on that project with uv.lock requires ruff 0.15.16 … Run 'uv sync --locked'. CI won't catch it, because python-lint.yml syncs each matrix directory itself, and it happens to have a .venv in existing containers, which is why local validation passed here.

Two options: extend provisioning to cover non-skill projects, or narrow local discovery to the roots that are actually provisioned.

Separately, copilot-setup-steps.yml:123 still runs bare uv sync while this moved to --locked; the repo instructions ask that both environments be evaluated together, so it's worth deciding deliberately whether they should diverge.


echo "Syncing Python environment for moderation eval..."
(cd scripts/evals/moderation && uv sync --locked)
Expand Down
14 changes: 11 additions & 3 deletions docs/architecture/workflows.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ title: Build Workflows
description: GitHub Actions CI/CD pipeline architecture for validation, security, and release automation
sidebar_position: 3
author: WilliamBerryiii
ms.date: 2026-08-10
ms.date: 2026-08-13
ms.topic: overview
---

Expand Down Expand Up @@ -120,7 +120,7 @@ Individual validation workflows called by orchestration workflows:
| `plugin-package.yml` | Plugin packaging | N/A |
| `plugin-validation.yml` | Marketplace package metadata and closure | `npm run lint:marketplace` |
| `extension-marketplace-publish.yml` | Extension marketplace publishing | N/A |
| `python-lint.yml` | Python linting (ruff) | `npm run lint:py` |
| `python-lint.yml` | Python lint and format checks (ruff) | `npm run lint:py` |
| `pytest-tests.yml` | Python unit tests | `npm run test:py` |
| `pip-audit.yml` | Python dependency auditing | N/A (pip-audit direct) |
| `fuzz-tests.yml` | Python fuzz testing | N/A (pytest direct) |
Expand Down Expand Up @@ -388,7 +388,7 @@ Workflows invoke validation through npm scripts defined in `package.json`:
| `extension:package:prerelease` | `Package-Extension.ps1 -PreRelease` | extension-package.yml |
| `plugin:generate` | `Generate-Plugins.ps1` + post-process | plugin-package.yml |
| `plugin:validate` | Marketplace package metadata and closure validation | plugin-validation.yml |
| `lint:py` | `ruff check` | python-lint.yml |
| `lint:py` | `ruff check` + `ruff format --check` | python-lint.yml |
| `lint:models` | `Validate-ModelReferences.ps1` | model-validation.yml |
| `lint:ai-artifacts` | `Validate-PlannerArtifacts.ps1 -FailOnMissing` | ai-artifact-validation.yml |
| `lint:permissions` | `Test-WorkflowPermissions.ps1` | workflow-permissions-scan.yml |
Expand Down Expand Up @@ -427,6 +427,14 @@ Workflows invoke validation through npm scripts defined in `package.json`:
| `ci:eval:agent:report` | Runs `ci:eval:agent:matrix` then `ci:eval:agent:dashboard` | CI-owned noninteractive report lane |
| `ci:eval:agent:report:dryrun` | Runs `ci:eval:agent:matrix:dryrun` then `ci:eval:agent:dashboard` | CI-owned noninteractive dry-run report lane |

### Python Lint Parity

`npm run lint:py` runs the same command set as `python-lint.yml`: `ruff check` followed by the non-mutating `ruff format --check`. Execution conditions still differ in three ways:

* Provisioning: the hosted lane runs `uv sync --locked` itself, while the local runner only verifies that a project committing `uv.lock` already provides that exact ruff version. When it does not, the local run fails before ruff executes and reports `uv sync --locked` as the setup action. Projects without a `uv.lock` fall back to the project `.venv` ruff and then a global ruff, with no version guarantee.
* Project scope: local discovery covers every directory containing a `pyproject.toml`, including projects that `pr-validation.yml` excludes from its per-PR matrix.

@katriendg Katrien De Graeve (katriendg) Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This bullet accurately describes the scope difference, but it understates the consequence now that resolution is strict.

scripts/evals/moderation is excluded from the per-PR matrix precisely because of the torch/detoxify stack (~935 MB). Local discovery still picks it up, and it now requires an exactly-locked ruff there. npm run lint:py is part of validate:local, so the practical effect is that every contributor must provision that heavyweight environment just to run the local aggregate — including contributors who previously had a green run via a global ruff.

Could this bullet say explicitly that the wider local scope is now fatal rather than merely broader? And is it worth giving the runner an exclusion that mirrors the CI one (or a -SkipProject), so the local aggregate doesn't inherit a cost that CI deliberately declined to pay per-PR?

* Execution gate: the hosted lane defaults to running only when a pull request changes `.py` or `.pyi` files, while the local lane always scans every discovered project.

## Related Documentation

* [Testing Architecture](testing.md) - PowerShell Pester test infrastructure
Expand Down
62 changes: 46 additions & 16 deletions scripts/linting/Invoke-PythonLint.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
# Invoke-PythonLint.ps1
#
# Purpose: Python lint runner. Discovers Python skills via pyproject.toml and
# invokes ruff against each. Defaults to read-only `ruff check` for CI
# gating. With `-Fix`, applies `ruff check --fix` followed by
# `ruff format` (mutates source; intended for local developer use).
# invokes ruff against each. Defaults to read-only `ruff check` plus
# `ruff format --check` for CI-equivalent gating. With `-Fix`, applies
# `ruff check --fix` followed by `ruff format` (mutates source;
# intended for local developer use). Projects that commit uv.lock must
# already provide a ruff binary matching the locked version; the runner
# verifies it and never synchronizes dependencies.
# Author: HVE Core Team

#Requires -Version 7.4
Expand Down Expand Up @@ -69,20 +72,27 @@ function Invoke-PythonLint {
if ($Fix) {
Write-Host "`nRunning ruff --fix and ruff format in $skillPath..." -ForegroundColor Cyan
} else {
Write-Host "`nRunning ruff in $skillPath..." -ForegroundColor Cyan
Write-Host "`nRunning ruff check and ruff format --check in $skillPath..." -ForegroundColor Cyan
}

Push-Location $skillPath
try {
$ruffCmd = Resolve-RuffCommand -SkillPath $skillPath -GlobalRuffAvailable $globalRuffAvailable
$resolution = Resolve-ProjectRuff -SkillPath $skillPath -GlobalRuffAvailable $globalRuffAvailable

@katriendg Katrien De Graeve (katriendg) Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth reconciling this with the sibling lane. scripts/linting/Invoke-PythonTests.ps1 (lines ~89-96) already does lock-aware resolution, but it provisions — uv sync --locked --dev when uv.lock exists, uv sync --dev otherwise — and then executes through uv run.

So after this change, npm run test:py repairs exactly the environment npm run lint:py refuses to repair, and a contributor blocked by the lint failure can unblock it by running the test lane. Going through uv run ruff here would have given exact CI command parity and automatic provisioning for free.

The verify-only choice is defensible — the PR description makes a good case that lint shouldn't implicitly install 935 MB — but right now it's a silent departure from an established in-repo precedent. A short comment here (or a line in the parity section) stating that lint intentionally verifies while the test lane provisions would keep the next person from "fixing" the inconsistency in the wrong direction.

$ruffCmd = $resolution.command

if (-not $ruffCmd) {
Write-Host '❌ ruff not available (no .venv and not installed globally)' -ForegroundColor Red
Write-Host "❌ ruff not available: $($resolution.reason)" -ForegroundColor Red
$results.success = $false
$results.errors += $skillPath
continue
}

if ($resolution.resolutionMode -eq 'locked') {
Write-Host " using ruff $($resolution.resolvedVersion) pinned by uv.lock" -ForegroundColor Gray
} else {
Write-Host ' using unlocked ruff fallback (no uv.lock in this project)' -ForegroundColor Gray
}

if ($Fix) {
# Step 1: autofix lint rules
$fixOutput = & $ruffCmd check . --fix 2>&1
Expand All @@ -101,6 +111,9 @@ function Invoke-PythonLint {
output = $combinedOutput
fixExitCode = $fixExit
formatExitCode = $formatExit
resolutionMode = $resolution.resolutionMode
lockedVersion = $resolution.lockedVersion
resolvedVersion = $resolution.resolvedVersion
}

$results.details += $result
Expand All @@ -123,28 +136,45 @@ function Invoke-PythonLint {
Write-Host '✓ Autofix and format complete' -ForegroundColor Green
}
} else {
$output = & $ruffCmd check . 2>&1
$exitCode = $LASTEXITCODE
# Both gates always run so a lint failure never hides a formatting failure.
$checkOutput = & $ruffCmd check . 2>&1
$checkExit = $LASTEXITCODE

$formatCheckOutput = & $ruffCmd format --check . 2>&1
$formatCheckExit = $LASTEXITCODE

$combinedOutput = (@($checkOutput) + @($formatCheckOutput)) | Out-String
$passed = ($checkExit -eq 0 -and $formatCheckExit -eq 0)

$result = @{
path = $skillPath
passed = ($exitCode -eq 0)
output = $output | Out-String
passed = $passed
output = $combinedOutput
checkExitCode = $checkExit
formatExitCode = $formatCheckExit
resolutionMode = $resolution.resolutionMode
lockedVersion = $resolution.lockedVersion
resolvedVersion = $resolution.resolvedVersion
}

$results.details += $result
$results.skillsChecked++

if ($exitCode -ne 0) {
Write-Host "$output" -ForegroundColor Red
Write-Host '❌ Linting issues found' -ForegroundColor Red
if (-not $passed) {
Write-Host "$combinedOutput" -ForegroundColor Red
if ($checkExit -ne 0) {
Write-Host '❌ Linting issues found' -ForegroundColor Red
}
if ($formatCheckExit -ne 0) {
Write-Host "❌ Formatting issues found (run 'npm run lint:py:fix' to apply)" -ForegroundColor Red
}
$results.success = $false
$results.errors += $skillPath
} else {
if ($output) {
Write-Host "$output"
if ($combinedOutput.Trim()) {
Write-Host "$combinedOutput"
}
Write-Host '✓ No linting issues' -ForegroundColor Green
Write-Host '✓ No linting or formatting issues' -ForegroundColor Green
}
}
} catch {
Expand Down
189 changes: 187 additions & 2 deletions scripts/linting/Modules/PythonLintHelpers.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ function Resolve-RuffCommand {
.DESCRIPTION
Prefers the skill's own .venv ruff binary (Linux or Windows path), then
falls back to a globally installed ruff. Returns $null when neither is
available.
available. This selector makes no version guarantee; callers that must
honor a committed uv.lock use Resolve-ProjectRuff instead.

.PARAMETER SkillPath
Skill directory to inspect.
Expand Down Expand Up @@ -89,6 +90,190 @@ function Resolve-RuffCommand {
return $null
}

function Get-LockedRuffVersion {
<#
.SYNOPSIS
Reads the ruff version recorded in a uv.lock file.

.DESCRIPTION
Parses the [[package]] entries of a uv.lock file and returns the version
string of the ruff package. Returns $null when the file is missing, cannot
be read, does not lock ruff, or records ruff without a parsable version.

.PARAMETER LockPath
Path to the uv.lock file.

.OUTPUTS
Version string, or $null when no locked ruff version is available.
#>
[CmdletBinding()]
[OutputType([string])]
param(
[Parameter(Mandatory = $true)]
[string]$LockPath
)

if (-not (Test-Path $LockPath)) { return $null }

try {
$lines = Get-Content -Path $LockPath -ErrorAction Stop
} catch {
return $null
}

$inRuffPackage = $false
foreach ($line in $lines) {
$trimmed = $line.Trim()

if ($trimmed -eq '[[package]]') {
$inRuffPackage = $false
continue
}

# Any other table header ends the current [[package]] block.
if ($trimmed -match '^\[' ) {
$inRuffPackage = $false
continue
}

if ($trimmed -match '^name\s*=\s*"([^"]+)"$') {
$inRuffPackage = ($Matches[1] -eq 'ruff')
continue
}

if ($inRuffPackage -and $trimmed -match '^version\s*=\s*"([^"]+)"$') {
return $Matches[1]
}
}

return $null
}

function Get-RuffVersionString {
<#
.SYNOPSIS
Reports the version of a ruff binary.

.DESCRIPTION
Invokes `<ruff> --version` and extracts the semantic version from its
output. Returns $null when the binary cannot be executed or its output
contains no recognizable version.

.PARAMETER RuffCommand
Path to a ruff binary, or 'ruff' for the binary on PATH.

.OUTPUTS
Version string, or $null when the version cannot be determined.
#>
[CmdletBinding()]
[OutputType([string])]
param(
[Parameter(Mandatory = $true)]
[string]$RuffCommand
)

try {
$output = & $RuffCommand --version 2>&1 | Out-String
} catch {
return $null
}

if ($output -match '(\d+\.\d+\.\d+[^\s]*)') { return $Matches[1] }
return $null
}

function Resolve-ProjectRuff {
<#
.SYNOPSIS
Resolves the ruff binary a Python project must use, honoring uv.lock.

.DESCRIPTION
When the project commits a uv.lock that records ruff, only a binary whose
reported version exactly matches that locked version is accepted. Candidates
are evaluated in order: the project's own .venv ruff (Linux then Windows
layout), then a globally installed ruff. No candidate is executed for
linting when none matches, and no dependency synchronization is performed;
the caller reports the required version and the `uv sync --locked` setup
action instead.

When the project has no uv.lock, resolution falls back to the historical
behavior of preferring the project's .venv ruff over a global ruff, and the
unlocked resolution mode is reported so callers can surface it.

.PARAMETER SkillPath
Project directory to inspect.

.PARAMETER GlobalRuffAvailable
Whether ruff is available on PATH.

.OUTPUTS
Hashtable with command, resolutionMode, lockedVersion, resolvedVersion,
mismatches, and reason keys. command is $null when resolution fails.
#>
[CmdletBinding()]
[OutputType([hashtable])]
param(
[Parameter(Mandatory = $true)]
[string]$SkillPath,

[Parameter(Mandatory = $true)]
[bool]$GlobalRuffAvailable
)

$resolution = @{
command = $null
resolutionMode = $null
lockedVersion = $null
resolvedVersion = $null
mismatches = @()
reason = $null
}

$lockPath = Join-Path $SkillPath 'uv.lock'

if (-not (Test-Path $lockPath)) {
$resolution.resolutionMode = 'unlocked-fallback'
$resolution.command = Resolve-RuffCommand -SkillPath $SkillPath -GlobalRuffAvailable $GlobalRuffAvailable
if (-not $resolution.command) {
$resolution.reason = 'ruff not available (no .venv ruff and no global ruff), and no uv.lock pins a version'
}
return $resolution
}

$resolution.resolutionMode = 'locked'
$lockedVersion = Get-LockedRuffVersion -LockPath $lockPath

if (-not $lockedVersion) {
$resolution.reason = 'uv.lock does not record a usable ruff version (ruff is not locked or the lock is malformed)'
return $resolution
}

$resolution.lockedVersion = $lockedVersion

$candidates = @(
(Join-Path $SkillPath '.venv/bin/ruff')
(Join-Path $SkillPath '.venv/Scripts/ruff.exe')
) | Where-Object { Test-Path $_ }

if ($GlobalRuffAvailable) { $candidates = @($candidates) + 'ruff' }

foreach ($candidate in $candidates) {
$candidateVersion = Get-RuffVersionString -RuffCommand $candidate
if ($candidateVersion -eq $lockedVersion) {
$resolution.command = $candidate
$resolution.resolvedVersion = $candidateVersion
return $resolution
}

$reported = if ($candidateVersion) { $candidateVersion } else { 'unknown version' }
$resolution.mismatches += "$candidate ($reported)"
}

$found = if ($resolution.mismatches) { "found $($resolution.mismatches -join ', ')" } else { 'found no ruff candidate' }
$resolution.reason = "uv.lock requires ruff $lockedVersion but $found. Run 'uv sync --locked' in this project."

@jkim323 Jamie Kim (jkim323) Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hm could we consider using uv sync --locked followed by uv run ruff here, matching python-lint.yml and the siblingInvoke-PythonTests.ps1 runner, instead of manually parsing uv.lock and comparing ruff --version strings? uv sync --locked would provide the missing fresh-environment bootstrap, while uv run ruff would use the project’s lock-selected executable—the same model CI uses. The current resolver recreates only part of uv’s lock/environment-selection behavior and creates a divergent provisioning contract from test:py for the same discovered projects. Was the verify-only divergence intentional and tested against every discovered project?

return $resolution
}

function Write-PythonLintResults {
<#
.SYNOPSIS
Expand Down Expand Up @@ -144,4 +329,4 @@ function Write-PythonLintResults {
return $OutputPath
}

Export-ModuleMember -Function Get-PythonSkill, Resolve-RuffCommand, Write-PythonLintResults
Export-ModuleMember -Function Get-PythonSkill, Resolve-RuffCommand, Get-LockedRuffVersion, Get-RuffVersionString, Resolve-ProjectRuff, Write-PythonLintResults
Loading
Loading