fix(ci): arm64 rollback smoke gate + dedicated PowerShell lint workflow#32
Closed
mobileskyfi wants to merge 1 commit into
Closed
fix(ci): arm64 rollback smoke gate + dedicated PowerShell lint workflow#32mobileskyfi wants to merge 1 commit into
mobileskyfi wants to merge 1 commit into
Conversation
Post-merge Extended Verification (run 28281373854 on main) surfaced two reds. PowerShell lint: the `$using:` fix from PR #22 left a non-ASCII em-dash in a comment, tripping PSUseBOMForUnicodeEncodedFile (the rule examples/ PSScriptAnalyzerSettings.psd1 documents as the ASCII-clean guard). Made version-matrix.ps1 ASCII-clean again. Also closes #28: extracted the PSScriptAnalyzer job into a reusable lint-powershell.yml that runs on its own for examples/**/*.ps1 push/PR changes (early signal) and is `uses:`-d by verify-extended.yml, so .ps1 regressions no longer hide behind a manual dispatch. arm64 rollback smoke: the rollback example failed on macos/arm64 + linux/aarch64 while passing on every x86 accelerator. QEMU's internal savevm/loadvm snapshots don't restore a working aarch64 `virt` CHR — loadvm returns clean but the guest wedges and REST never returns (60s waitForBoot exhausted). Gated the example to arch: ["x64"] in the smoke matrix (new `arch` field mirroring the existing `os` gate); documented in DESIGN.md, the snapshot API docs, and the example header. Real fix tracked in #31. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughExtracts PSScriptAnalyzer linting from ChangesPowerShell Lint Workflow Extraction
arm64 Rollback Gating
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 signal for examples and PowerShell scripts by (1) extracting PowerShell linting into a reusable workflow that also runs on normal PRs, and (2) gating the rollback example smoke run to x64-only due to an aarch64 QEMU snapshot/restore limitation (documented across design/API/example docs).
Changes:
- Add
archgating support to the examples smoke harness and restrictrollbacksmoke toarch: ["x64"]. - Introduce a dedicated reusable
lint-powershell.ymlworkflow and switch Extended Verification to call it. - Fix
version-matrix.ps1to be ASCII-clean again; document the snapshot limitation inDESIGN.md,types.ts, and the example header; update CI docs and backlog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/integration/examples-smoke.test.ts | Adds arch gating and restricts rollback to x64 in smoke runs. |
| src/lib/types.ts | Documents the practical x86-only behavior of internal snapshots for aarch64 virt. |
| examples/version-matrix/version-matrix.ps1 | Removes non-ASCII character to satisfy PSScriptAnalyzer ASCII guard. |
| examples/rollback/rollback.ts | Documents aarch64 snapshot limitation and smoke skip rationale. |
| DESIGN.md | Records the arm64 snapshot caveat as a design constraint. |
| BACKLOG.md | Captures the post-merge fixes and references the tracked follow-up. |
| .github/workflows/verify-extended.yml | Replaces inline PSScriptAnalyzer job with a reusable workflow call. |
| .github/workflows/lint-powershell.yml | New dedicated PSScriptAnalyzer workflow (push/PR paths + workflow_call). |
| .github/instructions/ci.instructions.md | Updates CI docs to include the new PowerShell lint workflow. |
Comment on lines
91
to
+94
| const want = (name: string) => FILTER.length === 0 || FILTER.includes(name); | ||
| const applies = (r: Runnable) => !r.os || r.os.includes(process.platform); | ||
| const applies = (r: Runnable) => | ||
| (!r.os || r.os.includes(process.platform)) && | ||
| (!r.arch || r.arch.includes(process.arch)); |
![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: 2
🤖 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/workflows/lint-powershell.yml:
- Around line 16-33: Add a concurrency guard to the lint workflow so only the
latest run for the same branch or PR stays active; in the workflow defined by
the on/pull_request and workflow_call sections, introduce a concurrency group
based on the ref (or PR head) and enable cancelation of in-progress runs. Keep
the change scoped to this workflow so repeated pushes do not leave older
windows-latest lint jobs running after newer commits.
- Line 40: The workflow currently uses a mutable actions reference for
actions/checkout, so update the checkout step to a full commit SHA and, if this
job does not need git authentication later, disable persisted credentials by
setting persist-credentials to false. Use the existing checkout step in the
workflow to make the change.
🪄 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: 86d7454d-cb89-451a-8e71-71d0cd579703
📒 Files selected for processing (9)
.github/instructions/ci.instructions.md.github/workflows/lint-powershell.yml.github/workflows/verify-extended.ymlBACKLOG.mdDESIGN.mdexamples/rollback/rollback.tsexamples/version-matrix/version-matrix.ps1src/lib/types.tstest/integration/examples-smoke.test.ts
Comment on lines
+16
to
+33
| on: | ||
| push: | ||
| branches: [main] | ||
| paths: | ||
| - "examples/**/*.ps1" | ||
| - "examples/PSScriptAnalyzerSettings.psd1" | ||
| - ".github/workflows/lint-powershell.yml" | ||
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - "examples/**/*.ps1" | ||
| - "examples/PSScriptAnalyzerSettings.psd1" | ||
| - ".github/workflows/lint-powershell.yml" | ||
| # Reusable: verify-extended.yml calls this so the dispatch path stays covered. | ||
| workflow_call: | ||
|
|
||
| permissions: | ||
| contents: read |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Cancel stale lint runs for the same ref.
Without a concurrency group, every push to the same PR can queue another full windows-latest lint run, so outdated results can outlive the newest commit. That cuts against the “fast PR feedback” goal of this workflow.
Suggested change
on:
push:
branches: [main]
paths:
- "examples/**/*.ps1"
- "examples/PSScriptAnalyzerSettings.psd1"
- ".github/workflows/lint-powershell.yml"
pull_request:
branches: [main]
paths:
- "examples/**/*.ps1"
- "examples/PSScriptAnalyzerSettings.psd1"
- ".github/workflows/lint-powershell.yml"
# Reusable: verify-extended.yml calls this so the dispatch path stays covered.
workflow_call:
+concurrency:
+ group: powershell-lint-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true
+
permissions:
contents: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| on: | |
| push: | |
| branches: [main] | |
| paths: | |
| - "examples/**/*.ps1" | |
| - "examples/PSScriptAnalyzerSettings.psd1" | |
| - ".github/workflows/lint-powershell.yml" | |
| pull_request: | |
| branches: [main] | |
| paths: | |
| - "examples/**/*.ps1" | |
| - "examples/PSScriptAnalyzerSettings.psd1" | |
| - ".github/workflows/lint-powershell.yml" | |
| # Reusable: verify-extended.yml calls this so the dispatch path stays covered. | |
| workflow_call: | |
| permissions: | |
| contents: read | |
| on: | |
| push: | |
| branches: [main] | |
| paths: | |
| - "examples/**/*.ps1" | |
| - "examples/PSScriptAnalyzerSettings.psd1" | |
| - ".github/workflows/lint-powershell.yml" | |
| pull_request: | |
| branches: [main] | |
| paths: | |
| - "examples/**/*.ps1" | |
| - "examples/PSScriptAnalyzerSettings.psd1" | |
| - ".github/workflows/lint-powershell.yml" | |
| # Reusable: verify-extended.yml calls this so the dispatch path stays covered. | |
| workflow_call: | |
| concurrency: | |
| group: powershell-lint-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }} | |
| cancel-in-progress: true | |
| permissions: | |
| contents: read |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 16-30: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 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 around lines 16 - 33, Add a
concurrency guard to the lint workflow so only the latest run for the same
branch or PR stays active; in the workflow defined by the on/pull_request and
workflow_call sections, introduce a concurrency group based on the ref (or PR
head) and enable cancelation of in-progress runs. Keep the change scoped to this
workflow so repeated pushes do not leave older windows-latest lint jobs running
after newer commits.
Source: Linters/SAST tools
| name: Lint PowerShell examples | ||
| runs-on: windows-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin actions/checkout to a full commit SHA.
actions/checkout@v5 is a mutable ref. If no later step needs git auth, set persist-credentials: false too.
🧰 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 workflow currently
uses a mutable actions reference for actions/checkout, so update the checkout
step to a full commit SHA and, if this job does not need git authentication
later, disable persisted credentials by setting persist-credentials to false.
Use the existing checkout step in the workflow to make the change.
Contributor
Author
|
On hold. The PowerShell parts (ASCII-clean fix + reusable |
mobileskyfi
added a commit
that referenced
this pull request
Jun 27, 2026
…e skipping (#33) Codifies the discipline that was missing when a single Extended Verification run got mistaken for a proven arm64 QEMU limitation and "fixed" by arch-gating the rollback example (held in #32, real grounding tracked in #31). The anti-masking rules already existed but were siloed in testing.instructions.md and framed only around timeout-bumps; the most dangerous masking move — skip / os-gate / arch-gate — was never named, and nothing required reproducing a claimed platform limitation locally (which we can: arm64 CHR runs under TCG on Intel). - examples.instructions.md: examples are canaries; a failing example is a quickchr bug until a LOCAL repro proves otherwise; never gate a failure as the first move; don't record an unproven cause in DESIGN/API docs/BACKLOG/issues/skills. - testing.instructions.md: name skip/gating as the worst masking move (worse than a timeout-bump); "platform limitation" is a hypothesis until reproduced locally; one CI failure must not cascade into doc/skill edits. - ci.instructions.md: one CI run is a signal, not a fact — reproduce locally, don't let one red run cascade. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Closing — split and superseded.
This closes the skip, not the question. The actual arm64 + qcow2 snapshot behavior stays open in #31 for a future session to reproduce locally under TCG and root-cause before anything is gated or documented (also tracked as a The branch |
mobileskyfi
added a commit
that referenced
this pull request
Jun 27, 2026
…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
added a commit
that referenced
this pull request
Jun 27, 2026
…ll lint workflow (#34) 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>
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.
Post-merge follow-up to #22. The Extended Verification run on
main(28281373854) surfaced two reds — both fixed here.1. PowerShell lint (underlying fix + #28)
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: extracted the PSScriptAnalyzer job into a dedicated, reusable
lint-powershell.ymlthat:examples/**/*.ps1(+ the analyzer settings) — so.ps1regressions surface in normal PR CI instead of hiding behind a manual Extended Verification dispatch;workflow_call, andverify-extended.ymlnowuses:it — one source of truth, dispatch path still covered.(This PR touches a
.ps1, so the new workflow runs on it — self-verifying.)2. arm64
rollbackexample smokeThe
rollbackexample failed on macos/arm64 + linux/aarch64 while passing on every x86 accelerator (KVM/HVF/WHPX):snapshot.save/snapshot.loadboth return clean — but the post-loadvmwaitForBoot(60_000)exhausts its full 60 s and REST never comes back.virtguest, not the accelerator and not a too-short timeout.This is the known QEMU limitation that internal
savevm/loadvmsnapshots don't restore a working aarch64 guest. Since it's not fixable in quickchr, the smoke matrix now gates the example toarch: ["x64"](newarchfield mirroring the existingosgate). Documented inDESIGN.md, the snapshot API docs (types.ts), and the example header. Real fix tracked in #31.Verification
bun run checkgreen (biome, tsc, markdownlint, cspell, validate-examples = 12 conform, shellcheck);actionlintclean on both workflows. The arm64 gate needs an Extended Verification dispatch on this branch to confirm green (the arm64 smoke only runs there).🤖 Generated with Claude Code
Summary by CodeRabbit