fix(ci): ASCII-clean version-matrix.ps1 + reusable PowerShell lint workflow (closes #28)#34
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 46 minutes and 47 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughA dedicated ChangesPowerShell Lint Reusable Workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens CI around the PowerShell example scripts by making examples/version-matrix/version-matrix.ps1 ASCII-clean again (to satisfy the repo’s PSScriptAnalyzer settings) and by extracting the PowerShell lint job into a reusable, path-triggered workflow that can also be called from Extended Verification.
Changes:
- Replaced a non-ASCII em-dash in
version-matrix.ps1to satisfyPSUseBOMForUnicodeEncodedFileexpectations. - Added
.github/workflows/lint-powershell.yml(push/PR path-triggered +workflow_call) and updatedverify-extended.ymlto reuse it. - Updated CI docs/backlog to reflect the new dedicated PowerShell lint workflow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/version-matrix/version-matrix.ps1 | Removes non-ASCII character to keep the script ASCII-clean for PSScriptAnalyzer rules. |
| .github/workflows/verify-extended.yml | Switches the PowerShell lint job to call the reusable workflow. |
| .github/workflows/lint-powershell.yml | New reusable workflow to run PSScriptAnalyzer on examples/**/*.ps1 for early CI feedback. |
| .github/instructions/ci.instructions.md | Documents the additional workflow, but currently drops prior CI guidance (see comment). |
| BACKLOG.md | Notes the change and related follow-up, but wording is more definitive than the PR description supports (see comment). |
Comment on lines
5
to
+9
| # CI System — quickchr | ||
|
|
||
| ## One CI run is a signal, not a fact | ||
|
|
||
| A red job tells you *something changed*, not *what is true*. Before acting on it: | ||
|
|
||
| - **Reproduce locally.** We run QEMU here (x86 under HVF, arm64 under TCG — slow but | ||
| real). A failure seen only in CI is a lead to investigate, not a proven limitation. | ||
| - **Don't let one run cascade.** A single unverified failure is not license to sweep | ||
| doc/code/skill edits. Above all, never `skip`/`os`-gate/`arch`-gate a failing test or | ||
| example to green the pipeline before the behavior is reproduced and root-caused — that | ||
| masks the bug (see `testing.instructions.md` and `examples.instructions.md`). | ||
|
|
||
| ## Workflow Overview | ||
|
|
||
| Three workflows, each with a distinct purpose: | ||
| Four workflows, each with a distinct purpose: |
| - [x] **CI hardening (PR #22 review pass, 2026-06-26):** examples smoke is now a **platform matrix** mirroring integration — `verify-extended.yml` replaces `include-examples` with mode toggles `run-integration`/`run-examples`; a `plan-smoke` job emits the smoke matrix from the platform toggles, so one dispatch runs integration and/or smoke across the chosen OSes against a chosen `routeros-target`. **A broken example gates** the workflow on KVM/HVF platforms (TCG stays informational). The harness picks per-OS representatives (`.ts` everywhere, `.sh`/`.py`-via-`uv` on POSIX, `.ps1` on Windows), runs Python via `uv run`, and **fails fast on a typo'd `EXAMPLE_FILTER`**. PowerShell `.ps1` made ASCII-clean + `examples/PSScriptAnalyzerSettings.psd1` (waives `PSAvoidUsingWriteHost` for interactive demos, with rationale); `Invoke-Qc` now fails on non-zero native exits. `ci.yml` Repo Checks installs shellcheck so `lint:shell` actually enforces POSIX-sh on every push. | ||
| - [x] **PR #22 close-out (2026-06-26):** fixed CodeQL `js/insecure-randomness` by replacing `Math.random()` with `crypto.randomUUID()` in the example helpers (`examples/lib.ts`, `examples/grounding/grounding.test.ts`); the taint flowed through example-built machine names into credential sinks. Fixed a latent unit-test bug (`qemu-args.test.ts` "x86 HVF uses host CPU model") that only surfaced on KVM runners — KVM, like HVF, adds `-cpu host`, so the `else` branch's `cpuIdx == -1` assertion was wrong; it never ran on a KVM runner in normal CI (unit job has no KVM; integration job doesn't run `test/unit/`). Fixed PowerShell `PSUseUsingScopeModifierInNewRunspaces` in `version-matrix.ps1` (switched `Start-Job` from `param()`+`-ArgumentList` to `$using:`). **Removed the standalone `coverage` job** (it re-ran the whole suite) — reworking coverage as a byproduct of the integration jobs is tracked in #30. Opened follow-ups: #26 (rename `tzspGatewayIp`), #27 (troubleshooting capture example), #28 (PowerShell workflow org), #29 (release/verification reuse), #30 (coverage byproduct). | ||
| - [x] **PowerShell lint hardening (2026-06-27, closes #28):** post-merge Extended Verification flagged `PSUseBOMForUnicodeEncodedFile` — the `$using:` fix had left a non-ASCII em-dash in a comment in `version-matrix.ps1` (the exact case `examples/PSScriptAnalyzerSettings.psd1` documents as its ASCII-clean guard); made the file ASCII-clean again. Extracted the PSScriptAnalyzer job into a dedicated reusable `lint-powershell.yml` that runs on its own for any push/PR touching `examples/**/*.ps1` (early signal instead of dispatch-only) and is `uses:`-d by `verify-extended.yml`. Verified green on the PR (the PR touches a `.ps1`, so the new workflow self-tests). | ||
| - [ ] **arm64 + qcow2 snapshots — under investigation (#31):** the `rollback` example smoke fails on macos/arm64 + linux/aarch64 (`savevm`/`loadvm` return clean but the restored CHR never reaches REST), passing on every x86 accelerator. The first-pass "fix" (arch-gating the example to skip it on arm64) was **rejected as masking** — a failing example is a signal, not something to silence (see `examples.instructions.md`). A future session will reproduce locally under TCG and root-cause before deciding whether anything is gated or documented. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/instructions/ci.instructions.md:
- Line 15: The PowerShell lint trigger description is incomplete because it only
mentions .ps1 example changes, but lint-powershell.yml also runs when
examples/PSScriptAnalyzerSettings.psd1 changes. Update the table entry and the
related wording in ci.instructions.md to mirror the actual trigger set, using
the lint-powershell.yml workflow as the source of truth and keeping the
descriptions aligned with both examples/**/*.ps1 and the analyzer settings file.
In @.github/workflows/lint-powershell.yml:
- Around line 44-47: The PowerShell lint workflow installs PSScriptAnalyzer
without pinning a version, so update the install step in lint-powershell.yml to
use a fixed -RequiredVersion alongside Install-Module and keep that version
intentional. Preserve the existing analyzer flow in the workflow job that builds
$settings and runs Invoke-ScriptAnalyzer, but make the module version explicit
so lint behavior stays stable across PSGallery updates.
- Line 40: The checkout step uses the mutable actions/checkout@v5 reference, so
update the workflow to pin the checkout action to a specific commit SHA and
configure the checkout step with persist-credentials set to false. Make this
change in the workflow job that contains the checkout usage so the step remains
easy to identify and review if it moves.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2e92c6cf-60bb-484a-a5db-088c6ceed523
📒 Files selected for processing (5)
.github/instructions/ci.instructions.md.github/workflows/lint-powershell.yml.github/workflows/verify-extended.ymlBACKLOG.mdexamples/version-matrix/version-matrix.ps1
| name: Lint PowerShell examples | ||
| runs-on: windows-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflow ==\n'
cat -n .github/workflows/lint-powershell.yml
printf '\n== ci instructions ==\n'
cat -n .github/instructions/ci.instructions.mdRepository: tikoci/quickchr
Length of output: 22060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== checkout usage across workflows ==\n'
rg -n "actions/checkout|persist-credentials|Install-Module -Name PSScriptAnalyzer" .github/workflows -g '*.yml' -g '*.yaml'
printf '\n== security / pinning guidance in repo ==\n'
rg -n "pin(ned)? action|checkout@|persist-credentials|supply chain|SHA" .github README.md . -g '*.md' -g '*.yml' -g '*.yaml' -g '!node_modules' -g '!dist' -g '!build'Repository: tikoci/quickchr
Length of output: 3534
Pin and harden the checkout step.
actions/checkout@v5 is a mutable tag; pin it to a commit SHA and set persist-credentials: false to reduce supply-chain risk and keep the repo token out of git config.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 40-40: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 40-40: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/lint-powershell.yml at line 40, The checkout step uses the
mutable actions/checkout@v5 reference, so update the workflow to pin the
checkout action to a specific commit SHA and configure the checkout step with
persist-credentials set to false. Make this change in the workflow job that
contains the checkout usage so the step remains easy to identify and review if
it moves.
Source: Linters/SAST tools
…ll lint workflow Post-merge Extended Verification flagged PSUseBOMForUnicodeEncodedFile: the $using: fix from #22 left a non-ASCII em-dash in a comment in version-matrix.ps1 (the exact case examples/PSScriptAnalyzerSettings.psd1 documents as its ASCII-clean guard). Made the file ASCII-clean again, matching the other 9 .ps1 examples. Closes #28: extracted the PSScriptAnalyzer job into a dedicated reusable lint-powershell.yml that runs on its own for any push/PR touching examples/**/*.ps1 (early signal in normal PR CI) and is `uses:`-d by verify-extended.yml, so .ps1 regressions no longer hide behind a manual dispatch. Split out of the held PR #32; the arm64/snapshot half stays there (#31). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mobileskyfi
force-pushed
the
fix/powershell-lint-workflow
branch
from
f9bf19a to
3f93686
Compare
Contributor
Author
|
Thanks both — addressed in the latest push (rebased onto Copilot
CodeRabbit
|
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.
The grounded, CI-verified half of #32, split out so it can land on its own. The arm64/snapshot part stays in #32 (see below).
Underlying fix
The
$using:change in #22 left a non-ASCII em-dash in a comment inversion-matrix.ps1, trippingPSUseBOMForUnicodeEncodedFile— the exact ruleexamples/PSScriptAnalyzerSettings.psd1documents as its ASCII-clean guard. Made the file ASCII-clean again (now consistent with the other 9.ps1examples).Closes #28 — dedicated reusable workflow
Extracted the PSScriptAnalyzer job into
lint-powershell.yml, which:examples/**/*.ps1(+ the analyzer settings) — so.ps1regressions surface in normal PR CI instead of only on a manual Extended Verification dispatch;workflow_call;verify-extended.ymlnowuses:it — one source of truth, dispatch path still covered.Because this PR touches a
.ps1, the new workflow runs on it — self-verifying. (On the prior combined branch it passed in 34s.)Verification
bun run checkgreen (biome, tsc, markdownlint, cspell, validate-examples = 12 conform, shellcheck);actionlintclean on both workflows.Note: touches
.github/instructions/ci.instructions.md(the "Four workflows" doc + the lint-powershell description), which #33 also edits in a different section — expect a trivial merge if both land. The arm64/snapshot masking change is not here; it remains in the held #32, with real grounding tracked in #31.🤖 Generated with Claude Code
Summary by CodeRabbit