Adds PR review skill and checklist for pull request evaluations

.github/skills/pr-review/SKILL.md

@@ -0,0 +1,203 @@
+---
+name: pr-review
+description: >-
+  This skill should be used when the user asks to "review a PR",
+  "check this pull request", "PR review", "review pull request",
+  "check PR against checklist", or needs to review a pull request
+  against the project's PR checklist and coding standards.
+---
+
+# PR Review
+
+Review pull requests against the project's PR checklist and coding standards. Produce a numbered list of findings for the user, then post inline review comments and a summary comment on the PR when approved.
+
+## Prerequisites
+
+- `gh` CLI authenticated with access to the repository.
+
+## Workflow
+
+### Phase 1: Gather PR Context
+
+1. If no PR number or URL was provided, prompt the user for it. Do not proceed without one.
+2. Extract the PR number. Accept formats: full URL, `#123`, or plain number.
+3. Run the following to collect PR metadata and diff:
+
+```bash
+gh pr view <number> --json number,title,body,author,files,additions,deletions,baseRefName,headRefName
+gh pr diff <number>
+```
+
+4. Identify the type of change (new command, bug fix, documentation, refactor, etc.) from the PR title, body, and changed files. This determines which checklist items apply.
+
+### Phase 2: Review
+
+Read `references/pr-checklist.md` for the full checklist.
+
+Review the diff against **all three categories** below. Not every checklist item applies to every PR — evaluate applicability based on the change type identified in Phase 1.
+
+#### A. Checklist Compliance
+
+Walk through each applicable item in the checklist and verify it against the diff. Key items to check:
+
+- **Single quotes** for strings in command files (not double quotes). For test files, this is less strictly enforced.
+- **Command class naming**: `[Service][Command]Command` pattern.
+- **Command name in `commands.ts`**: Must be placed in alphabetical order.
+- **`force` option** on remove commands.
+- **`async/await`** instead of `promise/then`.
+- **`handleRejectedODataJsonPromise`** when `responseType: 'json'` is used.
+- **`defaultProperties`** in list commands instead of conditional JSON output.
+- **Custom Zod validation** for mutually exclusive options.
+- **Verbose logging**: Every command should include at least one verbose log message that provides useful execution context.
+- **User input escaped** in XML and URLs using `formatting.encodeQueryParameter()` or `formatting.escapeXml()`.
+- **`GetFileByServerRelativePath`/`GetFolderByServerRelativePath`** in `spo` commands (not `...Url` variants).
+- **No `any` types**, no commented-out code.
+- **Bug fixes** include a test for the fixed case.
+- **Documentation** included where needed.
+
+#### B. Code Quality
+
+Review the implementation for:
+
+- Logic errors, off-by-one mistakes, unhandled edge cases.
+- Security issues (injection, unvalidated input, leaked credentials).
+- Performance concerns (unnecessary API calls, missing pagination).
+- TypeScript best practices (proper typing, no implicit any).
+
+#### C. Test Quality
+
+Check test files (`.spec.ts`) for:
+
+- Coverage of happy path and error cases.
+- Proper use of mocks and assertions.
+- Tests that actually verify behavior (not just that code runs).
+- Tests for command name, description not being `null`, and schema validation.
+- Use `sinon.restore()` in `after()` to reset stubs/spies.
+
+#### D. Documentation Quality
+
+Check documentation files (`.mdx`) for:
+
+- Every command needs a reference page at `docs/docs/cmd/<workload>/<command-name>.mdx`.
+- The `.mdx` file name matches the command file name.
+- Must include: title, description, usage, options table, examples, permissions, and response. A remarks section can be used for additional information, but is not mandatory.
+- When a command has a response, include sample output for all four output formats: JSON, Markdown, text, and CSV.
+- Include at least 2 examples for the examples section, covering different option combinations.
+- Import and use `<Global />` for standard CLI options.
+- Examples should use `m365` prefix and long option names (not short aliases).
+- Document the minimum required permissions that allow success with any option combination.
+- Docs must build without warnings.
+- When importing components in the docs, use absolute imports from `@site/src/components/` instead of relative paths.
+- When importing global options in the docs, use a relative import from `../_global.mdx`.
+
+### Phase 3: Present Findings
+
+Compile all findings into a **numbered list** and present it to the user in chat. Each finding must include:
+
+1. **File and line reference** (linked).
+2. **Category**: Checklist | Code Quality | Test Quality.
+3. **Severity**: Error (must fix) | Warning (should fix) | Suggestion (nice to have).
+4. **Description**: What the issue is and why it matters.
+5. **Suggestion**: What to change (be specific).
+
+If there are **zero findings**, state that the PR looks good.
+
+Sort findings: Errors first, then Warnings, then Suggestions.
+
+### Phase 4: Post Comments
+
+Always ask the user for confirmation before posting any comments on the PR. Present the planned comments and wait for explicit approval. Then:
+
+1. **Submit a review** with inline comments and the appropriate verdict:
+   - **Zero findings** → `APPROVE`
+   - **Any findings** (error, warning, or suggestion) → `REQUEST_CHANGES`
+
+Post inline comments and the summary as a single review submission. Always target the actual line where the issue exists.
+
+**API limitation**: The GitHub API can only target lines within diff hunk ranges (changed lines + surrounding context lines). If the target line falls in a gap between hunks, do **not** post it on a nearby line. Instead, include that finding in the summary (the review body) with the file path and line number. Only use `suggestion` blocks for findings posted on the exact target line.
+
+**Line number accuracy**: When posting inline comments, verify line numbers against the actual diff hunk content. The line number refers to the **new file** line number (RIGHT side). Count lines in the diff hunk carefully — off-by-one errors cause comments to land on the wrong line and suggestion blocks to replace the wrong code.
+
+```bash
+gh api repos/{owner}/{repo}/pulls/{number}/reviews \
+  --method POST \
+  --input - <<'EOF'
+{
+  "event": "APPROVE or REQUEST_CHANGES",
+  "body": "<summary>",
+  "comments": [
+    {
+      "path": "<file>",
+      "line": <line>,
+      "side": "RIGHT",
+      "body": "<comment>"
+    }
+  ]
+}
+EOF
+```
+
+The `comments` array can contain multiple entries. Omit it entirely when approving with zero findings.
+
+#### Comment Tone Guidelines
+
+These comments appear on a public repository and represent the user responding to a contributor's work. Every comment must be:
+
+- **Constructive**: Frame issues as suggestions, not demands. Use "Consider..." or "It might be worth..." instead of "You must..." or "This is wrong."
+- **Specific**: Reference exact lines, show the expected code, explain *why* a change matters.
+- **Appreciative**: Acknowledge good work. If the PR is mostly solid, say so. Contributors volunteer their time.
+- **Brief**: One or two sentences per inline comment. Save detail for the summary.
+- **Professional**: No sarcasm, no passive-aggressive phrasing, no exclamation marks on criticism.
+
+For inline comments, use this format:
+```
+[Category | Severity] Description.
+
+Suggestion: `<code or guidance>`
+```
+
+When confident a specific code change is correct, include a GitHub suggestion block to make it easy for the contributor to apply:
+
+````
+```suggestion
+<corrected code>
+```
+````
+
+Only use suggestion blocks when the fix is unambiguous and self-contained. Do not guess.
+
+For the summary comment, use this format:
+```markdown
+## PR Review Summary
+
+**PR**: #<number> - <title>
+**Author**: @<author>
+
+### Overview
+<1-2 sentence assessment>
+
+### Findings
+<numbered list of findings with severity>
+
+### Checklist Coverage
+<brief note on which checklist categories were verified>
+
+---
+*Reviewed against the [PR checklist](docs/docs/contribute/pr-checklist.mdx).*
+```
+
+If the PR has zero issues, the summary should simply acknowledge the quality of the work and approve.
+
+## Additional Resources
+
+### Reference Files
+
+- **`references/pr-checklist.md`** — Full PR checklist with all items organized by category (General, Coding Standards, Documentation).
+
+### Reference Commands
+
+When providing feedback, point contributors to well-implemented commands as examples. Use these as reference implementations:
+
+- **`src/m365/spo/commands/list/list-list.ts`** — Example of a list command with `defaultProperties` and proper text output.
+- **`src/m365/spo/commands/web/web-get.ts`** — Example of a get command with verbose logging and proper error handling.
+- **`src/m365/entra/commands/user/user-get.ts`** — Example of a command using Zod schema validation.

.github/skills/pr-review/references/pr-checklist.md

@@ -0,0 +1,42 @@
+# PR Checklist
+
+## General guidelines
+
+- Ensure the build passes.
+- No ESLint errors.
+- Achieve 100% code coverage.
+- The submission should match the specification.
+- Maintain a single commit, or squash multiple commits.
+- Use single quotes `' '` for strings in command files. For test files this is less strictly enforced, since responses copied from API requests often contain double quotes.
+- If submitting a sample, ensure it is properly formatted and indented.
+
+## Coding standards
+
+- Command options should follow the naming convention (kebab-case for CLI flags).
+- The command should have a correct name.
+- The command name added to `commands.ts` should be placed so that commands are sorted alphabetically.
+- The command class is named following the pattern `[Service][Command]Command`. For example, `SpoWebRemoveCommand`.
+- Verify the command works as expected.
+- List commands must have readable output in `text` mode, with each item fitting in one row of 130 characters preferably.
+- Avoid commented-out code and usage of `any` types, preferring specific types.
+- Remove commands should include a `force` option.
+- For bug fixes, include a test for the fixed use case.
+- Avoid unnecessary retrieval of form digest.
+- Handle failed promises properly when `responseType: 'json'` is used by using `handleRejectedODataJsonPromise`.
+- Escape user input in XML and URLs using `formatting.encodeQueryParameter()` or `formatting.escapeXml()`.
+- Verbose and debug outputs are logged to stdErr (`logger.logToStderr` instead of `logger.log`).
+- Do not do conditional output in JSON output mode; use `defaultProperties` for defining default properties in list commands.
+- For commands with multiple options where the user is required to choose one, define these options using a custom Zod validation.
+- Use `async/await` instead of `promise/then`.
+- When working with `spo` commands, use `GetFileByServerRelativePath` and `GetFolderByServerRelativePath` API endpoint instead of `GetFileByServerRelativeUrl` and `GetFolderByServerRelativeUrl`.
+- `npm test` must pass without errors.
+
+## Documentation
+
+- Include an `mdx` help page.
+- Reference the `mdx` help page in the sidebar navigation.
+- Start all code samples with `m365`.
+- Ensure samples use long names of options rather than short ones.
+- Include the marker to incorporate global options rather than listing them explicitly.
+- If there is an option modifying the output, include responses for both default and modified output.
+- For a command page, ensure it includes the following sections: title, description, usage, options, examples, permissions, and response. In some cases a remarks section is also allowed.