Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/skills/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ repo-specific developer tasks like reviewing a PR before push.

| Skill | Purpose |
|-------|---------|
| [`pr-lifecycle/`](pr-lifecycle/SKILL.md) | Prepare, present, and finish a PR with evidence, in-PR fixes, and a current-head readiness state distinct from human approval. Never merges; drafting or reviewing alone does not authorize mutations. |
| [`pr-review/`](pr-review/SKILL.md) | Multi-dimensional review of a PR / feature branch diff (security, correctness, CLI UX, alternative solutions, tests, docs/samples, packaging, multi-model cross-check). Reports findings to stdout; does not apply fixes. |
| [`spec-review/`](spec-review/SKILL.md) | Pre-code, decision-oriented review of a spec or proposed feature against the real codebase (necessity & scope, approach & alternatives, feasibility vs reality, risks/unknowns/edge cases, DX & user impact, multi-model cross-check). Reports a proceed / proceed-with-changes / reconsider recommendation to stdout; does not change code or the spec. |

Expand All @@ -25,4 +26,5 @@ repo-specific developer tasks like reviewing a PR before push.
instructions in `SKILL.md`.
- Prompt fragments meant to be passed verbatim to sub-agents live under a
`dimensions/` (or similarly named) subfolder.
- Output goes to stdout unless the user explicitly asks for a file.
- Reviews and drafts go to stdout unless the user explicitly asks for a file.
An authorized PR lifecycle also maintains the PR and one status comment.
238 changes: 238 additions & 0 deletions .github/skills/pr-lifecycle/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
---
name: pr-lifecycle
description: Prepare, present, and finish a contributor PR in microsoft/winappcli. Activate for "create a PR", "prepare this PR", "get this PR ready for review", or "take this PR through readiness". Keeps fixes in the existing PR, communicates evidence, and tracks current-head readiness separately from human approval. A body-only draft or review-only request does not authorize edits, posting, labels, or merge. Never merges or enables auto-merge.
infer: true
---

# PR lifecycle: Prepare, Present, Finish

Use this for work on this repository, not as a shipped `winapp` plugin skill.
The outcome is a clear PR and an honest readiness state, not a merge.

## Authority and repository policy

- Establish the requested scope first. "Draft a PR body" returns text only;
"review this PR" returns findings only. Neither authorizes changing code,
pushing, posting, requesting reviews, or changing labels. A request to create
or take a PR through readiness authorizes the necessary in-scope work, not
unrelated changes or merging. Confirm any additional authority you need.
- Read [AGENTS.md](../../../AGENTS.md), the
[PR template](../../PULL_REQUEST_TEMPLATE.md), affected workflow definitions in
`.github\workflows`, and applicable `.pipelines` instructions. Live branch
protection/rulesets are the source of truth for required checks and approvals;
do not freeze their names or counts here.
- [pr-review](../pr-review/SKILL.md) owns independent, practical review of
correctness, compatibility and performance. Use it for a full feature review;
keep a narrow docs/fix review proportional rather than copying its entire
orchestration. [spec-review](../spec-review/SKILL.md) is for unresolved design
questions before implementation, not another mandatory post-code ceremony.
- Fix findings **in the existing PR** by default. Create a follow-up fix PR only
when the user asks. Preserve the review-only default of `pr-review`.
- Use the host's `create_pull_request`, `update_pull_request`, and
`reply_and_resolve_review_thread` tools when available; obey their contracts
rather than substituting shell commands. Other GitHub reads/actions may use
`gh`. Never merge, enable auto-merge, bypass protection, dismiss a human
changes-request review, or manufacture an approval.

## Prepare

1. **Identify the real comparison.** Read the live PR (if it exists): repository,
number, state, head repository/ref/SHA, base ref/SHA, draft state, checks,
reviews and mergeability. Inspect local status, tracking refs and the full
diff including relevant untracked files. Fetch the actual head and base;
do not assume the local branch or `origin/main` is current. Record immutable
head/base SHAs for the assessment.
2. **Preserve other work.** Do not stage unrelated files, delete scratch files,
discard a dirty tree, or overwrite collaborator commits. If the remote head
differs from your expected head, inspect it before continuing; stop and
coordinate overlapping edits. Prefer a normal non-rewriting update. Rewrite
only with permission and an explicit expected-SHA lease
(`--force-with-lease=<ref>:<expected-sha>`); a failed lease means re-inspect,
not force harder.
3. **Respect stacks.** Review against the actual parent branch, not the entire
stack against main. Use the available native stack skill/tool to synchronize
parents and propagate updates. Check the parent and ultimate main as
appropriate to repository policy; do not blindly retarget to main or merge
main into a child. For an ordinary PR, incorporate required base updates
safely and resolve conflicts before assessing readiness.
4. **Review and repair.** Get independent review of the actual final diff.
Review concrete user paths, supported released compatibility contracts and
meaningful performance risks, not speculative edge cases. Fix actionable
defects with regression tests where applicable. Explain false positives with
evidence; do not silently mark them addressed. A prior review remains useful
only if its SHA is recorded and every subsequent delta is reviewed, or
range-equivalence is demonstrated. A rebase alone is not proof of equivalence.
5. **Validate the changed behavior.** Run the smallest relevant existing tests,
builds and user-path checks required by repository instructions. Update only
affected documentation. Record exact commands and observed outcomes,
including limitations. For contributor Markdown-only changes, check
frontmatter, links/paths and Markdown plus applicable docs tooling; no full
NativeAOT build is needed unless that tooling requires it.

Do not call a local test pass a CI pass, or call your own inspection independent
review. Pending independent review is a pending gate, not a reason to invent
edits to elicit a favorable review.

## Present

Use a short, outcome-oriented title and the repository PR template. Keep its
applicable headings; mark non-applicable sections honestly. The body describes
the **final feature**, not each round of agent activity:

- Explain the plain-language problem and what users can now do.
- Show the smallest concrete before/after command, input or interaction and
actual result. Distinguish observed output from an illustrative example.
- Include focused validation evidence, honest limitations, and only real
compatibility, dependency or performance tradeoffs. For compatibility, use
the published-contract test in `AGENTS.md`.
- For visual changes, capture actual before/after screenshots. For interactions,
use a short recording showing the relevant behavior. For nonvisual changes,
use textual CLI/API examples; documentation-only work needs no screenshots.
Performance claims need benchmark conditions (build, hardware, workload,
cache state and baseline) and measured results.
- Never fabricate/decorate evidence or include secrets, personal data or other
sensitive content. Use `github-pr-media` for uploads when available and follow
its permissions. If evidence cannot be captured, state the limitation.
- Omit file inventories, agent transcripts, internal finding IDs, review-round
logs and model coverage. Reviewers should not need the agent conversation.

Before editing an existing body, read its live content. Preserve human edits and
use the update tool's optimistic guard (`base_sha` when supplied). On a stale
edit, re-read and reconcile instead of retrying an old full-body replacement.
If the host has no guard, re-read immediately before a minimal update and stop
on conflicting edits.

Maintain **one separate lifecycle status comment** on the PR. Find it in the
paginated comments or recover its saved ID before creating one; do not append a
new comment for every round. Update only your workflow comment, preserving any
collaborator edits. Keep history out of the feature body and avoid repeating it
all in chat. A compact comment can be:

```markdown
<!-- pr-lifecycle-status -->
**State:** agent-preparing
**Assessed:** head `<sha>` against `<base-ref>@<sha>`; main `<sha>` if stacked
**Evidence:** <focused commands/results and links to current-head CI/review>
**Review:** <independent coverage SHA; actionable findings; human approval status>
**Pending:** <remaining gates, or none>
**Blocker / owner / next step:** <concrete intervention, or none>
**Resume:** <trigger or supported wakeup; otherwise "Not actively monitoring">
```

Record the PR URL/number, head/base SHAs (and main/parent when relevant),
status-comment ID, pending gates and next action in a durable session checkpoint
too. Do not commit operational checkpoints to the repository.

## Finish

### Read the complete live discussion

Read **all pages** of inline review threads, including replies, and all
top-level review bodies and issue comments. Inspect the current PR body for
actionable requests too. Do not infer "no findings" from a summary, the last
review, unresolved-thread count alone, or the first 100 results.

Reply to each actionable thread with the fix/evidence or a reasoned disagreement
**before** resolving it. Use `reply_and_resolve_review_thread` when available.
For actionable findings in a review body or top-level comment, answer in the
appropriate discussion surface and link the disposition from the status
comment. Substantive disagreement stays explicit until reassessed.

Never dismiss a human's changes request yourself. After addressing it, re-request
that reviewer and keep the blocker/owner/next step explicit until they reassess.
Do not repeatedly request Copilot reviews merely to obtain favorable wording.
Distinguish:

| Evidence | Meaning |
|----------|---------|
| Current-head Copilot review with zero actionable findings | Technical review evidence, not a human approval |
| Formal `APPROVED` review | An approval event; still check author, SHA and repository rules |
| Human review needed, with no substantive reservations | Approval remains pending; technical readiness may still be true |
| Unresolved defect, substantive reservation or changes request | Readiness is blocked until addressed and reassessed |

The absence of the exact words "Approval recommended" is not a defect. Do not
make fake changes, rubber-stamp, or argue an automated reviewer into approval.

### Verify checks and integration

Required CI must be completed and green for the **latest head** and the
integration/merge commit where applicable, not an older successful run. Inspect
live required-check policy as well as actual runs: missing required checks are
pending, never a pass. Accept neutral/skipped only where expected under
repository policy. Do not treat an inaccessible policy or unknown mergeability
as verified readiness.

For failure, inspect the failing job and logs first. Retry only a bounded number
of times when evidence shows a transient infrastructure failure; record why.
Fix deterministic failures. Never weaken assertions, bypass checks, or dismiss
failures as "environmental" without investigation and a concrete resolution.

Use bounded waits/backoff rather than a tight polling loop (for example, inspect
after 30, 60 and 120 seconds, then checkpoint). Prefer notifications or an
available supported wakeup mechanism. This skill **cannot wake an idle agent**.
If no mechanism is configured, say "Not actively monitoring" and name the pending
gate and resume action. Do not claim to be watching indefinitely.

### Set exactly one lifecycle label

For an open PR managed by this workflow, the following labels are mutually
exclusive. Preserve unrelated labels.

| Label | Apply when |
|-------|------------|
| `agent-preparing` | The agent is fixing findings or waiting normally for CI/review; readiness has not yet been verified |
| `agent-blocked` | Progress requires external intervention; name the concrete blocker, owner and next step |
| `agent-ready-for-review` | All technical gates are verified for the final current head/base; **not** human approval or merge permission |

Ordinary pending human approval alone does not prevent technical readiness.
A substantive reservation or outstanding changes request does. Agent-fixable
findings mean preparing; a necessary reviewer reassessment or other external
intervention means blocked.

Before **each label transition**, fetch a live snapshot. Before ready, confirm:
independent final-diff coverage, no unresolved actionable findings/reservations,
required CI completed successfully, required base/stack integration current,
and no merge conflicts. Re-fetch head/base (and relevant parent/main), discussion
and checks after assessment. If any changed, reassess rather than applying a
stale result. Unknown conflict state means not ready.

Replace the other lifecycle labels with the selected one. When adopting this
PR, also remove legacy `pr-review-done` if present; it is not a readiness signal
for this workflow. Create missing labels only with authorization. Do not relabel
unrelated PRs or redefine/delete their labels. If permissions prevent a change,
report the intended state and failed action; never claim a label was applied.
Re-read labels and head/base after writing; if a race invalidated readiness,
remove the ready label and reassess.

A new commit, actionable finding or conflict invalidates readiness; an unrelated
comment does not. Base/main movement requires appropriate integration and
revalidation, not a promise the PR stays permanently clean. When next invoked on
a merged/closed PR, remove active lifecycle labels and stop; do not reopen it.

### Useful read-only GitHub commands

Set `$repo` and `$number` from the actual PR, not from an unrelated checkout.
These reads supplement, not replace, the complete discussion and policy checks:

```powershell
gh pr view $number --repo $repo --json url,state,headRefOid,headRefName,headRepository,baseRefName,baseRefOid,isDraft,mergeable,mergeStateStatus,reviewDecision,body,labels,statusCheckRollup
gh pr checks $number --repo $repo --required
gh api --paginate "repos/$repo/pulls/$number/reviews"
gh api --paginate "repos/$repo/issues/$number/comments"
gh api --paginate "repos/$repo/pulls/$number/comments"
# URL-encode the actual base ref as a single path segment for this endpoint.
gh api "repos/$repo/rules/branches/$([Uri]::EscapeDataString($baseRef))"
# For a failing GitHub Actions run; use its provider's logs for external CI.
gh run view $runId --repo $repo --log-failed
```

REST pull comments do not expose thread resolution state. Use a GraphQL
`pullRequest.reviewThreads(first:100, after:...)` query with `pageInfo` and
`comments(first:100, after:...)`, fetching every thread page **and every nested
comment page**. Capture each thread `id` and comment `databaseId` for the reply
tool. If GraphQL/SSO access is unavailable, report the missing thread assessment
as a gate; a REST-only partial read is not a clean review.

End with the PR link, actual state, and any remaining blocker/next action.
Separate "technically ready" from "approved" and "merged"; never imply either
of the latter from a lifecycle label.
6 changes: 4 additions & 2 deletions .github/skills/pr-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,10 @@ needs proof. Zero findings is a great result.
Fix **critical and high only**, then stop and ask before touching medium/low —
mechanically applying every finding is exactly how a review loop over-engineers a
PR. Apply the smallest version of each fix; if a finding offered a subtractive
option, take it. Never push fixes onto the branch under review — use a new branch
and a separate PR.
option, take it. Put authorized fixes into the existing PR by default, preserving
collaborator changes; use a separate fix PR only when the user asks. For publishing
and readiness tracking, use [pr-lifecycle](../pr-lifecycle/SKILL.md). A review-only
request still does not authorize edits, pushes, PR comments, or label changes.

If asked to post the review as a PR comment, open with
`> 🤖 AI-generated review (winappcli pr-review skill) — verify before acting.`
Expand Down
Loading