[skill] Generalize verification skill into 3 scenarios (module / release-PR / active-PR)#48848
Open
LegendaryBlair wants to merge 7 commits into
Open
Conversation
…ase-PR / active-PR) Implements the Option-3 "shared engine + per-scenario reference docs" design for the powertoys-module-verification skill: - SKILL.md: broaden description and "When to use" to cover all three scenarios, add a Step-0 scenario router, make the supplied-checklist read Scenario-A-only, and make placeholders/"What NOT to do" scenario-aware. - references/scenarios/index.md: router table, the per-scenario "bits under test" contract (installed-immutable for A/B vs build+sideload for C), and the verdict-vocabulary mapping between the engine and legacy B labels. - references/scenarios/module-checklist.md (A): supplied-checklist run order. - references/scenarios/release-pr-signoff.md (B): derive each PR's checklist from its description+diff against installed bits, plus a PR-discovery model with a size gate (auto-do <=25 hotfix-sized; scope-and-confirm for full ~100-PR releases) and a non-runtime-PR pre-filter. - references/scenarios/active-pr-validation.md (C): build+sideload front-end (delegate to PrepareWorktree), unpackaged-vs-packaged deploy recipes, inverted test discipline, and restore-to-shipped cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix six valid review comments left on the merged skill PR (verified against the helper scripts and PowerToys source): - power-rename.md: context-menu snippet used non-existent params (-FolderPath/-FileNames) and treated Get-PtContextMenuItems output as objects; rewrite to the canonical flow (-ExplorerHwnd/-FileName + string results). - explorer-context-menu-flow.md: New+ settings path New\settings.json -> NewPlus\settings.json; Image Resizer caption Resize images -> Resize with Image Resizer (source string ImageResizer_Context_Menu_Entry). - SKILL.md: File Locksmith JSON key ShowInExtendedContextMenu -> showInExtendedContextMenu (camelCase, per FileLocksmithLib Constants.h JsonKeyShowInExtendedContextMenu). - pt-session-diagnose.ps1: guard Add-Type so dot-sourcing/re-running in the same session no longer throws "type already exists". - environment-variables.md: typo "aplpied" -> "applied". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ed D:\fixtures The review-fix snippet carried over a hardcoded 'D:\fixtures' path from the original buggy example. Replace it with the doc's own disposable-temp-fixtures convention (\C:\Users\bozhang\AppData\Local\Temp\pr-fixture-<random>, same as Entry-path 1) and match the Explorer window by the actual fixture folder leaf name. Also de-hardcode the adjacent Entry-path 3 line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…areWorktree agent PrepareWorktree is a personal/environment custom agent, not shipped in microsoft/PowerToys, so it must not be the documented default. Make the repo-portable path primary (gh pr checkout + tools\build\New-WorktreeFromBranch.ps1 + the documented build commands, same helper FixIssue.agent.md uses) and demote PrepareWorktree to a clearly-labelled optional convenience. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the personal-agent mention (not shipped in this repo) rather than keeping it as optional; the in-repo worktree helper + documented build commands are the only path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n PR Add a 'PR state' row (B = merged & shipped; C = unmerged/open) and reword the C trigger + B-vs-C disambiguation so a bare 'validate PR #N' is correctly routed: open/unmerged => C (build+sideload), already-merged-into-the-build => one-PR B. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the existing powertoys-module-verification agent skill from “verify a single module checklist” into a shared verification engine that supports three scenarios: (A) module checklist sign-off, (B) release/hotfix PR sign-off against installed bits, and (C) active/unmerged PR validation via build + sideload.
Changes:
- Generalizes
SKILL.mdto route between scenarios A/B/C while keeping a single shared drive engine. - Adds scenario-specific playbooks under
references/scenarios/(router + per-scenario procedures). - Improves supporting docs/scripts (PowerShell Add-Type guard, context-menu caption updates, minor checklist typo fix).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/skills/powertoys-module-verification/SKILL.md | Broadens skill description and updates the “required reads” + scenario routing to support A/B/C. |
| .github/skills/powertoys-module-verification/scripts/pt-session-diagnose.ps1 | Guards Add-Type to allow dot-sourcing / re-running without “type already exists” failures. |
| .github/skills/powertoys-module-verification/references/scenarios/index.md | Adds a scenario router, bits-under-test contract, and verdict vocabulary mapping. |
| .github/skills/powertoys-module-verification/references/scenarios/module-checklist.md | Documents Scenario A (supplied checklist, installed immutable bits). |
| .github/skills/powertoys-module-verification/references/scenarios/release-pr-signoff.md | Documents Scenario B (derive checklist per PR; size gate; live-drive floor). |
| .github/skills/powertoys-module-verification/references/scenarios/active-pr-validation.md | Documents Scenario C (PR-driven checklist, build + deploy/sideload, cleanup). |
| .github/skills/powertoys-module-verification/references/release-checklist/environment-variables.md | Fixes a typo in a checklist item (“aplpied” → “applied”). |
| .github/skills/powertoys-module-verification/references/modules/power-rename.md | Updates the menu-presence recipe to use temp fixtures + current helper signatures. |
| .github/skills/powertoys-module-verification/references/explorer-context-menu-flow.md | Updates context-menu guidance (NewPlus path, Image Resizer caption/source string). |
| 1. **`references/scenarios/index.md`** — **read FIRST**: the scenario router (A/B/C), the | ||
| per-scenario **"bits under test" contract** (installed-and-immutable for A/B vs build-and-sideload | ||
| for C), and the verdict-vocabulary mapping. Then read the **one** matching scenario doc: | ||
| `references/scenarios/module-checklist.md` (A) · `release-pr-signoff.md` (B) · `active-pr-validation.md` (C). |
| per-scenario **"bits under test" contract** (installed-and-immutable for A/B vs build-and-sideload | ||
| for C), and the verdict-vocabulary mapping. Then read the **one** matching scenario doc: | ||
| `references/scenarios/module-checklist.md` (A) · `release-pr-signoff.md` (B) · `active-pr-validation.md` (C). | ||
| 2. **`references/winapp-ui-testing.md`** — the **prerequisite** UIA mechanics doc (winapp ui verbs, scripted batch testing, file pickers, accessibility audits, screenshots, click-vs-invoke, PostMessage, SendInput cb=40, stunted-UIA recovery, settings-mutation safety contract). **Read this first** — this skill assumes you know its content and only adds PT-specific extensions. |
Comment on lines
+45
to
+46
| # then build ONLY the affected project folder: | ||
| tools\build\build.ps1 -Platform x64 -Configuration Release # run from the changed .csproj/.vcxproj dir |
Comment on lines
+60
to
+62
| The shared engine in `SKILL.md` Step 3 uses **PASS / FAIL(product|checklist) / BLOCKED(reason)**. | ||
| Scenario B's legacy reports sometimes use **PASS / FAIL / Don't-know-what-to-test / Incapable**. | ||
| They map 1:1 — use the `SKILL.md` set and treat the legacy labels as aliases: |
| $hwnd = Open-PtExplorerContextMenu -FolderPath 'D:\fixtures' -FileNames 'a.txt' | ||
| $items = Get-PtContextMenuItems -MenuHwnd $hwnd | ||
| $has = $items | Where-Object Name -match 'Rename with PowerRename' | ||
| # Disposable fixtures folder (same convention as Entry-path 1) |
…ication The skill now covers three scenarios (module checklist, release/hotfix PR sign-off, active-PR validation), so the module-only name was misleading. Folder rename via git mv (34 files are pure path-renames); only SKILL.md (name frontmatter + example path + obsolete 'historical name' note) and winapp-ui-testing.md (provenance line) change content. No references to the old name exist outside the skill folder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.
Summary of the Pull Request
Generalizes the
powertoys-verificationagent skill from "one module checklist" into a single shared engine that serves three verification scenarios, using the shared engine + per-scenario reference docs design (no duplication of the winapp mechanics):The three scenarios share the same
winapp uidrive techniques, helper scripts, per-module profiles, taxonomy, and report format. They differ only on four axes (checklist source, bits under test, discipline, scope), so the only new content is a thin scenario layer.PR Checklist
powertoys-verificationskill)Detailed Description of the Pull Request / Additional comments
SKILL.md(shared engine) — broadendescription+ "When to use" to cover all three scenarios, add a Step-0 scenario router, make the supplied-checklist read Scenario-A-only, and make the bootstrap/placeholders/"What NOT to do" scenario-aware.references/scenarios/(new):index.md— router table; the per-scenario "bits under test" contract (the one real conflict: installed-and-immutable for A/B vs build-and-sideload for C, echoed in the report header so the evidence chain is trustworthy); and a verdict-vocabulary mapping (enginePASS/FAIL/BLOCKED↔ legacy B labels).module-checklist.md(A) — supplied checklist, installed bits.release-pr-signoff.md(B) — derive each PR's checklist fromgh pr view/diff; per-PR folders; a PR-discovery model with a size gate (auto-verify hotfix-sized sets ≤25; for full ~100-PR releases, scope-and-confirm instead of blind-looping) and a non-runtime-PR pre-filter.active-pr-validation.md(C) — build+sideload front-end (in-repo worktree helper + documented build commands), unpackaged (run the built runner) vs packaged/CmdPal (Add-AppxPackage -Register) deploy recipes, the inverted test discipline, and restore-to-shipped cleanup.The existing engine docs (
winapp-ui-testing.md,pre-flight.md,reporting-format.md), per-module profiles, and helper scripts are reused unchanged — no duplication, single source of truth.Validation Steps Performed
SKILL.mdwithin authoring limits: 335 lines (< 500) and description 943 chars (< 1024).references/scenarios/*.mdresolve, and every engine doc they cite exists.