From 378b65a7f2d44dd95f52ea8d7db1c7bacb9285b6 Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:03:32 -0700
Subject: [PATCH 1/7] Improve pr-review skill: real validation,
 necessity/simplicity, and independent multi-model
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Evidence-driven improvements grounded in 5 real past pr-review sessions:

- New 'Validate findings for real' phase: build, install the CLI as a user
  would (npm pack/install, MSIX, global tool — not dotnet run), scaffold a
  throwaway app, exercise each changed command, and confirm/drop every
  critical/high finding with runtime evidence. Replaces the old
  'No build/test execution' rule. Findings are tagged validated vs static-only.
- New necessity-and-simplicity dimension ('can merge' != 'should merge';
  earns-its-complexity; fills-a-need; smaller/staged/in-scope) registered as
  dimension #5.
- Strengthened alternative-solution: require >=1 concrete named alternative
  (in-repo helper or ecosystem tool) with a tradeoff, backed by a quick search;
  softened the guidance that suppressed necessity critiques.
- Reworked multi-model: independent pass from raw diff + code (not the
  consolidated findings), record the model family that ran, 2-3 families for
  high-risk PRs.
- Added regression analysis (correctness) and a threat-model checklist +
  red-team (security); tag static-only findings 'needs runtime confirmation'.
- Added multi-PR/multi-target mode and sub-agent fault-tolerance (from the real
  'Policy hook failed' incident).
- Added opt-in follow-through (AI-labelled PR comment, test-setup handoff,
  follow-up fix PR only on request); default stays stdout-only/no-fix.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .github/skills/pr-review/SKILL.md             | 239 ++++++++++++++----
 .../pr-review/dimensions/_shared-contract.md  |   9 +-
 .../dimensions/alternative-solution.md        |  38 ++-
 .../pr-review/dimensions/correctness.md       |  23 ++
 .../pr-review/dimensions/multi-model.md       |  76 ++++--
 .../dimensions/necessity-and-simplicity.md    |  67 +++++
 .../skills/pr-review/dimensions/security.md   |  26 ++
 7 files changed, 389 insertions(+), 89 deletions(-)
 create mode 100644 .github/skills/pr-review/dimensions/necessity-and-simplicity.md

diff --git a/.github/skills/pr-review/SKILL.md b/.github/skills/pr-review/SKILL.md
index c3be39e0..434ed526 100644
--- a/.github/skills/pr-review/SKILL.md
+++ b/.github/skills/pr-review/SKILL.md
@@ -1,13 +1,14 @@
 ---
 name: pr-review
-description: Multi-dimensional review of a PR or feature branch in the microsoft/winappcli repo. Activate when a contributor asks to "review my PR", "review my changes", "vet my branch before pushing", "do a full review", "PR review", "review this feature", or similar. Fans out parallel sub-agents covering security, correctness/edge cases, CLI UX, alternative-solution check, test coverage, docs & samples sync, packaging/release impact, and a multi-model cross-check. Reports a consolidated finding list to stdout. Does NOT apply fixes.
+description: Multi-dimensional review of a PR or feature branch in the microsoft/winappcli repo. Activate when a contributor asks to "review my PR", "review my changes", "vet my branch before pushing", "do a full review", "PR review", "review this feature", or similar. Fans out parallel sub-agents covering security, correctness/edge cases, CLI UX, alternative-solution check, necessity & simplicity, test coverage, docs & samples sync, packaging/release impact, and an independent multi-model cross-check, then validates high/critical findings by building and running the CLI the way a user would. Reports a consolidated finding list to stdout. Does NOT apply fixes unless explicitly asked.
 infer: true
 ---
 
 You are the **PR Review orchestrator** for the `microsoft/winappcli` repo.
 Your job is to give a contributor a thorough, high-signal review of their
-in-progress branch before they push, by fanning out parallel sub-agents and
-consolidating their findings.
+in-progress branch before they push, by fanning out parallel sub-agents,
+validating the high-risk findings against a real build, and consolidating the
+results.
 
 ## When to activate
 
@@ -105,8 +106,8 @@ buckets in this repo:
 
 | Path prefix | Likely owner |
 |-------------|--------------|
-| `src/winapp-CLI/WinApp.Cli/Commands/` | CLI UX, correctness, security |
-| `src/winapp-CLI/WinApp.Cli/Services/` | correctness, alternative-solution, security |
+| `src/winapp-CLI/WinApp.Cli/Commands/` | CLI UX, correctness, security, necessity-and-simplicity |
+| `src/winapp-CLI/WinApp.Cli/Services/` | correctness, alternative-solution, security, necessity-and-simplicity |
 | `src/winapp-CLI/WinApp.Cli.Tests/` | test-coverage |
 | `src/winapp-npm/` | packaging, CLI UX (npm wrapper) |
 | `src/winapp-NuGet/` | packaging |
@@ -118,29 +119,44 @@ buckets in this repo:
 
 ### 4. Fan out parallel sub-agents
 
-Launch all 8 dimension sub-agents in **the same response** using the `task`
-tool, mode `"sync"`, agent type `general-purpose` (or `explore` for read-only
-dimensions — see per-dimension files). Each prompt must be self-contained:
-include the diff, the base/head refs, the file classification, and the contents
-of the corresponding `dimensions/<name>.md` file as instructions.
+Launch all 8 specialist dimension sub-agents (#1–#8) in **the same response**
+using the `task` tool, mode `"sync"`, agent type `general-purpose` (or `explore`
+for read-only dimensions — see per-dimension files). Each prompt must be
+self-contained: include the diff, the base/head refs, the file classification,
+and the contents of the corresponding `dimensions/<name>.md` file as
+instructions.
 
-The 8 dimensions and their fragment files:
+The 9 dimensions and their fragment files:
 
 | # | Dimension | Fragment | Default agent |
 |---|-----------|----------|---------------|
 | 1 | security | `dimensions/security.md` | general-purpose |
-| 2 | correctness & edge cases | `dimensions/correctness.md` | general-purpose |
+| 2 | correctness & edge cases (incl. regression) | `dimensions/correctness.md` | general-purpose |
 | 3 | CLI UX & usability | `dimensions/cli-ux.md` | general-purpose |
 | 4 | alternative-solution check | `dimensions/alternative-solution.md` | general-purpose |
-| 5 | test coverage | `dimensions/test-coverage.md` | general-purpose |
-| 6 | docs & samples sync | `dimensions/docs-and-samples.md` | explore |
-| 7 | packaging & release impact | `dimensions/packaging.md` | general-purpose |
-| 8 | multi-model cross-check | `dimensions/multi-model.md` | general-purpose, with `model` override |
-
-For #8 (multi-model), wait until #1–#7 finish first, then pass that sub-agent
-the consolidated critical/high findings and require it to use a **different
-model family** than the orchestrator (e.g. if you are a Claude model, override
-to `gpt-5.4`; if you are GPT, override to `claude-opus-4.7`).
+| 5 | necessity & simplicity | `dimensions/necessity-and-simplicity.md` | general-purpose |
+| 6 | test coverage | `dimensions/test-coverage.md` | general-purpose |
+| 7 | docs & samples sync | `dimensions/docs-and-samples.md` | explore |
+| 8 | packaging & release impact | `dimensions/packaging.md` | general-purpose |
+| 9 | multi-model cross-check | `dimensions/multi-model.md` | general-purpose, with `model` override |
+
+For #9 (multi-model), wait until #1–#8 finish first. Pass it the **raw diff plus
+the actual changed code files** — **not** the consolidated findings, which anchor
+it into a rubber-stamp — and require an **independent pass** first; the
+specialists' critical/high list is optional reconciliation input it reads only
+afterward. Require it to (a) use a **different model family** than the
+orchestrator (if you are Claude, override to `gpt-5.4`; if GPT, to
+`claude-opus-4.7`) and (b) record which model family actually ran. For high-risk
+PRs, run it 2–3 times across different families (opus / gemini / gpt), each
+independent.
+
+**Sub-agent fault-tolerance.** Tell every sub-agent that if a tool call is
+blocked by a policy hook or otherwise denied, it must **continue with the tools
+it has and still return findings**, never abort silently. After the fan-out, if
+a sub-agent returned nothing, errored, or died (e.g. a "Policy hook failed"
+abort), **detect it and re-run that one dimension once, hardened** (drop the
+blocked tool, keep the analysis). Mark a dimension `✗ skipped + reason` only if
+the retry also fails.
 
 ### 5. Consolidate
 
@@ -153,13 +169,61 @@ Collect all findings. Then:
 2. **Assign IDs.** `C1, C2, ...` for critical, `H1, H2, ...` for high,
    `M1, ...` for medium, `L1, ...` for low.
 3. **Sort.** critical → high → medium → low; within severity, sort by file path.
-4. **Note multi-model status.** For each critical/high finding, mark it as
-   `confirmed`, `disputed`, or `not reviewed` based on the multi-model output.
-
-### 6. Report to stdout
-
-Print exactly the format below. **Do not** save to a file unless the user
-explicitly asks. **Do not** apply fixes — your job ends at reporting.
+4. **Reconcile the multi-model pass.** For each critical/high finding, compare
+   it against the independent multi-model output and mark it `confirmed`,
+   `disputed`, `downgrade`, `upgrade`, or `not reviewed`. Record which model
+   family ran.
+5. **Seed validation status.** Every finding enters this step as
+   `static-only (needs runtime confirmation)`. The Validate phase (next) is what
+   promotes critical/high findings to `validated` or drops them.
+
+### 6. Validate findings for real
+
+Static review misses what only shows up at runtime (empty output files,
+physical- vs logical-pixel bugs, dead gates that never fired, cold-cache version
+drift). Before reporting, **try to confirm or drop every critical/high finding
+with runtime evidence.** This phase deliberately replaces the old "no build/test
+execution" rule.
+
+Do as much of this as the environment allows, and **record what you could not
+do** rather than silently skipping it:
+
+1. **Build the branch** (`scripts/build-cli.ps1`, or a targeted `dotnet build`).
+   A build failure is itself a critical finding.
+2. **Install the CLI as a *user* would — not dev mode.** Do **not** validate via
+   `dotnet run` or a Debug worktree; that hides the cold-cache and packaging
+   bugs real users hit. Pick one:
+   - `npm pack` in `src/winapp-npm`, then `npm i -g` the tarball and run
+     `winapp …`;
+   - the built **MSIX** / installed global tool;
+   - the published single-file binary on a clean PATH.
+   Prefer a **fresh / cold cache** so first-run download and version-drift bugs
+   surface.
+3. **Scaffold a throwaway app** in a temp dir to exercise the changed commands
+   against something real (`winapp init`, package, sign, run). For WinUI /
+   desktop apps you may delegate the scaffold + build to the `winui-dev` agent.
+4. **Exercise each changed command** end-to-end. For UI-automation changes,
+   actually drive a window and read back real events/values — do not trust test
+   fakes or slug-format selectors, which mask real behavior.
+5. **Red-team the top security finding** using the concrete attempt the security
+   sub-agent proposed (input-injection / process-invocation / signing).
+6. **Resolve each critical/high finding:**
+   - **validated** — you reproduced the problem (or confirmed a fix): set
+     `Validation: validated` and add the runtime evidence to its Evidence.
+   - **dropped** — the runtime check refutes it: remove it and say why in
+     `Coverage notes`.
+   - **static-only** — you could not run it (missing creds, hardware, admin,
+     sample app): keep `Validation: static-only (needs runtime confirmation)`
+     and state exactly what was needed. This feeds the opt-in test-setup handoff.
+
+Never mark a finding `validated` without real evidence — false confidence is
+worse than an honest "static-only, needs runtime confirmation."
+
+### 7. Report to stdout
+
+Print exactly the format below. **Do not** save to a file or post a PR comment
+unless the user explicitly asks (see Opt-in follow-through). **Do not** apply
+fixes by default — reporting is where the default run ends.
 
 The header line varies by scope:
 
@@ -179,10 +243,13 @@ Coverage
   correctness           ...
   cli-ux                ...
   alternative-solution  ...
+  necessity-simplicity  ...
   test-coverage         ...
   docs-and-samples      ...
   packaging             ...
-  multi-model           <✓ X/Y critical+high confirmed>
+  multi-model           <✓ X/Y critical+high confirmed  ·  models: opus,gpt>
+  regression            <✓ no change to existing flows | ⚠ N regressions>
+  validation            <✓ K/L critical+high validated at runtime | ⚠ static-only: ...>
 
 Findings
   C1  <file>:<lines>   <domain>      <one-line>
@@ -194,10 +261,11 @@ Details
 ## C1  <file>:<lines>
 - Severity: critical
 - Confidence: high
+- Validation: validated | static-only (needs runtime confirmation)
 - Domain: security
 - Multi-model: confirmed
 - Finding: <one-line>
-- Evidence: <code refs and quoted lines>
+- Evidence: <code refs and quoted lines; runtime evidence if validated>
 - Recommendation: <concrete next step>
 
 ## C2 ...
@@ -207,20 +275,47 @@ If a sub-agent returned zero findings, list its dimension as `✓ clean` in the
 Coverage block and include its short "what I checked" note in a final
 `Coverage notes` section so the user can see scope, not just verdict.
 
+## Multi-PR / multi-target mode
+
+When the user asks to review **several PRs or targets in one run** ("review
+#583, #573 and #587", "review all the UI-input PRs"), don't make them re-ask per
+PR:
+
+1. Enumerate the targets (PR numbers, branches, or refs).
+2. For **each** target, run the full workflow (scope → fan-out → validate →
+   consolidate) inside its **own trackable sub-agent**, launched in parallel and
+   named per target (e.g. `review-pr-583`) so progress is visible and a stall is
+   obvious.
+3. Apply the same fault-tolerance rule: if a per-PR sub-agent dies, retry it
+   once; if it still fails, report that target as `✗ skipped + reason` rather
+   than dropping it.
+4. Print **one report per target**, then a short **cross-PR roll-up**: shared
+   findings, ordering / stacking risks, and — when the user is deciding what to
+   ship — a per-PR "ship / stage / hold" line drawn from the
+   necessity-and-simplicity dimension.
+
 ## Rules the orchestrator must enforce
 
-- **Parallelism in one turn.** Fan out all of #1–#7 in a single response.
-- **No fix application.** Even if findings are obvious, do not edit code.
-- **No file output.** Stdout only, unless the user explicitly asked for a file.
-- **No build/test execution.** Flag staleness (e.g. `cli-schema.json` not
-  regenerated, npm `winapp-commands.ts` out of sync) but do not run
-  `scripts/build-cli.ps1` or test suites yourself — they are slow and the
-  contributor will run them.
+- **Parallelism in one turn.** Fan out all of #1–#8 in a single response; run
+  #9 (multi-model) after they return.
+- **Validate high/critical for real.** Build and run the branch to confirm or
+  drop critical/high findings (see the Validate phase). This **replaces** the
+  old "no build/test execution" rule. Still flag staleness you can see
+  statically (e.g. `cli-schema.json` not regenerated, npm `winapp-commands.ts`
+  out of sync). Mark anything you could not run
+  `static-only (needs runtime confirmation)` — never fake a runtime result.
+- **No fixes by default.** Do not edit code unless the user explicitly asks for
+  follow-through (see Opt-in follow-through). Reporting is the default endpoint.
+- **Stdout by default.** No file output and no posted PR comment unless the user
+  explicitly asks.
+- **Fault-tolerant fan-out.** A sub-agent whose tool call is blocked must keep
+  going and still return findings; the orchestrator must detect a dead or
+  aborted sub-agent and re-run that one dimension once, hardened.
 - **Signal-to-noise.** Reject sub-agent findings that are pure style nits,
   formatting, or things the compiler / analyzers / linter already catch. The
   Team Lead Test (see any dimension file) is mandatory.
-- **Cite evidence.** Every kept finding must reference a specific file and
-  line range visible in the diff.
+- **Cite evidence.** Every kept finding must reference a specific file and line
+  range visible in the diff (plus runtime evidence once validated).
 
 ## Sub-agent prompt template
 
@@ -240,9 +335,12 @@ from these blocks (in order):
 6. **Closing instruction.** "Return only the markdown specified by the shared
    contract. No preamble, no apologies, no narration."
 
-For the multi-model sub-agent, additionally pass the consolidated
-critical/high findings from the other 7 sub-agents, and set the `model`
-parameter on the `task` call to a different model family than yourself.
+For the multi-model sub-agent, pass the **raw diff and the actual changed code
+files** (not the consolidated findings) so it does a genuinely independent pass,
+set the `model` parameter on the `task` call to a different model family than
+yourself, and require it to record which family ran. You may pass the
+specialists' critical/high findings as *optional* reconciliation input it reads
+only after its own pass.
 
 ## Example invocation pattern
 
@@ -250,10 +348,11 @@ parameter on the `task` call to a different model family than yourself.
 1. git diff --stat origin/main...HEAD          → 12 files, +340/-87
 2. git diff origin/main...HEAD                 → captured for sub-agents
 3. Map files to areas                          → mostly Services + Commands + 1 doc
-4. Fan out 7 task() calls in parallel          → wait for all
-5. Fan out task() #8 with model override       → wait
-6. Dedupe, sort, ID, mark multi-model status
-7. Print stdout report
+4. Fan out 8 task() calls in parallel          → wait; retry any that died
+5. Fan out task() #9 (diff+code, model override) → wait, record model family
+6. Dedupe, sort, ID, reconcile multi-model, seed validation
+7. Validate: build + install as a user + run changed cmds → confirm/drop crit+high
+8. Print stdout report   (opt-in follow-through only if asked)
 ```
 
 ## Example consolidated stdout
@@ -262,37 +361,50 @@ parameter on the `task` call to a different model family than yourself.
 PR Review — feat/sparse-trustedlaunch vs origin/main  (4 commits, 9 files, +312/-58)
 
 Summary
-  Critical: 0   High: 2   Medium: 3   Low: 1
+  Critical: 0   High: 2   Medium: 4   Low: 1
 
 Coverage
   security              ⚠ 1 finding
   correctness           ⚠ 1 finding
   cli-ux                ⚠ 1 finding
   alternative-solution  ✓ clean
+  necessity-simplicity  ⚠ 1 finding
   test-coverage         ⚠ 1 finding
   docs-and-samples      ⚠ 2 findings
   packaging             ✓ clean
-  multi-model           ✓ 2/2 high confirmed
+  multi-model           ✓ 2/2 high confirmed  ·  models: gpt
+  regression            ✓ no change to existing flows
+  validation            ⚠ 1/2 high validated at runtime; H2 static-only (no signing cert)
 
 Findings
   H1  src/winapp-CLI/.../SparsePackageService.cs:142-160   security        Process.Start with manifest-derived path
   H2  src/winapp-CLI/.../Commands/CreateExternalCatalogCommand.cs:34-49  test-coverage   New command has no unit tests
   M1  src/winapp-CLI/.../SparsePackageService.cs:88-95     correctness     Manifest auto-detect skips Package.appxmanifest
   M2  src/winapp-CLI/.../Commands/CreateExternalCatalogCommand.cs:18-25  cli-ux          --catalog-name has no default; breaks scripted use
-  M3  docs/usage.md (missing)                               docs-and-samples  New command not documented
+  M3  src/winapp-CLI/.../Commands/CreateExternalCatalogCommand.cs:1-60   necessity-simplicity  New top-level verb overlaps `package`; could be a `--external-catalog` flag
+  M4  docs/usage.md (missing)                               docs-and-samples  New command not documented
   L1  .github/plugin/agents/winapp.agent.md:189            docs-and-samples  Command list out of order
 
 Details
 ## H1  src/winapp-CLI/WinApp.Cli/Services/SparsePackageService.cs:142-160
 - Severity: high
 - Confidence: high
+- Validation: validated
 - Domain: security
 - Multi-model: confirmed
 - Finding: Process.Start invokes makeappx.exe with a path read from the input manifest without validation.
-- Evidence: Line 148 builds args via $"makeappx.exe pack /d {manifestPath} ..." where manifestPath comes from XElement.Attribute("Source").Value at line 131. A crafted manifest could inject an additional makeappx flag or shell metacharacters via an unquoted path with spaces.
+- Evidence: Line 148 builds args via $"makeappx.exe pack /d {manifestPath} ..." where manifestPath comes from XElement.Attribute("Source").Value at line 131. Validate-phase red-team on the npm-installed CLI (cold cache): a manifest with Source='a b" /p x' made the extra /p flag reach makeappx.
 - Recommendation: Validate manifestPath is a rooted, existing file path under the project root, and pass it via ProcessStartInfo.ArgumentList rather than concatenating into Arguments.
 
-## H2 ...
+## H2  src/winapp-CLI/WinApp.Cli/Commands/CreateExternalCatalogCommand.cs:34-49
+- Severity: high
+- Confidence: high
+- Validation: static-only (needs runtime confirmation)
+- Domain: test-coverage
+- Multi-model: confirmed
+- Finding: New create-external-catalog command has no unit tests.
+- Evidence: Command added at lines 34-49; no matching test in WinApp.Cli.Tests. Could not exercise end-to-end — needs a signing cert to run against a scaffolded package (see Test-setup handoff).
+- Recommendation: Add a happy-path + error-path unit test; supply a dev cert to enable a runtime smoke test.
 
 Coverage notes
   alternative-solution: Inspected new sparse-package code paths against
@@ -300,8 +412,27 @@ Coverage notes
   packaging: Inspected version.json, npm wrapper, NuGet targets — no impact.
 ```
 
+## Opt-in follow-through
+
+Default behavior is unchanged: **stdout only, no fixes, no posting.** Only when
+the user explicitly asks, offer these follow-ups:
+
+- **Post an AI-labelled PR review comment.** On request, post the consolidated
+  findings as a PR comment that **opens with a clear banner**, e.g.
+  `> 🤖 AI-generated review (winappcli pr-review skill) — verify before acting.`
+  Never post silently and never drop the banner.
+- **Emit a test-setup / prerequisites handoff.** Turn every `static-only`
+  finding into a checklist of what a human needs to validate it for real —
+  signing certs, cloud / Azure credentials, specific hardware, a sample app,
+  admin rights. This is the honest handoff for anything the Validate phase could
+  not reach ("here's what I'd need to test it for real").
+- **Open a follow-up fix PR — only on explicit request.** If (and only if) the
+  user asks you to fix the findings, make the changes on a **new** branch and
+  open a separate PR; never push fixes onto the branch under review.
+
 ## Output discipline
 
-The final stdout block is the *only* user-visible output. Do not narrate the
-process, do not summarize what each sub-agent did, do not apologize for noise.
-The Coverage table already conveys what ran.
+The final stdout report is the primary user-visible output — and, unless the
+user opted into follow-through, the *only* one. Do not narrate the process, do
+not summarize what each sub-agent did, do not apologize for noise. The Coverage
+table already conveys what ran.
diff --git a/.github/skills/pr-review/dimensions/_shared-contract.md b/.github/skills/pr-review/dimensions/_shared-contract.md
index 88b11d10..6e730bcd 100644
--- a/.github/skills/pr-review/dimensions/_shared-contract.md
+++ b/.github/skills/pr-review/dimensions/_shared-contract.md
@@ -11,8 +11,8 @@ Start with exactly one line:
 ```
 
 Where `<dimension name>` is one of: `security`, `correctness`, `cli-ux`,
-`alternative-solution`, `test-coverage`, `docs-and-samples`, `packaging`,
-`multi-model`.
+`alternative-solution`, `necessity-and-simplicity`, `test-coverage`,
+`docs-and-samples`, `packaging`, `multi-model`.
 
 ## Per-finding block
 
@@ -22,6 +22,7 @@ Each finding is a level-2 heading followed by labeled bullets:
 ## <relative file path>:<start_line>-<end_line>
 - **Severity**: critical | high | medium | low
 - **Confidence**: high | medium | low
+- **Validation**: static-only (needs runtime confirmation) | validated
 - **Domain**: <dimension name>
 - **Finding**: <one-line statement of what is wrong>
 - **Evidence**: <specific code evidence — quote 1-3 lines, cite line refs in the diff>
@@ -35,6 +36,10 @@ Notes:
   For `working` / `staged` / `all` scopes this means the working-tree or staged
   state, not a committed version.
 - For findings that span discontiguous regions, emit them as separate findings.
+- **Validation** starts as `static-only (needs runtime confirmation)` for every
+  finding you emit — you are reading the diff, not running it. The orchestrator
+  flips it to `validated` in the Validate phase when a runtime check confirms
+  the finding, and drops the finding if a runtime check refutes it.
 
 ## Trailing "what I checked" note
 
diff --git a/.github/skills/pr-review/dimensions/alternative-solution.md b/.github/skills/pr-review/dimensions/alternative-solution.md
index ded6fdcf..5fab2ab6 100644
--- a/.github/skills/pr-review/dimensions/alternative-solution.md
+++ b/.github/skills/pr-review/dimensions/alternative-solution.md
@@ -1,10 +1,36 @@
 # Alternative-solution review
 
 You are reviewing a PR diff for the `microsoft/winappcli` repo and asking:
-**is there a simpler, more idiomatic, or already-existing way to do this in
-this codebase?** Apply the shared output contract in `_shared-contract.md`.
+**is there a simpler, more idiomatic, or already-existing way to do this —
+either in this codebase or in the surrounding ecosystem (a standard tool,
+pattern, or API)?** Apply the shared output contract in `_shared-contract.md`.
 Set `Domain: alternative-solution` on every finding.
 
+(Scope, necessity, and "should this ship at all" belong to the
+`necessity-and-simplicity` dimension — stay focused on *how* the work is done.
+But do not self-censor a genuine "there's a better approach" critique just
+because it borders on scope; raise the concrete alternative here and let that
+dimension own the necessity framing.)
+
+## Required output: name a concrete alternative
+
+Do not stop at "uses `AppxManifestDocument` correctly." Every run must produce
+**at least one concrete, named alternative** — an in-repo helper/service/pattern
+*or* an ecosystem tool/library/API — with a one-line tradeoff, backed by a quick
+search (grep the repo for the helper, or draw on ecosystem knowledge). Examples
+of the bar to clear:
+
+- "Extend the existing `wait-for` command instead of adding a new streaming
+  watch — reuses its polling loop, one fewer command to maintain."
+- "Resolve selectors through `SelectorService` rather than re-parsing slugs — a
+  grep shows it already handles the ambiguous-match case."
+- "For the accessibility pass, lean on Accessibility Insights / Axe and frame it
+  as a quick lint, not a WCAG audit."
+
+If after searching you genuinely find no better alternative, say so in
+`## What I checked` **and name what you searched for**, so the sign-off is
+verifiable. A silent "no alternatives" is not acceptable.
+
 ## Repo-specific patterns to enforce
 
 - **Manifest reading/writing → use `AppxManifestDocument`.** New code that
@@ -52,9 +78,11 @@ Set `Domain: alternative-solution` on every finding.
 ## What to drop
 
 - Generic "this could be more functional" / "consider LINQ" without a
-  concrete callable alternative in the repo.
-- Refactor suggestions that exceed the scope of the PR ("rewrite this whole
-  service") — note them only as `low` with a tight recommendation, or skip.
+  concrete callable alternative.
+- A wholesale "rewrite this entire service" with no incremental path — offer the
+  smallest concrete reuse instead. Do **not** drop a critique just because it
+  touches scope or necessity; hand that framing to the
+  `necessity-and-simplicity` dimension and keep your concrete alternative here.
 
 ## Severity guide for this dimension
 
diff --git a/.github/skills/pr-review/dimensions/correctness.md b/.github/skills/pr-review/dimensions/correctness.md
index 211ea092..773d647c 100644
--- a/.github/skills/pr-review/dimensions/correctness.md
+++ b/.github/skills/pr-review/dimensions/correctness.md
@@ -47,6 +47,28 @@ You are a correctness specialist reviewing a PR diff for the
   COM objects, or registers temporary appx packages without cleaning up on
   failure.
 
+## Regression analysis (required)
+
+Beyond new-code bugs, explicitly ask **what changes for existing users and
+existing flows.** For every behavior an existing user relies on that this diff
+touches:
+
+- State the before → after change concretely.
+- Classify it as one of:
+  - **defect** — an unintended change that breaks or degrades a working flow.
+  - **test-gap** — behavior only *looks* changed because it was never tested;
+    the diff exposes it rather than causing it.
+  - **intended change** — a deliberate behavior change (still a docs finding if
+    docs/samples don't reflect it).
+- A gate/check that was **previously dead or never firing** and now fires is a
+  behavior change: say whether it will start failing builds/flows that used to
+  pass. Whether that is a defect or a long-overdue fix usually can only be
+  answered at runtime — emit it and let the Validate phase settle it.
+
+Emit regression concerns as normal findings with `Domain: correctness`, lead the
+Finding line with `Regression:`, and keep them
+`Validation: static-only (needs runtime confirmation)` until the Validate phase.
+
 ## What to drop
 
 - "Consider extracting to a method." (Style.)
@@ -56,6 +78,7 @@ You are a correctness specialist reviewing a PR diff for the
 ## Severity guide for this dimension
 
 - A guaranteed crash on a realistic input → high.
+- A regression that breaks an existing user flow → high.
 - A latent bug that requires unusual inputs to trigger → medium.
 - A defensive improvement with no concrete failure mode → low (and only emit
   if the recommendation is specific).
diff --git a/.github/skills/pr-review/dimensions/multi-model.md b/.github/skills/pr-review/dimensions/multi-model.md
index 3e1577ee..eb6d3547 100644
--- a/.github/skills/pr-review/dimensions/multi-model.md
+++ b/.github/skills/pr-review/dimensions/multi-model.md
@@ -1,48 +1,66 @@
 # Multi-model cross-check
 
 You are the **multi-model cross-check** sub-agent for the PR review skill.
-Your purpose is to catch model-specific blind spots: a finding that one model
-family confidently asserts may be a hallucination, and a real issue that one
-model overlooks may be obvious to another.
+Your purpose is to catch model-specific blind spots by doing a genuinely
+**independent** review with a different model family — not to rubber-stamp the
+specialists. A finding one family confidently asserts may be a hallucination; a
+real issue one family overlooks may be obvious to another. That only works if
+you form your own view **before** you look at anyone else's.
 
-You **must** be invoked with a `model` override that selects a different
-model family than the orchestrator. The orchestrator should set this
-explicitly (e.g., a Claude orchestrator passes `model: "gpt-5.4"`; a GPT
-orchestrator passes `model: "claude-opus-4.7"`).
+You **must** be invoked with a `model` override selecting a different model
+family than the orchestrator (e.g., a Claude orchestrator passes
+`model: "gpt-5.4"`; a GPT orchestrator passes `model: "claude-opus-4.7"`). For
+high-risk PRs the orchestrator may run you 2–3 times across different families
+(opus / gemini / gpt); each run is independent.
+
+## Record which model actually ran (required)
+
+Your **first output line** must be exactly:
+
+```
+Model family: <opus | gemini | gpt | other> (<model id if known>)
+```
+
+This is mandatory — past runs never recorded which model executed, so there was
+no proof the cross-check used a different family. If you cannot determine your
+own family, write `Model family: unknown` and explain why.
 
 ## Input
 
 The orchestrator passes you:
 
 1. The unified diff (`git diff <base>...HEAD`).
-2. The repo file map / area classification.
-3. The consolidated **critical and high** findings from the 7 specialist
-   sub-agents, each with its full Evidence and Recommendation.
+2. The **actual changed code files** (full context, not just the diff hunks) and
+   the repo file map / area classification.
+3. *(Optional, for reconciliation only)* the specialists' consolidated
+   critical/high findings — **do not read these until after your own pass.**
 
 ## What you do
 
-For each critical/high finding, independently verify:
+**Step 1 — independent pass (primary).** Working only from the diff and the real
+code files, do your own research and form your own list of critical/high issues.
+Re-trace input → sink paths yourself, read the surrounding code, and search the
+repo where needed. Do not anchor on the specialists' conclusions — the point is
+an independent second opinion, not agreement.
 
-1. **Does the cited code actually exist in the diff?** Reject hallucinated
-   line references.
-2. **Is the cause-and-effect chain real?** Re-trace the input → sink path
-   yourself.
-3. **Is the severity reasonable?** If you would set it lower, say so and
-   why.
-4. **Is the recommendation sound?** Flag fixes that would introduce new
-   bugs (e.g., "wrap in try/catch" suggestions that swallow errors).
+**Step 2 — reconcile (secondary).** *Only now* compare your independent list
+against the specialists' critical/high findings, if they were provided. For each
+of theirs, verify: does the cited code exist? Is the cause-and-effect chain
+real? Is the severity reasonable? Is the recommendation sound, or would it
+introduce a new bug (e.g., a "wrap in try/catch" that swallows errors)? Emit a
+verdict for each.
 
-Then, **independently scan the diff** for any critical/high issue the
-specialists missed. Be parsimonious: only emit findings that meet the bar
-for critical or high — not medium/low. The other sub-agents have already
-covered that ground.
+Be parsimonious about *new* findings: only emit critical or high — the other
+sub-agents already cover medium/low.
 
 ## Output contract
 
-Apply `_shared-contract.md`. Set `Domain: multi-model` on every finding.
+Apply `_shared-contract.md`. Set `Domain: multi-model` on every finding, and
+start with the `Model family:` line described above.
 
-In addition to the standard finding format, **for each input finding** emit
-one of:
+If the orchestrator gave you the specialists' critical/high findings, then
+**after your independent pass** emit one reconciliation block per input
+finding:
 
 ```markdown
 ## Cross-check: <original finding ID or file:lines>
@@ -63,8 +81,10 @@ one of:
 - **upgrade** — the issue is real and larger than claimed (rare; only when
   the original missed a worse downstream effect).
 
-After cross-checking each input finding, list any **new** critical/high
-findings you discovered as standard finding blocks (`## file:lines` etc.).
+After your independent pass (and the reconciliation blocks, if input findings
+were provided), list every critical/high issue you found as standard finding
+blocks (`## file:lines` etc.) — including ones the specialists also raised, so
+your independent list stands on its own.
 
 ## Discipline
 
diff --git a/.github/skills/pr-review/dimensions/necessity-and-simplicity.md b/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
new file mode 100644
index 00000000..11488577
--- /dev/null
+++ b/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
@@ -0,0 +1,67 @@
+# Necessity & simplicity review
+
+You are reviewing a PR diff for the `microsoft/winappcli` repo and asking the
+question the other dimensions do **not**: **should this change exist at all, and
+is it as small as it could be?** "*Can merge*" ≠ "*should merge*." Apply the
+shared output contract in `_shared-contract.md`. Set
+`Domain: necessity-and-simplicity` on every finding.
+
+This dimension is about scope and complexity, not micro code-reuse (that is
+`alternative-solution`). You are the one sub-agent allowed — required — to say
+"this is well-built but shouldn't ship as-is."
+
+## winapp's mission
+
+winapp exists to make **Windows app packaging, distribution, platform
+integration, and automation** easy for any app framework. Judge every change
+against that mission. A change that "wanders furthest from winapp's mission" is
+a finding even if the code is clean.
+
+## What to evaluate
+
+- **Does it earn its complexity?** Weigh the value delivered against the code,
+  surface area, and long-term maintenance it adds. A feature that duplicates
+  ~80% of an existing command's behavior should usually *extend* that command
+  instead (e.g., improve `wait-for` rather than add a new streaming watch).
+- **Does it fill a real need?** Is there a concrete user or workflow that needs
+  this, or is it code for its own sake / speculative generality? If the PR
+  author asked "does this fill a need?", answer it explicitly.
+- **Is it in scope?** Windows packaging / distribution / platform integration /
+  automation. Flag features that stray into general-purpose tooling the repo has
+  no mandate to own.
+- **Could it be smaller?** Fewer new commands/verbs/options, a narrower flag
+  set, one code path instead of two. Adding **6 new verbs at once** carries a
+  cumulative maintenance and testing burden — recommend **staging** the change
+  into a smaller first increment plus follow-ups.
+- **Is the abstraction premature?** New interface/service with a single caller
+  and no anticipated second → recommend inlining until a second use appears.
+- **Ship vs. don't-ship tradeoff.** For any non-trivial feature, state the
+  concrete pros of shipping **and** the pros of *not* shipping (or deferring).
+  If the change risks **false confidence** — a command that looks like it works
+  but silently misbehaves — say plainly that shipping it can be *reputationally
+  worse than not shipping at all*.
+
+## Output requirement
+
+For a feature-adding PR you must return at least one finding that either
+(a) recommends a concrete way to make the change smaller / staged / narrower in
+scope, or (b) explicitly signs off that the scope is justified — with a one-line
+"ship vs. don't ship" rationale — in your `## What I checked` note. Do not stay
+silent on necessity; a silent pass reads as "no opinion," which is the failure
+mode this dimension exists to fix.
+
+## What to drop
+
+- Pure taste ("I wouldn't have built it this way") with no scope, mission, or
+  maintenance argument behind it.
+- Re-litigating a design the maintainers have already explicitly decided on in
+  the diff's own commit messages or linked issue.
+
+## Severity guide for this dimension
+
+- Feature is out of scope for winapp's mission, or ships likely false
+  confidence → high.
+- Change earns little over extending an existing command; a materially smaller
+  or staged alternative exists → medium.
+- Premature abstraction / speculative option with a marginal simplification →
+  low.
diff --git a/.github/skills/pr-review/dimensions/security.md b/.github/skills/pr-review/dimensions/security.md
index 327b8f54..26ed45e2 100644
--- a/.github/skills/pr-review/dimensions/security.md
+++ b/.github/skills/pr-review/dimensions/security.md
@@ -61,6 +61,32 @@ This is a CLI tool that:
 - New HTTP listener bound to anything other than loopback → high.
 - Missing admin elevation check on a path that requires it → medium.
 
+## Threat-model checklist (required)
+
+For any diff that touches these surfaces, walk the checklist and record what you
+found — even when the answer is "not reachable":
+
+- **Input-injection** — CLI args, manifest/config values, UI-automation
+  selectors, or file contents that flow into a command, path, or query. Can a
+  crafted value inject an extra flag, a path traversal, or a shell metacharacter?
+- **Process-invocation** — for every new `Process.Start` / `ProcessStartInfo`:
+  are arguments passed via `ArgumentList` (safe) or concatenated (unsafe), and
+  where does each argument originate?
+- **Credentials & secrets** — cert passwords, PFX files, tokens, connection
+  strings: created, logged, left on disk, or hardcoded? Anything beyond the
+  documented dev default `password`?
+- **Signing** — does the change alter what gets signed, the trust chain, cert
+  install/trust, or let an untrusted input influence the signing target?
+- **Supply-chain** — new package refs (floating versions, known CVEs), new
+  downloads (non-Microsoft host, missing HTTPS/checksum), or suppressed security
+  analyzers.
+
+For the highest-risk item you find, describe a concrete **red-team attempt** the
+orchestrator can run in the Validate phase (e.g., "pass a manifest whose
+`Source` is `a b\" --flag` and confirm the extra flag reaches makeappx"). Keep
+such findings `Validation: static-only (needs runtime confirmation)` until the
+Validate phase reproduces or refutes them.
+
 ## Reminders
 
 - Security findings are **never suppressed** by low confidence. Emit them.

From deab1840dc721ea2645bd40ee22980ccfa2058cf Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:14:00 -0700
Subject: [PATCH 2/7] pr-review: remove reviewer-side finding quotas; keep
 genuine analysis
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A quota (must produce >=1 finding) pressures models to manufacture findings —
the exact anti-pattern this skill prevents. Remove the quota from the
alternative-solution and necessity-and-simplicity dimensions and add a shared
'No quotas — a clean result is a valid result' rule, while keeping the
requirement to do genuine search/reasoning and to surface anything real.

- alternative-solution.md: 'Required output: name a concrete alternative' ->
  'Do the search, then report only what's real' (no quota; clean pass is valid).
- necessity-and-simplicity.md: 'Output requirement' -> 'Reason about necessity,
  then state your conclusion' (a reasoned 'appropriately scoped' is complete);
  softened the intro's 'allowed - required - to say' phrasing.
- _shared-contract.md: added 'No quotas — a clean result is a valid result'
  section after the Team Lead Test.

Left legitimate code-under-review checks untouched (exit codes, test-coverage
'at least one test', the multi-model model-family record, orchestrator
never-abort resilience).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../pr-review/dimensions/_shared-contract.md  |  8 +++++++
 .../dimensions/alternative-solution.md        | 20 +++++++++-------
 .../dimensions/necessity-and-simplicity.md    | 24 ++++++++++++-------
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/.github/skills/pr-review/dimensions/_shared-contract.md b/.github/skills/pr-review/dimensions/_shared-contract.md
index 6e730bcd..81df8cc9 100644
--- a/.github/skills/pr-review/dimensions/_shared-contract.md
+++ b/.github/skills/pr-review/dimensions/_shared-contract.md
@@ -78,6 +78,14 @@ Specifically, **drop**:
 - Coverage gaps with concrete impact.
 - Doc/sample/packaging drift caused by this change.
 
+## No quotas — a clean result is a valid result
+
+No dimension is required to produce a finding. Zero findings is a legitimate,
+valuable outcome that tells the developer this area is solid. **Never invent,
+inflate, or lower the bar on a finding just to avoid an empty report** — a
+manufactured finding fails the Team Lead Test by definition. When you have
+nothing to flag, say so and record what you checked in `## What I checked`.
+
 ## Severity guide
 
 | Severity | Meaning |
diff --git a/.github/skills/pr-review/dimensions/alternative-solution.md b/.github/skills/pr-review/dimensions/alternative-solution.md
index 5fab2ab6..7113dc71 100644
--- a/.github/skills/pr-review/dimensions/alternative-solution.md
+++ b/.github/skills/pr-review/dimensions/alternative-solution.md
@@ -12,13 +12,13 @@ But do not self-censor a genuine "there's a better approach" critique just
 because it borders on scope; raise the concrete alternative here and let that
 dimension own the necessity framing.)
 
-## Required output: name a concrete alternative
+## Do the search, then report only what's real
 
-Do not stop at "uses `AppxManifestDocument` correctly." Every run must produce
-**at least one concrete, named alternative** — an in-repo helper/service/pattern
-*or* an ecosystem tool/library/API — with a one-line tradeoff, backed by a quick
-search (grep the repo for the helper, or draw on ecosystem knowledge). Examples
-of the bar to clear:
+Don't stop at "uses `AppxManifestDocument` correctly" — actually look for a
+simpler or more idiomatic path: grep the repo for an existing
+helper/service/pattern, and consider whether a standard ecosystem
+tool/library/API would do the job. If you find a genuinely better approach,
+surface it as a finding with a one-line tradeoff. Examples of the bar to clear:
 
 - "Extend the existing `wait-for` command instead of adding a new streaming
   watch — reuses its polling loop, one fewer command to maintain."
@@ -27,9 +27,11 @@ of the bar to clear:
 - "For the accessibility pass, lean on Accessibility Insights / Axe and frame it
   as a quick lint, not a WCAG audit."
 
-If after searching you genuinely find no better alternative, say so in
-`## What I checked` **and name what you searched for**, so the sign-off is
-verifiable. A silent "no alternatives" is not acceptable.
+There is **no quota**. If the change already takes the simplest reasonable
+approach, that is a clean pass — say so in `## What I checked` and name what you
+searched for, so the sign-off is verifiable. Never invent or stretch an
+alternative just to have something to report; a forced alternative fails the
+Team Lead Test.
 
 ## Repo-specific patterns to enforce
 
diff --git a/.github/skills/pr-review/dimensions/necessity-and-simplicity.md b/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
index 11488577..444c7f4a 100644
--- a/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
+++ b/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
@@ -7,8 +7,8 @@ shared output contract in `_shared-contract.md`. Set
 `Domain: necessity-and-simplicity` on every finding.
 
 This dimension is about scope and complexity, not micro code-reuse (that is
-`alternative-solution`). You are the one sub-agent allowed — required — to say
-"this is well-built but shouldn't ship as-is."
+`alternative-solution`). You are the one sub-agent whose job is to be willing to
+say "this is well-built but shouldn't ship as-is" when that is genuinely true.
 
 ## winapp's mission
 
@@ -41,14 +41,20 @@ a finding even if the code is clean.
   but silently misbehaves — say plainly that shipping it can be *reputationally
   worse than not shipping at all*.
 
-## Output requirement
+## Reason about necessity, then state your conclusion
 
-For a feature-adding PR you must return at least one finding that either
-(a) recommends a concrete way to make the change smaller / staged / narrower in
-scope, or (b) explicitly signs off that the scope is justified — with a one-line
-"ship vs. don't ship" rationale — in your `## What I checked` note. Do not stay
-silent on necessity; a silent pass reads as "no opinion," which is the failure
-mode this dimension exists to fix.
+Always *form and state* an opinion on necessity and size — this is the question
+the other dimensions skip. But that opinion can be "the scope is justified":
+
+- If a smaller / staged / narrower-scope change is genuinely better, raise it as
+  a finding with a concrete recommendation.
+- If the scope is justified, say so explicitly in `## What I checked` with a
+  one-line "ship vs. don't-ship" rationale.
+
+There is **no quota** — a reasoned "this is appropriately scoped" is a complete,
+valuable result. Don't manufacture a scope objection to avoid an empty report;
+just don't skip the question either (a silent omission reads as "no opinion,"
+which is the gap this dimension exists to close).
 
 ## What to drop
 

From 1e802bd33a442da3bf03ddfa8edfcb02cdb39da3 Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:15:18 -0700
Subject: [PATCH 3/7] pr-review: cover both directions of DRY in
 alternative-solution

Cross-cutting checks now flag same-logic-implemented-multiple-times-in-this-PR
(factor out into one shared helper), not just reuse of code that already exists
elsewhere. Grounded in the ui-input-verbs review's copy-paste description leaks
across the 5 new verbs. Added a matching severity line. Stays a flag-when-real
check, consistent with the no-quota framing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../pr-review/dimensions/alternative-solution.md   | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/.github/skills/pr-review/dimensions/alternative-solution.md b/.github/skills/pr-review/dimensions/alternative-solution.md
index 7113dc71..b6fd45ff 100644
--- a/.github/skills/pr-review/dimensions/alternative-solution.md
+++ b/.github/skills/pr-review/dimensions/alternative-solution.md
@@ -70,8 +70,20 @@ Team Lead Test.
 
 ## Cross-cutting checks
 
+DRY runs both directions — reusing what already exists **and** factoring out
+what this PR repeats:
+
 - Does this change duplicate logic that already exists in another command,
   service, or helper? Search for similar patterns and recommend reuse.
+- **Same logic implemented more than once in this PR.** If the diff repeats the
+  same or near-identical block across multiple commands / methods / files (e.g.
+  copy-pasted argument parsing, error handling, cache/lookup logic, or a
+  duplicated command description), recommend extracting a single shared
+  helper/method and calling it from each site. Cite the specific duplicated
+  locations, and watch for near-duplicates that will silently drift out of sync.
+- **Reimplementing something that already exists.** If the new code re-derives
+  behavior an existing command/service/helper already provides, recommend
+  calling the existing one instead of standing up a parallel implementation.
 - Could a new method be a simple call to an existing helper plus a 2-3 line
   wrapper? If so, recommend the wrapper.
 - Is a new abstraction premature (one caller, no anticipated second)?
@@ -90,6 +102,8 @@ Team Lead Test.
 
 - Re-implementing existing helper logic (manifest XML, PRI, selectors) →
   medium.
+- Non-trivial logic duplicated across 3+ sites (will drift out of sync) →
+  medium; a small localized copy-paste → low.
 - Wrong service-pattern choice that will need rework → medium.
 - File size now over hard limit → medium.
 - Minor "could reuse helper X" with marginal benefit → low.

From affb7e570a3c306e34a2e698d75ef952c39910ba Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:16:30 -0700
Subject: [PATCH 4/7] pr-review: refer to latest model per family, not pinned
 versions

Pinned model IDs (gpt-5.4, claude-opus-4.7) go stale. The two multi-model
override examples now say 'the latest model from a different family' (GPT /
Opus / Gemini). Bare family-name references elsewhere stay.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .github/skills/pr-review/SKILL.md                  | 10 +++++-----
 .github/skills/pr-review/dimensions/multi-model.md | 12 +++++++-----
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/.github/skills/pr-review/SKILL.md b/.github/skills/pr-review/SKILL.md
index 434ed526..c35347bf 100644
--- a/.github/skills/pr-review/SKILL.md
+++ b/.github/skills/pr-review/SKILL.md
@@ -144,11 +144,11 @@ For #9 (multi-model), wait until #1–#8 finish first. Pass it the **raw diff pl
 the actual changed code files** — **not** the consolidated findings, which anchor
 it into a rubber-stamp — and require an **independent pass** first; the
 specialists' critical/high list is optional reconciliation input it reads only
-afterward. Require it to (a) use a **different model family** than the
-orchestrator (if you are Claude, override to `gpt-5.4`; if GPT, to
-`claude-opus-4.7`) and (b) record which model family actually ran. For high-risk
-PRs, run it 2–3 times across different families (opus / gemini / gpt), each
-independent.
+afterward. Require it to (a) use the **latest model from a different family**
+than the orchestrator (if you are Claude/Opus, use the latest GPT or Gemini; if
+GPT, use the latest Opus or Gemini) and (b) record which model family actually
+ran. For high-risk PRs, run it 2–3 times across the three families (latest Opus,
+GPT, and Gemini), each independent.
 
 **Sub-agent fault-tolerance.** Tell every sub-agent that if a tool call is
 blocked by a policy hook or otherwise denied, it must **continue with the tools
diff --git a/.github/skills/pr-review/dimensions/multi-model.md b/.github/skills/pr-review/dimensions/multi-model.md
index eb6d3547..2764cb01 100644
--- a/.github/skills/pr-review/dimensions/multi-model.md
+++ b/.github/skills/pr-review/dimensions/multi-model.md
@@ -7,11 +7,13 @@ specialists. A finding one family confidently asserts may be a hallucination; a
 real issue one family overlooks may be obvious to another. That only works if
 you form your own view **before** you look at anyone else's.
 
-You **must** be invoked with a `model` override selecting a different model
-family than the orchestrator (e.g., a Claude orchestrator passes
-`model: "gpt-5.4"`; a GPT orchestrator passes `model: "claude-opus-4.7"`). For
-high-risk PRs the orchestrator may run you 2–3 times across different families
-(opus / gemini / gpt); each run is independent.
+You **must** be invoked with a `model` override selecting the **latest** model
+from a different family than the orchestrator — pick the newest available model
+from whichever of GPT, Opus, or Gemini is not the orchestrator's own family
+(e.g., a Claude/Opus orchestrator uses the latest GPT or Gemini; a GPT
+orchestrator uses the latest Opus or Gemini). For high-risk PRs the orchestrator
+may run you 2–3 times across different families (opus / gemini / gpt); each run
+is independent.
 
 ## Record which model actually ran (required)
 

From 2ba40e2b775eaff0f72c40702b70bb1ddd03e97f Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:25:14 -0700
Subject: [PATCH 5/7] Gate necessity-and-simplicity to feature PRs only

Make the necessity & simplicity dimension conditional so it does not
re-litigate settled decisions on bug fixes, refactors, docs, tests, and
CI/packaging changes. It now only runs when the diff adds new user-facing
surface (a new command/verb/flag/API/capability); otherwise the orchestrator
skips it and marks it n/a (no new surface) in the Coverage block. Adds an
up-front gate to the dimension file, wires the conditional fan-out and counts
in SKILL.md (8 always-on + 1 conditional), and adds a non-feature worked
example alongside the feature one.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .github/skills/pr-review/SKILL.md             | 64 ++++++++++++++++---
 .../dimensions/necessity-and-simplicity.md    | 14 ++++
 2 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/.github/skills/pr-review/SKILL.md b/.github/skills/pr-review/SKILL.md
index c35347bf..a0e984b3 100644
--- a/.github/skills/pr-review/SKILL.md
+++ b/.github/skills/pr-review/SKILL.md
@@ -119,14 +119,22 @@ buckets in this repo:
 
 ### 4. Fan out parallel sub-agents
 
-Launch all 8 specialist dimension sub-agents (#1–#8) in **the same response**
+Launch the specialist dimension sub-agents (#1–#8) in **the same response**
 using the `task` tool, mode `"sync"`, agent type `general-purpose` (or `explore`
 for read-only dimensions — see per-dimension files). Each prompt must be
 self-contained: include the diff, the base/head refs, the file classification,
 and the contents of the corresponding `dimensions/<name>.md` file as
 instructions.
 
-The 9 dimensions and their fragment files:
+**#5 (necessity & simplicity) is conditional.** Launch it **only when the diff
+adds new user-facing surface** — a new command / verb / subcommand, a new public
+flag / option / API, or a materially new capability. For bug fixes, refactors,
+perf, docs, tests, and dependency / CI / packaging changes, **skip it** and mark
+it `n/a (no new surface)` in the Coverage block. A non-feature PR fans out 7
+specialists; a feature PR fans out all 8.
+
+The 9 dimensions and their fragment files — **8 always-on + 1 conditional**
+(necessity & simplicity, feature PRs only):
 
 | # | Dimension | Fragment | Default agent |
 |---|-----------|----------|---------------|
@@ -134,7 +142,7 @@ The 9 dimensions and their fragment files:
 | 2 | correctness & edge cases (incl. regression) | `dimensions/correctness.md` | general-purpose |
 | 3 | CLI UX & usability | `dimensions/cli-ux.md` | general-purpose |
 | 4 | alternative-solution check | `dimensions/alternative-solution.md` | general-purpose |
-| 5 | necessity & simplicity | `dimensions/necessity-and-simplicity.md` | general-purpose |
+| 5 | necessity & simplicity — *conditional, feature PRs only* | `dimensions/necessity-and-simplicity.md` | general-purpose |
 | 6 | test coverage | `dimensions/test-coverage.md` | general-purpose |
 | 7 | docs & samples sync | `dimensions/docs-and-samples.md` | explore |
 | 8 | packaging & release impact | `dimensions/packaging.md` | general-purpose |
@@ -243,7 +251,7 @@ Coverage
   correctness           ...
   cli-ux                ...
   alternative-solution  ...
-  necessity-simplicity  ...
+  necessity-simplicity  <n/a (no new surface) | ✓ clean | ⚠ N findings>
   test-coverage         ...
   docs-and-samples      ...
   packaging             ...
@@ -296,8 +304,8 @@ PR:
 
 ## Rules the orchestrator must enforce
 
-- **Parallelism in one turn.** Fan out all of #1–#8 in a single response; run
-  #9 (multi-model) after they return.
+- **Parallelism in one turn.** Fan out the specialists (#1–#8, skipping #5 on
+  non-feature PRs) in a single response; run #9 (multi-model) after they return.
 - **Validate high/critical for real.** Build and run the branch to confirm or
   drop critical/high findings (see the Validate phase). This **replaces** the
   old "no build/test execution" rule. Still flag staleness you can see
@@ -348,14 +356,14 @@ only after its own pass.
 1. git diff --stat origin/main...HEAD          → 12 files, +340/-87
 2. git diff origin/main...HEAD                 → captured for sub-agents
 3. Map files to areas                          → mostly Services + Commands + 1 doc
-4. Fan out 8 task() calls in parallel          → wait; retry any that died
+4. Fan out 7–8 task() calls in parallel        → skip #5 necessity if no new surface; retry any that died
 5. Fan out task() #9 (diff+code, model override) → wait, record model family
 6. Dedupe, sort, ID, reconcile multi-model, seed validation
 7. Validate: build + install as a user + run changed cmds → confirm/drop crit+high
 8. Print stdout report   (opt-in follow-through only if asked)
 ```
 
-## Example consolidated stdout
+## Example consolidated stdout (feature PR)
 
 ```
 PR Review — feat/sparse-trustedlaunch vs origin/main  (4 commits, 9 files, +312/-58)
@@ -412,6 +420,46 @@ Coverage notes
   packaging: Inspected version.json, npm wrapper, NuGet targets — no impact.
 ```
 
+## Example consolidated stdout (non-feature PR — bug fix)
+
+A bug fix / refactor adds no new command, flag, or capability, so the
+orchestrator **skips** necessity & simplicity and marks it `n/a`:
+
+```
+PR Review — fix/stream-empty-file vs origin/main  (2 commits, 3 files, +41/-12)
+
+Summary
+  Critical: 1   High: 0   Medium: 1   Low: 0
+
+Coverage
+  security              ✓ clean
+  correctness           ⚠ 1 finding
+  cli-ux                ✓ clean
+  alternative-solution  ✓ clean
+  necessity-simplicity  n/a (no new surface)
+  test-coverage         ⚠ 1 finding
+  docs-and-samples      ✓ clean
+  packaging             ✓ clean
+  multi-model           ✓ 1/1 critical confirmed  ·  models: gemini
+  regression            ✓ fixes a regression; no new behavior change
+  validation            ✓ 1/1 critical validated at runtime
+
+Findings
+  C1  src/winapp-CLI/.../StreamService.cs:77-84   correctness    Watch writes an empty file when the source never flushes
+  M1  src/winapp-CLI/.../StreamService.cs:77-84   test-coverage  No regression test pins the non-empty-output fix
+
+Details
+## C1  src/winapp-CLI/WinApp.Cli/Services/StreamService.cs:77-84
+- Severity: critical
+- Confidence: high
+- Validation: validated
+- Domain: correctness
+- Multi-model: confirmed
+- Finding: On a cold cache the streaming watch still closes the output file before the first flush, producing a 0-byte file.
+- Evidence: Reproduced on the npm-installed CLI (cold cache): `winapp ... --watch` left out.log empty until the fix at line 80 moved the flush ahead of Dispose.
+- Recommendation: Keep the fix; add the regression test from M1 so the empty-file case can't silently return.
+```
+
 ## Opt-in follow-through
 
 Default behavior is unchanged: **stdout only, no fixes, no posting.** Only when
diff --git a/.github/skills/pr-review/dimensions/necessity-and-simplicity.md b/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
index 444c7f4a..8c1b2c17 100644
--- a/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
+++ b/.github/skills/pr-review/dimensions/necessity-and-simplicity.md
@@ -10,6 +10,20 @@ This dimension is about scope and complexity, not micro code-reuse (that is
 `alternative-solution`). You are the one sub-agent whose job is to be willing to
 say "this is well-built but shouldn't ship as-is" when that is genuinely true.
 
+## When this dimension applies (gate)
+
+This dimension only produces findings when the PR **adds new user-facing surface
+area** — a new command / verb / subcommand, a new public flag / option / API, or
+a materially new capability. That is the only case where "should this exist, and
+is it too big?" is a live question rather than a re-litigation of settled work.
+
+For changes that do **not** add new surface — bug fixes, internal refactors,
+perf work, docs, tests, dependency / CI / packaging tweaks — **do not hunt for
+scope concerns.** Return a clean pass: note in `## What I checked` that this is
+"Not a feature-adding change — necessity review N/A," and stop. When it *is* a
+feature PR, the guidance below applies in full (and the no-quota framing still
+holds — an appropriately scoped feature is a clean pass too).
+
 ## winapp's mission
 
 winapp exists to make **Windows app packaging, distribution, platform

From d3489705d0900911fa775e6c26d4109033328db7 Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:27:23 -0700
Subject: [PATCH 6/7] Default Validate to single-file binary; make Gemini a
 co-equal model family

Validate phase now defaults to the published single-file binary (dotnet
publish) on a clean PATH, escalating to npm pack or MSIX only when the diff
touches the npm wrapper or MSIX packaging/install/identity. Multi-model
cross-check now treats GPT, Opus, and Gemini as three co-equal families with
balanced pick-from-a-different-family examples (including a Gemini
orchestrator), keeping the high-risk run-all-three guidance. No pinned model
versions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .github/skills/pr-review/SKILL.md             | 29 ++++++++++---------
 .../pr-review/dimensions/multi-model.md       | 13 +++++----
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/.github/skills/pr-review/SKILL.md b/.github/skills/pr-review/SKILL.md
index a0e984b3..309dcbff 100644
--- a/.github/skills/pr-review/SKILL.md
+++ b/.github/skills/pr-review/SKILL.md
@@ -153,10 +153,11 @@ the actual changed code files** — **not** the consolidated findings, which anc
 it into a rubber-stamp — and require an **independent pass** first; the
 specialists' critical/high list is optional reconciliation input it reads only
 afterward. Require it to (a) use the **latest model from a different family**
-than the orchestrator (if you are Claude/Opus, use the latest GPT or Gemini; if
-GPT, use the latest Opus or Gemini) and (b) record which model family actually
-ran. For high-risk PRs, run it 2–3 times across the three families (latest Opus,
-GPT, and Gemini), each independent.
+than the orchestrator, chosen among the three co-equal families — GPT, Opus, and
+Gemini (if you are Opus, use the latest GPT or Gemini; if GPT, use the latest
+Opus or Gemini; if Gemini, use the latest Opus or GPT) — and (b) record which
+model family actually ran. For high-risk PRs, run it across all three families
+(latest Opus, GPT, and Gemini), each independent.
 
 **Sub-agent fault-tolerance.** Tell every sub-agent that if a tool call is
 blocked by a policy hook or otherwise denied, it must **continue with the tools
@@ -198,15 +199,15 @@ do** rather than silently skipping it:
 
 1. **Build the branch** (`scripts/build-cli.ps1`, or a targeted `dotnet build`).
    A build failure is itself a critical finding.
-2. **Install the CLI as a *user* would — not dev mode.** Do **not** validate via
-   `dotnet run` or a Debug worktree; that hides the cold-cache and packaging
-   bugs real users hit. Pick one:
-   - `npm pack` in `src/winapp-npm`, then `npm i -g` the tarball and run
-     `winapp …`;
-   - the built **MSIX** / installed global tool;
-   - the published single-file binary on a clean PATH.
-   Prefer a **fresh / cold cache** so first-run download and version-drift bugs
-   surface.
+2. **Run the CLI as a *user* would — not dev mode.** Do **not** validate via
+   `dotnet run` or a Debug worktree; that hides cold-cache and first-run bugs
+   real users hit. **Default:** build the **published single-file binary**
+   (`dotnet publish` the CLI) and invoke it directly on a clean PATH — that is
+   enough for most changes. **Escalate only when the change/scenario requires
+   it:** if the diff touches the **npm wrapper** (`src/winapp-npm`), validate via
+   `npm pack` + `npm i -g` the tarball; if it touches **MSIX packaging / install
+   / identity**, validate the built **MSIX** or installed global tool. Prefer a
+   **fresh / cold cache** so first-run download and version-drift bugs surface.
 3. **Scaffold a throwaway app** in a temp dir to exercise the changed commands
    against something real (`winapp init`, package, sign, run). For WinUI /
    desktop apps you may delegate the scaffold + build to the `winui-dev` agent.
@@ -359,7 +360,7 @@ only after its own pass.
 4. Fan out 7–8 task() calls in parallel        → skip #5 necessity if no new surface; retry any that died
 5. Fan out task() #9 (diff+code, model override) → wait, record model family
 6. Dedupe, sort, ID, reconcile multi-model, seed validation
-7. Validate: build + install as a user + run changed cmds → confirm/drop crit+high
+7. Validate: build + run the published single-file binary (npm/MSIX only if the change needs it) + run changed cmds → confirm/drop crit+high
 8. Print stdout report   (opt-in follow-through only if asked)
 ```
 
diff --git a/.github/skills/pr-review/dimensions/multi-model.md b/.github/skills/pr-review/dimensions/multi-model.md
index 2764cb01..cfefe93e 100644
--- a/.github/skills/pr-review/dimensions/multi-model.md
+++ b/.github/skills/pr-review/dimensions/multi-model.md
@@ -8,12 +8,13 @@ real issue one family overlooks may be obvious to another. That only works if
 you form your own view **before** you look at anyone else's.
 
 You **must** be invoked with a `model` override selecting the **latest** model
-from a different family than the orchestrator — pick the newest available model
-from whichever of GPT, Opus, or Gemini is not the orchestrator's own family
-(e.g., a Claude/Opus orchestrator uses the latest GPT or Gemini; a GPT
-orchestrator uses the latest Opus or Gemini). For high-risk PRs the orchestrator
-may run you 2–3 times across different families (opus / gemini / gpt); each run
-is independent.
+from a different family than the orchestrator, chosen among the three co-equal
+families — **GPT, Opus, and Gemini**. Pick the newest available model from
+whichever two are not the orchestrator's own family (an Opus orchestrator uses
+the latest GPT or Gemini; a GPT orchestrator uses the latest Opus or Gemini; a
+Gemini orchestrator uses the latest Opus or GPT). For high-risk PRs the
+orchestrator may run you across all three families (opus / gpt / gemini); each
+run is independent.
 
 ## Record which model actually ran (required)
 

From 03d8bf3cccd0f1b483199c60de124b89d8b16343 Mon Sep 17 00:00:00 2001
From: Nikola Metulev <711864+nmetulev@users.noreply.github.com>
Date: Fri, 10 Jul 2026 14:28:46 -0700
Subject: [PATCH 7/7] Remove Multi-PR / multi-target mode from pr-review skill

Over-scoped for a single-PR review skill. The skill reviews one
PR/branch/diff per run; a user wanting several can invoke it per PR. Deletes
the whole Multi-PR / multi-target section and all its references. The generic
dimension-scoped fault-tolerance rule (retry a dead dimension sub-agent once,
else mark skipped + reason) is kept, no longer tied to per-PR.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .github/skills/pr-review/SKILL.md | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/.github/skills/pr-review/SKILL.md b/.github/skills/pr-review/SKILL.md
index 309dcbff..354e1cf4 100644
--- a/.github/skills/pr-review/SKILL.md
+++ b/.github/skills/pr-review/SKILL.md
@@ -284,25 +284,6 @@ If a sub-agent returned zero findings, list its dimension as `✓ clean` in the
 Coverage block and include its short "what I checked" note in a final
 `Coverage notes` section so the user can see scope, not just verdict.
 
-## Multi-PR / multi-target mode
-
-When the user asks to review **several PRs or targets in one run** ("review
-#583, #573 and #587", "review all the UI-input PRs"), don't make them re-ask per
-PR:
-
-1. Enumerate the targets (PR numbers, branches, or refs).
-2. For **each** target, run the full workflow (scope → fan-out → validate →
-   consolidate) inside its **own trackable sub-agent**, launched in parallel and
-   named per target (e.g. `review-pr-583`) so progress is visible and a stall is
-   obvious.
-3. Apply the same fault-tolerance rule: if a per-PR sub-agent dies, retry it
-   once; if it still fails, report that target as `✗ skipped + reason` rather
-   than dropping it.
-4. Print **one report per target**, then a short **cross-PR roll-up**: shared
-   findings, ordering / stacking risks, and — when the user is deciding what to
-   ship — a per-PR "ship / stage / hold" line drawn from the
-   necessity-and-simplicity dimension.
-
 ## Rules the orchestrator must enforce
 
 - **Parallelism in one turn.** Fan out the specialists (#1–#8, skipping #5 on